diff --git a/docs/protocols/alpha.rst b/docs/protocols/alpha.rst index cb81495d1cdb572ef75cdd4d183ab6f6f6a0a7f7..54bf92b78aa74905f5b26dab30bc1f5b706369f9 100644 --- a/docs/protocols/alpha.rst +++ b/docs/protocols/alpha.rst @@ -87,6 +87,18 @@ Minor Changes - Errors related to consensus operations have been reworked. See ``Validate_errors.Consensus``. (MR :gl:`!5927`) +- A delegate can no longer propose the same protocol hash multiple + times in Proposals operations. An operation that contains a proposal + which has already been proposed by the delegate in the same voting + period will now fail (and so will an operation that contains + multiple occurrences of the same proposal). This prevents the replay + of a Proposals operation. (MR :gl:`!5828`) + +- Change the names and types of errors related to voting operations + (Proposals and Ballot), and move them to ``Validate_errors``. + (MR :gl:`!5828`) + + Internal -------- @@ -126,3 +138,19 @@ Internal - Move the checks part of consensus operation to ``validate_operation.ml``. The effects part remains in ``apply_operation``. (MR :gl:`!5927`) + +- Implement ``Validate_operation.validate_operation`` on voting + operations (Proposals and Ballot). The checks are now done there, + while ``Apply.apply_operation`` only applies the effects. + (MR :gl:`!5828`) + +- A Testnet Dictator Proposals operation is now mutually exclusive + with any other voting operation inside a same block or mempool. + (MR :gl:`!5828`) + +- Remove redundant ``Delegate_storage.pubkey`` and use directly + ``Contract_manager_storage.get_manager_key`` instead. In situations + where the later used to fail with ``Unregistered_delegate``, we now + get either ``Missing_manager_contract`` or + ``Unrevealed_manager_key``, which describe the issue more + precisely. (MR :gl:`!5828`) diff --git a/src/proto_alpha/lib_plugin/RPC.ml b/src/proto_alpha/lib_plugin/RPC.ml index fc28a53f0c829390fbbc7ff41008c1a42dea36b5..80323ac05ecd0eb0ad91160be91032464121bcd3 100644 --- a/src/proto_alpha/lib_plugin/RPC.ml +++ b/src/proto_alpha/lib_plugin/RPC.ml @@ -373,11 +373,6 @@ module Scripts = struct ~query:RPC_query.empty RPC_path.(path / "normalize_type") - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/3401 - - Contrary to its description, this RPC currently does check the - signature of voting operations. This is in the process of being - fixed. *) let run_operation = RPC_service.post_service ~description: diff --git a/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL b/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL index 781e110a3ecdbebbfdd021e0ba39cfde58d94271..97a71eb9497f459ce75710a907f54331d6c4bd43 100644 --- a/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL +++ b/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL @@ -216,8 +216,8 @@ "Dal_apply", "Baking", - "Amendment", "Validate_errors", + "Amendment", "Validate_operation", "Apply", diff --git a/src/proto_alpha/lib_protocol/alpha_context.mli b/src/proto_alpha/lib_protocol/alpha_context.mli index cf70be6a50476c9117912cd2c118dd1ab8aeb0b7..d7dfee4067b75127297a01f725d872c3462cc64c 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/alpha_context.mli @@ -2573,6 +2573,7 @@ module Voting_period : sig val get_rpc_succ_info : context -> info tzresult Lwt.t module Testnet_dictator : sig + (** See {!Voting_period_storage.Testnet_dictator.overwrite_current_kind}. *) val overwrite_current_kind : context -> Chain_id.t -> Voting_period_repr.kind -> context tzresult Lwt.t end @@ -2582,16 +2583,24 @@ end module Vote : sig type proposal = Protocol_hash.t - val record_proposal : - context -> Protocol_hash.t -> public_key_hash -> context tzresult Lwt.t + (** See {!Vote_storage.get_delegate_proposal_count}. *) + val get_delegate_proposal_count : + context -> public_key_hash -> int tzresult Lwt.t + + (** See {!Vote_storage.set_delegate_proposal_count}. *) + val set_delegate_proposal_count : + context -> public_key_hash -> int -> context Lwt.t + + (** See {!Vote_storage.has_proposed}. *) + val has_proposed : context -> public_key_hash -> proposal -> bool Lwt.t + + (** See {!Vote_storage.add_proposal}. *) + val add_proposal : context -> public_key_hash -> proposal -> context Lwt.t val get_proposals : context -> int64 Protocol_hash.Map.t tzresult Lwt.t val clear_proposals : context -> context Lwt.t - val recorded_proposal_count_for_delegate : - context -> public_key_hash -> int tzresult Lwt.t - val listings_encoding : (Signature.Public_key_hash.t * int64) list Data_encoding.t @@ -2664,13 +2673,20 @@ module Vote : sig val set_participation_ema : context -> int32 -> context tzresult Lwt.t + (** See {!Vote_storage.current_proposal_exists}. *) + val current_proposal_exists : context -> bool Lwt.t + + (** See {!Vote_storage.get_current_proposal}. *) val get_current_proposal : context -> proposal tzresult Lwt.t + (** See {!Vote_storage.find_current_proposal}. *) val find_current_proposal : context -> proposal option tzresult Lwt.t + (** See {!Vote_storage.init_current_proposal}. *) val init_current_proposal : context -> proposal -> context tzresult Lwt.t - val clear_current_proposal : context -> context tzresult Lwt.t + (** See {!Vote_storage.clear_current_proposal}. *) + val clear_current_proposal : context -> context Lwt.t end module Dal : sig diff --git a/src/proto_alpha/lib_protocol/amendment.ml b/src/proto_alpha/lib_protocol/amendment.ml index 9bc9a8d4829f513dfe0e429ce0a3e6d676800ac4..08cb2a151633678ddf59f1443b6a286df4f21dcd 100644 --- a/src/proto_alpha/lib_protocol/amendment.ml +++ b/src/proto_alpha/lib_protocol/amendment.ml @@ -130,203 +130,87 @@ let start_new_voting_period ctxt = >>=? fun (ctxt, approved) -> if approved then Voting_period.succ ctxt else - Vote.clear_current_proposal ctxt >>=? fun ctxt -> + Vote.clear_current_proposal ctxt >>= fun ctxt -> Voting_period.reset ctxt | Cooldown -> Voting_period.succ ctxt | Promotion -> get_approval_and_update_participation_ema ctxt >>=? fun (ctxt, approved) -> if approved then Voting_period.succ ctxt - else Vote.clear_current_proposal ctxt >>=? Voting_period.reset + else Vote.clear_current_proposal ctxt >>= Voting_period.reset | Adoption -> Vote.get_current_proposal ctxt >>=? fun proposal -> activate ctxt proposal >>= fun ctxt -> - Vote.clear_current_proposal ctxt >>=? Voting_period.reset) + Vote.clear_current_proposal ctxt >>= Voting_period.reset) >>=? fun ctxt -> Vote.update_listings ctxt -type error += - | (* `Branch *) - Invalid_proposal - | Unexpected_proposal - | Unauthorized_proposal - | Too_many_proposals - | Empty_proposal - | Unexpected_ballot - | Unauthorized_ballot - | Duplicate_ballot - | (* `Permanent *) - Invalid_dictator_proposal - -let () = - let open Data_encoding in - (* Invalid proposal *) - register_error_kind - `Branch - ~id:"invalid_proposal" - ~title:"Invalid proposal" - ~description:"Ballot provided for a proposal that is not the current one." - ~pp:(fun ppf () -> Format.fprintf ppf "Invalid proposal") - empty - (function Invalid_proposal -> Some () | _ -> None) - (fun () -> Invalid_proposal) ; - (* Unexpected proposal *) - register_error_kind - `Branch - ~id:"unexpected_proposal" - ~title:"Unexpected proposal" - ~description:"Proposal recorded outside of a proposal period." - ~pp:(fun ppf () -> Format.fprintf ppf "Unexpected proposal") - empty - (function Unexpected_proposal -> Some () | _ -> None) - (fun () -> Unexpected_proposal) ; - (* Unauthorized proposal *) - register_error_kind - `Branch - ~id:"unauthorized_proposal" - ~title:"Unauthorized proposal" - ~description: - "The delegate provided for the proposal is not in the voting listings." - ~pp:(fun ppf () -> Format.fprintf ppf "Unauthorized proposal") - empty - (function Unauthorized_proposal -> Some () | _ -> None) - (fun () -> Unauthorized_proposal) ; - (* Unexpected ballot *) - register_error_kind - `Branch - ~id:"unexpected_ballot" - ~title:"Unexpected ballot" - ~description:"Ballot recorded outside of a voting period." - ~pp:(fun ppf () -> Format.fprintf ppf "Unexpected ballot") - empty - (function Unexpected_ballot -> Some () | _ -> None) - (fun () -> Unexpected_ballot) ; - (* Unauthorized ballot *) - register_error_kind - `Branch - ~id:"unauthorized_ballot" - ~title:"Unauthorized ballot" - ~description: - "The delegate provided for the ballot is not in the voting listings." - ~pp:(fun ppf () -> Format.fprintf ppf "Unauthorized ballot") - empty - (function Unauthorized_ballot -> Some () | _ -> None) - (fun () -> Unauthorized_ballot) ; - (* Duplicate ballot *) - register_error_kind - `Branch - ~id:"duplicate_ballot" - ~title:"Duplicate ballot" - ~description:"The delegate has already submitted a ballot." - ~pp:(fun ppf () -> Format.fprintf ppf "Duplicate ballot") - empty - (function Duplicate_ballot -> Some () | _ -> None) - (fun () -> Duplicate_ballot) ; - (* Too many proposals *) - register_error_kind - `Branch - ~id:"too_many_proposals" - ~title:"Too many proposals" - ~description:"The delegate reached the maximum number of allowed proposals." - ~pp:(fun ppf () -> Format.fprintf ppf "Too many proposals") - empty - (function Too_many_proposals -> Some () | _ -> None) - (fun () -> Too_many_proposals) ; - (* Empty proposal *) - register_error_kind - `Branch - ~id:"empty_proposal" - ~title:"Empty proposal" - ~description:"Proposal lists cannot be empty." - ~pp:(fun ppf () -> Format.fprintf ppf "Empty proposal") - empty - (function Empty_proposal -> Some () | _ -> None) - (fun () -> Empty_proposal) ; - (* Invalid dictator proposal *) - register_error_kind - `Permanent - ~id:"invalid_dictator_proposal" - ~title:"Invalid dictator proposal" - ~description:"A testnet dictator can only submit one proposal at a time." - ~pp:(fun ppf () -> - Format.fprintf - ppf - "A testnet dictator can only submit one proposal at a time.") - empty - (function Invalid_dictator_proposal -> Some () | _ -> None) - (fun () -> Invalid_dictator_proposal) - -let record_delegate_proposals ctxt delegate proposals = - (match proposals with - | [] -> error Empty_proposal - | _ :: _ -> Result.return_unit) - >>?= fun () -> - Voting_period.get_current_kind ctxt >>=? function - | Proposal -> - Vote.in_listings ctxt delegate >>= fun in_listings -> - if in_listings then ( - Vote.recorded_proposal_count_for_delegate ctxt delegate - >>=? fun count -> - assert (Compare.Int.(Constants.max_proposals_per_delegate >= count)) ; - error_when - Compare.Int.( - List.compare_length_with - proposals - (Constants.max_proposals_per_delegate - count) - > 0) - Too_many_proposals - >>?= fun () -> - List.fold_left_es - (fun ctxt proposal -> Vote.record_proposal ctxt proposal delegate) - ctxt - proposals) - else fail Unauthorized_proposal - | Exploration | Cooldown | Promotion | Adoption -> fail Unexpected_proposal - -let record_testnet_dictator_proposals ctxt chain_id proposals = - Vote.clear_ballots ctxt >>= fun ctxt -> - Vote.clear_proposals ctxt >>= fun ctxt -> - Vote.clear_current_proposal ctxt >>=? fun ctxt -> - match proposals with - | [] -> - Voting_period.Testnet_dictator.overwrite_current_kind - ctxt - chain_id - Proposal - | [proposal] -> - Vote.init_current_proposal ctxt proposal >>=? fun ctxt -> - Voting_period.Testnet_dictator.overwrite_current_kind - ctxt - chain_id - Adoption - | _ :: _ :: _ -> fail Invalid_dictator_proposal +let may_start_new_voting_period ctxt = + Voting_period.is_last_block ctxt >>=? fun is_last -> + if is_last then start_new_voting_period ctxt else return ctxt let is_testnet_dictator ctxt chain_id delegate = (* This function should always, ALWAYS, return false on mainnet!!!! *) match Constants.testnet_dictator ctxt with - | Some pkh when Chain_id.(chain_id <> Constants_repr.mainnet_id) -> + | Some pkh when Chain_id.(chain_id <> Constants.mainnet_id) -> Signature.Public_key_hash.equal pkh delegate | _ -> false -let record_proposals ctxt chain_id delegate proposals = - if is_testnet_dictator ctxt chain_id delegate then - record_testnet_dictator_proposals ctxt chain_id proposals - else record_delegate_proposals ctxt delegate proposals +(** {2 Application of voting operations} *) -let record_ballot ctxt delegate proposal ballot = - Voting_period.get_current_kind ctxt >>=? function - | Exploration | Promotion -> - Vote.get_current_proposal ctxt >>=? fun current_proposal -> - error_unless - (Protocol_hash.equal proposal current_proposal) - Invalid_proposal - >>?= fun () -> - Vote.has_recorded_ballot ctxt delegate >>= fun has_ballot -> - error_when has_ballot Duplicate_ballot >>?= fun () -> - Vote.in_listings ctxt delegate >>= fun in_listings -> - if in_listings then Vote.record_ballot ctxt delegate ballot - else fail Unauthorized_ballot - | Cooldown | Proposal | Adoption -> fail Unexpected_ballot +(** Helpers to apply [Proposals] operations from a + registered dictator of a test chain. These operations let the + dictator immediately change the current voting period's kind, and + the current proposal if applicable. Of course, there must never be + such a dictator on mainnet. *) +module Testnet_dictator = struct + (** Forcibly update the voting period according to a voting + dictator's Proposals operation. -let may_start_new_voting_period ctxt = - Voting_period.is_last_block ctxt >>=? fun is_last -> - if is_last then start_new_voting_period ctxt else return ctxt + {!check_proposals} should guarantee that this function cannot + return an error. *) + let record_proposals ctxt chain_id proposals = + let open Lwt_tzresult_syntax in + let*! ctxt = Vote.clear_ballots ctxt in + let*! ctxt = Vote.clear_proposals ctxt in + let*! ctxt = Vote.clear_current_proposal ctxt in + match proposals with + | [] -> + Voting_period.Testnet_dictator.overwrite_current_kind + ctxt + chain_id + Proposal + | [proposal] -> + let* ctxt = Vote.init_current_proposal ctxt proposal in + Voting_period.Testnet_dictator.overwrite_current_kind + ctxt + chain_id + Adoption + | _ :: _ :: _ -> + (* This does not fail if validate proposal was previously + called. *) + fail Validate_errors.Voting.Testnet_dictator_multiple_proposals +end + +let apply_proposals ctxt chain_id (Proposals {source; period = _; proposals}) = + let open Lwt_tzresult_syntax in + let* ctxt = + if is_testnet_dictator ctxt chain_id source then + Testnet_dictator.record_proposals ctxt chain_id proposals + else + let* count = Vote.get_delegate_proposal_count ctxt source in + let new_count = count + List.length proposals in + let*! ctxt = Vote.set_delegate_proposal_count ctxt source new_count in + let*! ctxt = + List.fold_left_s + (fun ctxt proposal -> Vote.add_proposal ctxt source proposal) + ctxt + proposals + in + return ctxt + in + return (ctxt, Apply_results.Single_result Proposals_result) + +let apply_ballot ctxt (Ballot {source; period = _; proposal = _; ballot}) = + let open Lwt_tzresult_syntax in + let* ctxt = Vote.record_ballot ctxt source ballot in + return (ctxt, Apply_results.Single_result Ballot_result) diff --git a/src/proto_alpha/lib_protocol/amendment.mli b/src/proto_alpha/lib_protocol/amendment.mli index fda070b1ac8d1ccb9163fb8d773fd238926043fa..328b30691826fccca861b6eb1c5e91234b0ce8a9 100644 --- a/src/proto_alpha/lib_protocol/amendment.mli +++ b/src/proto_alpha/lib_protocol/amendment.mli @@ -74,36 +74,56 @@ open Alpha_context the state machine of the amendment procedure. *) val may_start_new_voting_period : context -> context tzresult Lwt.t -(** Records a list of proposals for a delegate. - @raise Unexpected_proposal if [ctxt] is not in a proposal period. - @raise Unauthorized_proposal if [delegate] is not in the listing. *) -val record_proposals : +(** Check whether the given public key hash corresponds to the + registered testchain dictator, if any. This function will always + return false on mainnet. *) +val is_testnet_dictator : context -> Chain_id.t -> public_key_hash -> bool + +(** {2 Application of voting operations} + + There are two kinds of voting operations: + + - Proposals: A delegate submits a list of protocol amendment + proposals. This operation is only accepted during a Proposal period + (see above). + + - Ballot: A delegate casts a vote for/against the current proposal + (or pass). This operation is only accepted during an Exploration + or Promotion period (see above). *) + +(** Update the [context] with the effects of a Proposals operation: + + - Its proposals are added to the source's recorded proposals. + + - The recorded proposal count of the source is increased by the + number of proposals in the operation. + + Note that a Proposals operation from a testnet dictator (which may + be set up when a test chain is initialized) has completely + different effects: + + - If the operation contains no proposal, then the current voting + period is immediately and forcibly set to a Proposal period. + + - If the operation contains exactly one proposal, then the current + voting period is immediately and forcibly set to an Adoption period + for this proposal. + + {!validate_proposals} must have been called beforehand, and is + responsible for ensuring that [apply_proposals] cannot fail. *) +val apply_proposals : context -> Chain_id.t -> - public_key_hash -> - Protocol_hash.t list -> - context tzresult Lwt.t - -type error += - | Invalid_proposal - | Unexpected_proposal - | Unauthorized_proposal - | Too_many_proposals - | Empty_proposal - | Unexpected_ballot - | Unauthorized_ballot - | Duplicate_ballot - -(** Records a vote for a delegate if the current voting period is - Exploration or Promotion. - @raise Invalid_proposal if [proposal] ≠ [current_proposal]. - @raise Duplicate_ballot if delegate already voted. - @raise Unauthorized_ballot if delegate is not listed to vote, - or if current period differs from Exploration or Promotion. -*) -val record_ballot : + Kind.proposals contents -> + (context * Kind.proposals Apply_results.contents_result_list) tzresult Lwt.t + +(** Update the [context] with the effects of a Ballot operation: + + The couple (source of the operation, submitted ballot) is recorded. + + {!validate_ballot} must have been called beforehand, and is + responsible for ensuring that [apply_ballot] cannot fail. *) +val apply_ballot : context -> - public_key_hash -> - Protocol_hash.t -> - Vote.ballot -> - context tzresult Lwt.t + Kind.ballot contents -> + (context * Kind.ballot Apply_results.contents_result_list) tzresult Lwt.t diff --git a/src/proto_alpha/lib_protocol/apply.ml b/src/proto_alpha/lib_protocol/apply.ml index 92fc6e43f47437f524528f317e54cf94e7954251..03d7399fc9a8fddb40b098b7ee1d28cd61daf0e2 100644 --- a/src/proto_alpha/lib_protocol/apply.ml +++ b/src/proto_alpha/lib_protocol/apply.ml @@ -40,11 +40,9 @@ type error += | Tx_rollup_invalid_transaction_ticket_amount | Cannot_transfer_ticket_to_implicit | Sc_rollup_feature_disabled - | Wrong_voting_period of {expected : int32; provided : int32} | Internal_operation_replay of Apply_internal_results.packed_internal_operation | Multiple_revelation - | Failing_noop_error | Zero_frozen_deposits of Signature.Public_key_hash.t | Invalid_transfer_to_sc_rollup_from_implicit_account @@ -197,20 +195,6 @@ let () = (function Sc_rollup_feature_disabled -> Some () | _ -> None) (fun () -> Sc_rollup_feature_disabled) ; - register_error_kind - `Temporary - ~id:"operation.wrong_voting_period" - ~title:"Wrong voting period" - ~description: - "Trying to include a proposal or ballot meant for another voting period" - ~pp:(fun ppf (e, p) -> - Format.fprintf ppf "Wrong voting period %ld, current is %ld" p e) - Data_encoding.( - obj2 (req "current_index" int32) (req "provided_index" int32)) - (function - | Wrong_voting_period {expected; provided} -> Some (expected, provided) - | _ -> None) - (fun (expected, provided) -> Wrong_voting_period {expected; provided}) ; register_error_kind `Permanent ~id:"internal_operation_replay" @@ -237,20 +221,6 @@ let () = Data_encoding.empty (function Multiple_revelation -> Some () | _ -> None) (fun () -> Multiple_revelation) ; - register_error_kind - `Permanent - ~id:"operation.failing_noop" - ~title:"Failing_noop operations are not executed by the protocol" - ~description: - "The failing_noop operation is an operation that is not and never will \ - be executed by the protocol." - ~pp:(fun ppf () -> - Format.fprintf - ppf - "The failing_noop operation cannot be executed by the protocol") - Data_encoding.empty - (function Failing_noop_error -> Some () | _ -> None) - (fun () -> Failing_noop_error) ; register_error_kind `Permanent ~id:"delegate.zero_frozen_deposits" @@ -2081,8 +2051,7 @@ let punish_double_baking ctxt (bh1 : Block_header.t) ~payload_producer = (fun balance_updates -> Double_baking_evidence_result balance_updates) let apply_contents_list (type kind) ctxt chain_id (apply_mode : apply_mode) - ~payload_producer op_validated_stamp (operation : kind operation) - (contents_list : kind contents_list) : + ~payload_producer op_validated_stamp (contents_list : kind contents_list) : (context * kind contents_result_list) tzresult Lwt.t = let mempool_mode = match apply_mode with @@ -2142,29 +2111,13 @@ let apply_contents_list (type kind) ctxt chain_id (apply_mode : apply_mode) Token.transfer ctxt src (`Contract contract) amount >>=? fun (ctxt, bupds) -> return (ctxt, Single_result (Activate_account_result bupds)) - | Single (Proposals {source; period; proposals}) -> - Delegate.pubkey ctxt source >>=? fun delegate -> - Operation.check_signature delegate chain_id operation >>?= fun () -> - Voting_period.get_current ctxt >>=? fun {index = current_period; _} -> - error_unless - Compare.Int32.(current_period = period) - (Wrong_voting_period {expected = current_period; provided = period}) - >>?= fun () -> - Amendment.record_proposals ctxt chain_id source proposals >|=? fun ctxt -> - (ctxt, Single_result Proposals_result) - | Single (Ballot {source; period; proposal; ballot}) -> - Delegate.pubkey ctxt source >>=? fun delegate -> - Operation.check_signature delegate chain_id operation >>?= fun () -> - Voting_period.get_current ctxt >>=? fun {index = current_period; _} -> - error_unless - Compare.Int32.(current_period = period) - (Wrong_voting_period {expected = current_period; provided = period}) - >>?= fun () -> - Amendment.record_ballot ctxt source proposal ballot >|=? fun ctxt -> - (ctxt, Single_result Ballot_result) + | Single (Proposals _ as contents) -> + Amendment.apply_proposals ctxt chain_id contents + | Single (Ballot _ as contents) -> Amendment.apply_ballot ctxt contents | Single (Failing_noop _) -> - (* Failing_noop _ always fails *) - fail Failing_noop_error + (* This operation always fails. It should already have been + rejected by {!Validate_operation.validate_operation}. *) + fail Validate_errors.Failing_noop_error | Single (Manager_operation _) -> apply_manager_operations ctxt @@ -2192,7 +2145,6 @@ let apply_operation ctxt chain_id (apply_mode : apply_mode) ~payload_producer apply_mode ~payload_producer op_validated_stamp - operation operation.protocol_data.contents >|=? fun (ctxt, result) -> let ctxt = Gas.set_unlimited ctxt in diff --git a/src/proto_alpha/lib_protocol/apply.mli b/src/proto_alpha/lib_protocol/apply.mli index 9c401388e4111ea74b62b73a6b35724f7f4dfd03..fbd99dc7f5308e6c9d3f93928b17dbe3b495558d 100644 --- a/src/proto_alpha/lib_protocol/apply.mli +++ b/src/proto_alpha/lib_protocol/apply.mli @@ -43,7 +43,6 @@ type error += | Tx_rollup_invalid_transaction_ticket_amount | Sc_rollup_feature_disabled | Empty_transaction of Contract.t - | Wrong_voting_period of {expected : int32; provided : int32} val begin_partial_construction : context -> @@ -109,15 +108,6 @@ type apply_mode = For non-manager operations, the application of a validated operation should always fully succeed. - TODO: https://gitlab.com/tezos/tezos/-/issues/2603 - - Currently, {!Validate_operation.validate_operation} does nothing - on voting operations. The "validation" of these operations is - instead handled by [apply_operation], which may thus return an - error if the operation is ill-formed. Once [validate_operation] has - been extended to every kind of operation, [apply_operation] should - never return an error. - For manager operations, the application has two stages. The first stage consists in updating the context to: diff --git a/src/proto_alpha/lib_protocol/dune b/src/proto_alpha/lib_protocol/dune index 5bb4173c8a56f155cb641565482f504fcfd7205f..1ca2458363a80dfe406f1bdc436d14cee7c99a82 100644 --- a/src/proto_alpha/lib_protocol/dune +++ b/src/proto_alpha/lib_protocol/dune @@ -227,8 +227,8 @@ Sc_rollup_operations Dal_apply Baking - Amendment Validate_errors + Amendment Validate_operation Apply Services_registration @@ -473,8 +473,8 @@ sc_rollup_operations.ml sc_rollup_operations.mli dal_apply.ml dal_apply.mli baking.ml baking.mli - amendment.ml amendment.mli validate_errors.ml validate_errors.mli + amendment.ml amendment.mli validate_operation.ml validate_operation.mli apply.ml apply.mli services_registration.ml services_registration.mli @@ -699,8 +699,8 @@ sc_rollup_operations.ml sc_rollup_operations.mli dal_apply.ml dal_apply.mli baking.ml baking.mli - amendment.ml amendment.mli validate_errors.ml validate_errors.mli + amendment.ml amendment.mli validate_operation.ml validate_operation.mli apply.ml apply.mli services_registration.ml services_registration.mli @@ -930,8 +930,8 @@ sc_rollup_operations.ml sc_rollup_operations.mli dal_apply.ml dal_apply.mli baking.ml baking.mli - amendment.ml amendment.mli validate_errors.ml validate_errors.mli + amendment.ml amendment.mli validate_operation.ml validate_operation.mli apply.ml apply.mli services_registration.ml services_registration.mli diff --git a/src/proto_alpha/lib_protocol/test/helpers/assert.ml b/src/proto_alpha/lib_protocol/test/helpers/assert.ml index 91f1ba7971f0d7dc4c6771f911d085fba2eac100..e4739337d3af90873f6faaff0ac08bd12d3a4282 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/assert.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/assert.ml @@ -157,6 +157,25 @@ let not_equal_pkh ~loc (a : Signature.Public_key_hash.t) let module PKH = Signature.Public_key_hash in not_equal ~loc PKH.equal "Public key hashes are equal" PKH.pp a b +(* protocol hash *) +let equal_protocol_hash ~loc (a : Protocol_hash.t) (b : Protocol_hash.t) = + equal + ~loc + Protocol_hash.equal + "Protocol hashes aren't equal" + Protocol_hash.pp + a + b + +let not_equal_protocol_hash ~loc (a : Protocol_hash.t) (b : Protocol_hash.t) = + not_equal + ~loc + Protocol_hash.equal + "Protocol hashes are equal" + Protocol_hash.pp + a + b + let get_some ~loc = function | Some x -> return x | None -> failwith "Unexpected None (%s)" loc diff --git a/src/proto_alpha/lib_protocol/test/helpers/context.ml b/src/proto_alpha/lib_protocol/test/helpers/context.ml index b102aa4a31ede8e50dda2f9a10fe711696e2bed5..822d166f16d5d74240c167ae5d8e212cbbf92d4c 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/context.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/context.ml @@ -265,9 +265,9 @@ module Vote = struct remaining_proposals : int; } - let recorded_proposal_count_for_delegate ctxt pkh = + let get_delegate_proposal_count ctxt pkh = to_alpha_ctxt ctxt >>=? fun alpha_ctxt -> - Vote.recorded_proposal_count_for_delegate alpha_ctxt pkh + Vote.get_delegate_proposal_count alpha_ctxt pkh >|= Environment.wrap_tzresult end diff --git a/src/proto_alpha/lib_protocol/test/helpers/context.mli b/src/proto_alpha/lib_protocol/test/helpers/context.mli index 66fed3b9e5c5966ee04b024d8c75bad8a91d44a2..071960e5a7592f3793d8cca1bf658d81a6d3624e 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/context.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/context.mli @@ -124,12 +124,11 @@ module Vote : sig remaining_proposals : int; } - (** See {!Vote_storage.recorded_proposal_count_for_delegate}. + (** See {!Vote_storage.get_delegate_proposal_count}. Note that unlike most functions in the current module, this one does not call an RPC. *) - val recorded_proposal_count_for_delegate : - t -> public_key_hash -> int tzresult Lwt.t + val get_delegate_proposal_count : t -> public_key_hash -> int tzresult Lwt.t end module Contract : sig diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_failing_noop.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_failing_noop.ml index 798c949977fe70025f4b21a23999b24b9dd536db..917fe3dec5f429d2ce65d8d48b7278aad45bf027 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_failing_noop.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_failing_noop.ml @@ -43,10 +43,9 @@ let failing_noop_must_fail_when_injected () = let source = Context.Contract.pkh contract in Op.failing_noop (B blk) source "tezos" >>=? fun operation -> Block.bake ~operation blk >>= fun res -> - Assert.proto_error_with_info - ~loc:__LOC__ - res - "Failing_noop operations are not executed by the protocol" + Assert.proto_error ~loc:__LOC__ res (function + | Protocol.Validate_errors.Failing_noop_error -> true + | _ -> false) let tests = [Tztest.tztest "injection fails" `Quick failing_noop_must_fail_when_injected] diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_voting.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_voting.ml index c4076897eb588b8f4fe92bdcb115c3becc2a69f7..772137cfa47585f4fd9422a4426eb404059a4ff1 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_voting.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_voting.ml @@ -287,9 +287,9 @@ let context_init2 = context_init_tup Context.T2 blocks in order to move on to an Exploration period. Return a block, a delegate (distinct from the one who submitted the Proposals), and the current proposal. *) -let context_init_exploration ?(proposal = protos.(0)) () = +let context_init_exploration ?(proposal = protos.(0)) ?blocks_per_cycle () = let open Lwt_result_syntax in - let* block, (proposer, other_delegate) = context_init2 () in + let* block, (proposer, other_delegate) = context_init2 ?blocks_per_cycle () in let* operation = Op.proposals (B block) proposer [proposal] in let* block = Block.bake block ~operation in let* block = bake_until_first_block_of_next_period block in @@ -309,11 +309,6 @@ let wrong_error expected_error_name actual_error_trace loc = Error_monad.pp_print_trace actual_error_trace -let unregistered_delegate delegate loc = function - | [Environment.Ecoproto_error (Delegate_storage.Unregistered_delegate pkh)] -> - Assert.equal_pkh ~loc:(append_loc ~caller_loc:loc __LOC__) pkh delegate - | err -> wrong_error "Unregistered_delegate" err loc - let missing_signature loc = function | [Environment.Ecoproto_error Operation.Missing_signature] -> return_unit | err -> wrong_error "Missing_signature" err loc @@ -322,9 +317,11 @@ let invalid_signature loc = function | [Environment.Ecoproto_error Operation.Invalid_signature] -> return_unit | err -> wrong_error "Invalid_signature" err loc -let wrong_voting_period ~current_index ~op_index loc = function +open Validate_errors.Voting + +let wrong_voting_period_index ~current_index ~op_index loc = function | [ - Environment.Ecoproto_error (Apply.Wrong_voting_period {expected; provided}); + Environment.Ecoproto_error (Wrong_voting_period_index {expected; provided}); ] -> let open Lwt_result_syntax in let make_loc = append_loc ~caller_loc:loc in @@ -332,39 +329,76 @@ let wrong_voting_period ~current_index ~op_index loc = function Assert.equal_int32 ~loc:(make_loc __LOC__) expected current_index in Assert.equal_int32 ~loc:(make_loc __LOC__) provided op_index - | err -> wrong_error "Wrong_voting_period" err loc + | err -> wrong_error "Wrong_voting_period_index" err loc + +let wrong_voting_period_kind loc = function + | [Environment.Ecoproto_error (Wrong_voting_period_kind _)] -> return_unit + | err -> wrong_error "Wrong_voting_period_kind" err loc -let unexpected_proposal loc = function - | [Environment.Ecoproto_error Amendment.Unexpected_proposal] -> return_unit - | err -> wrong_error "Unexpected_proposal" err loc +let source_not_in_vote_listings loc = function + | [Environment.Ecoproto_error Source_not_in_vote_listings] -> return_unit + | err -> wrong_error "Source_not_in_vote_listings" err loc -let unauthorized_proposal loc = function - | [Environment.Ecoproto_error Amendment.Unauthorized_proposal] -> return_unit - | err -> wrong_error "Unauthorized_proposal" err loc +let empty_proposals loc = function + | [Environment.Ecoproto_error Empty_proposals] -> return_unit + | err -> wrong_error "Empty_proposals" err loc -let empty_proposal loc = function - | [Environment.Ecoproto_error Amendment.Empty_proposal] -> return_unit - | err -> wrong_error "Empty_proposal" err loc +let proposals_contain_duplicate duplicate_proposal loc = function + | [Environment.Ecoproto_error (Proposals_contain_duplicate {proposal})] -> + Assert.equal_protocol_hash + ~loc:(append_loc ~caller_loc:loc __LOC__) + proposal + duplicate_proposal + | err -> wrong_error "Proposals_contain_duplicate" err loc let too_many_proposals loc = function - | [Environment.Ecoproto_error Amendment.Too_many_proposals] -> return_unit + | [Environment.Ecoproto_error Too_many_proposals] -> return_unit | err -> wrong_error "Too_many_proposals" err loc -let unexpected_ballot loc = function - | [Environment.Ecoproto_error Amendment.Unexpected_ballot] -> return_unit - | err -> wrong_error "Unexpected_ballot" err loc - -let invalid_proposal loc = function - | [Environment.Ecoproto_error Amendment.Invalid_proposal] -> return_unit - | err -> wrong_error "Invalid_proposal" err loc +let already_proposed already_proposed_proposal loc = function + | [Environment.Ecoproto_error (Already_proposed {proposal})] -> + Assert.equal_protocol_hash + ~loc:(append_loc ~caller_loc:loc __LOC__) + proposal + already_proposed_proposal + | err -> wrong_error "Already_proposed" err loc + +let conflict_too_many_proposals loc = function + | [Environment.Ecoproto_error (Conflict_too_many_proposals _)] -> return_unit + | err -> wrong_error "Conflict_too_many_proposals" err loc + +let conflict_already_proposed already_proposed_proposal loc = function + | [Environment.Ecoproto_error (Conflict_already_proposed {proposal; _})] -> + Assert.equal_protocol_hash + ~loc:(append_loc ~caller_loc:loc __LOC__) + proposal + already_proposed_proposal + | err -> wrong_error "Conflict_already_proposed" err loc + +let ballot_for_wrong_proposal ~current_proposal ~op_proposal loc = function + | [ + Environment.Ecoproto_error (Ballot_for_wrong_proposal {current; submitted}); + ] -> + let open Lwt_result_syntax in + let* () = + Assert.equal_protocol_hash + ~loc:(append_loc ~caller_loc:loc __LOC__) + current_proposal + current + in + Assert.equal_protocol_hash + ~loc:(append_loc ~caller_loc:loc __LOC__) + op_proposal + submitted + | err -> wrong_error "Ballot_for_wrong_proposal" err loc -let duplicate_ballot loc = function - | [Environment.Ecoproto_error Amendment.Duplicate_ballot] -> return_unit - | err -> wrong_error "Duplicate_ballot" err loc +let already_submitted_a_ballot loc = function + | [Environment.Ecoproto_error Already_submitted_a_ballot] -> return_unit + | err -> wrong_error "Already_submitted_a_ballot" err loc -let unauthorized_ballot loc = function - | [Environment.Ecoproto_error Amendment.Unauthorized_ballot] -> return_unit - | err -> wrong_error "Unauthorized_ballot" err loc +let conflicting_ballot loc = function + | [Environment.Ecoproto_error (Conflicting_ballot _)] -> return_unit + | err -> wrong_error "Conflicting_ballot" err loc let assert_validate_proposals_fails ~expected_error ~proposer ~proposals ?period block loc = @@ -483,7 +517,7 @@ let test_successful_vote num_delegates () = >>=? fun () -> (* proposing less than one proposal fails *) assert_validate_proposals_fails - ~expected_error:empty_proposal + ~expected_error:empty_proposals ~proposer:del1 ~proposals:[] b @@ -533,7 +567,7 @@ let test_successful_vote num_delegates () = belongs to [delegates_p2], so they have already sent a ballot during the unanimous vote right above). *) assert_validate_ballot_fails - ~expected_error:duplicate_ballot + ~expected_error:already_submitted_a_ballot ~voter:del1 ~proposal:Protocol_hash.zero ~ballot:Vote.Nay @@ -803,37 +837,6 @@ let test_not_enough_quorum_in_promotion num_delegates () = assert_period ~expected_kind:Proposal b __LOC__ >>=? fun () -> assert_listings_not_empty b ~loc:__LOC__ >>=? fun () -> return_unit -(** Identical proposals (identified by their hash) must be counted as - one. *) -let test_multiple_identical_proposals_count_as_one () = - context_init 1 () >>=? fun (b, delegates) -> - assert_period ~expected_kind:Proposal b __LOC__ >>=? fun () -> - let proposer = WithExceptions.Option.get ~loc:__LOC__ @@ List.hd delegates in - Op.proposals (B b) proposer [Protocol_hash.zero; Protocol_hash.zero] - >>=? fun operation -> - Block.bake ~operation b >>=? fun b -> - (* compute the weight of proposals *) - Context.Vote.get_proposals (B b) >>=? fun ps -> - (* compute the voting power of proposer *) - let pkh = Context.Contract.pkh proposer in - Context.Vote.get_listings (B b) >>=? fun l -> - (match List.find_opt (fun (del, _) -> del = pkh) l with - | None -> failwith "%s - Missing delegate" __LOC__ - | Some (_, proposer_power) -> return proposer_power) - >>=? fun proposer_power -> - (* correctly count the double proposal for zero as one proposal *) - let expected_weight_proposer = proposer_power in - match Environment.Protocol_hash.(Map.find zero ps) with - | Some v -> - if v = expected_weight_proposer then return_unit - else - failwith - "%s - Wrong count %Ld is not %Ld; identical proposals count as one" - __LOC__ - v - expected_weight_proposer - | None -> failwith "%s - Missing proposal" __LOC__ - (** Assume the initial balance of accounts allocated by Context.init_n is at least 4 times the value of the minimal_stake constant. *) let test_supermajority_in_proposal there_is_a_winner () = @@ -1238,19 +1241,6 @@ let test_voting_period_pp () = (** {3 Proposal -- Negative tests} *) -(** Test that a Proposals operation fails when its source is not a - registered delegate. *) -let test_proposals_unregistered_delegate () = - let open Lwt_result_syntax in - let* block, _delegate = context_init1 () in - let fresh_account = Account.new_account () in - assert_validate_proposals_fails - ~expected_error:(unregistered_delegate fresh_account.pkh) - ~proposer:(Contract.Implicit fresh_account.pkh) - ~proposals:[Protocol_hash.zero] - block - __LOC__ - (** Test that a Proposals operation fails when it is unsigned. *) let test_proposals_missing_signature () = let open Lwt_result_syntax in @@ -1275,14 +1265,14 @@ let test_proposals_invalid_signature () = (** Test that a Proposals operation fails when the period index provided in the operation is not the current voting period index. *) -let test_proposals_wrong_voting_period () = +let test_proposals_wrong_voting_period_index () = let open Lwt_result_syntax in let* block, proposer = context_init1 () in let* current_period = Context.Vote.get_current_period (B block) in let current_index = current_period.voting_period.index in let op_index = Int32.succ current_index in assert_validate_proposals_fails - ~expected_error:(wrong_voting_period ~current_index ~op_index) + ~expected_error:(wrong_voting_period_index ~current_index ~op_index) ~proposer ~proposals:[Protocol_hash.zero] ~period:op_index @@ -1291,13 +1281,13 @@ let test_proposals_wrong_voting_period () = (** Test that a Proposals operation fails when it occurs in a non-Proposal voting period. *) -let test_unexpected_proposal () = +let test_proposals_wrong_voting_period_kind () = let open Lwt_result_syntax in let* block, proposer = context_init1 () in let proposal = protos.(0) in let assert_proposals_fails_with_unexpected_proposal = assert_validate_proposals_fails - ~expected_error:unexpected_proposal + ~expected_error:wrong_voting_period_kind ~proposer ~proposals:[proposal] in @@ -1331,34 +1321,58 @@ let test_unexpected_proposal () = assert_proposals_fails_with_unexpected_proposal block __LOC__ (** Test that a Proposals operation fails when the proposer is not in - the vote listings. *) -let test_unauthorized_proposal () = + the vote listings (with the same error, no matter how far the + source is from being a delegate with voting rights). *) +let test_proposals_source_not_in_vote_listings () = let open Lwt_result_syntax in - let* block, funder = context_init1 () in - let account = Account.new_account () in - let proposer = Contract.Implicit account.pkh in + (* The chosen [blocks_per_cycle] is an arbitrary value that we will + not reach with the blocks baked in this test. *) + let* block, funder = context_init1 ~blocks_per_cycle:10l () in + let fresh_account = Account.new_account () in + let proposer = Contract.Implicit fresh_account.pkh in + let assert_fails_with_source_not_in_vote_listings block = + assert_validate_proposals_fails + ~expected_error:source_not_in_vote_listings + ~proposer + ~proposals:[Protocol_hash.zero] + block + in + (* Fail when the source has no contract in the storage. *) + let* () = assert_fails_with_source_not_in_vote_listings block __LOC__ in let* operation = Op.transaction (B block) funder proposer Tez.one in let* block = Block.bake block ~operation in - let* operation = - Op.delegation ~force_reveal:true (B block) proposer (Some account.pkh) - in + (* Fail when the contract's public key is unreavealed. *) + let* () = assert_fails_with_source_not_in_vote_listings block __LOC__ in + let* operation = Op.revelation (B block) fresh_account.pk in + let* block = Block.bake block ~operation in + (* Fail when the source is not a delegate. *) + let* () = assert_fails_with_source_not_in_vote_listings block __LOC__ in + let* operation = Op.delegation (B block) proposer (Some fresh_account.pkh) in let* block = Block.bake block ~operation in + (* Fail when the source is a delegate, but not yet in the vote listings. *) + assert_fails_with_source_not_in_vote_listings block __LOC__ + +(** Test that a Proposals operation fails when its proposal list is + empty. *) +let test_empty_proposals () = + let open Lwt_result_syntax in + let* block, proposer = context_init1 () in assert_validate_proposals_fails - ~expected_error:unauthorized_proposal + ~expected_error:empty_proposals ~proposer - ~proposals:[Protocol_hash.zero] + ~proposals:[] block __LOC__ -(** Test that a Proposals operation fails when its proposal list is - empty. *) -let test_empty_proposal () = +(** Test that a Proposals operation fails when its proposal list + contains multiple occurrences of the same proposal. *) +let test_proposals_contain_duplicate () = let open Lwt_result_syntax in let* block, proposer = context_init1 () in assert_validate_proposals_fails - ~expected_error:empty_proposal + ~expected_error:(proposals_contain_duplicate protos.(1)) ~proposer - ~proposals:[] + ~proposals:[protos.(0); protos.(1); protos.(2); protos.(1); protos.(3)] block __LOC__ @@ -1397,6 +1411,104 @@ let test_too_many_proposals () = block __LOC__ +(** Test that a Proposals operation fails when one of its proposals has + already been submitted by the same proposer in an earlier block. *) +let test_already_proposed () = + let open Lwt_result_syntax in + let* block, proposer = context_init1 () in + let* operation = Op.proposals (B block) proposer [protos.(0); protos.(1)] in + let* block = Block.bake block ~operation in + (* The [proposer] cannot submit protocol [0] again. *) + let* () = + assert_validate_proposals_fails + ~expected_error:(already_proposed protos.(0)) + ~proposer + ~proposals:[protos.(0)] + block + __LOC__ + in + (* The [proposer] cannot submit protocol [1] again, even among other + new proposals. *) + let* () = + assert_validate_proposals_fails + ~expected_error:(already_proposed protos.(1)) + ~proposer + ~proposals:[protos.(2); protos.(1); protos.(3)] + block + __LOC__ + in + (* The initial [operation] cannot be replayed. *) + let* () = + Incremental.assert_validate_operation_fails + (already_proposed protos.(0) __LOC__) + operation + block + in + let* block = bake_until_first_block_of_next_period block in + Incremental.assert_validate_operation_fails + (wrong_voting_period_index ~current_index:1l ~op_index:0l __LOC__) + operation + block + +(** Test that a Proposals operation fails when it would make the total + count of proposals submitted by the proposer exceed the + [max_proposals_per_delegate] protocol constant, because of + previously validated operations in the current block/mempool. *) +let test_conflict_too_many_proposals () = + let open Lwt_result_syntax in + let* block, proposer = context_init1 () in + let n_proposals_in_previous_blocks = 5 in + assert (Array.length protos >= Constants.max_proposals_per_delegate + 1) ; + let proposals_in_previous_blocks = + List.map (Array.get protos) (1 -- n_proposals_in_previous_blocks) + in + let* operation = + Op.proposals (B block) proposer proposals_in_previous_blocks + in + let* block = Block.bake block ~operation in + let* current_block_state = Incremental.begin_construction block in + let proposals_in_current_block = + List.map + (Array.get protos) + (n_proposals_in_previous_blocks + 1 + -- Constants.max_proposals_per_delegate) + in + let* op_in_current_block = + Op.proposals (B block) proposer proposals_in_current_block + in + let* current_block_state = + Incremental.validate_operation current_block_state op_in_current_block + in + let* op = Op.proposals (B block) proposer [protos.(0)] in + let* _i = + Incremental.validate_operation + ~expect_failure:(conflict_too_many_proposals __LOC__) + current_block_state + op + in + return_unit + +(** Test that a Proposals operation fails when one of its proposals + has already been submitted by the same proposer in a previously + validated operation of the current block/mempool. *) +let test_conflict_already_proposed () = + let open Lwt_result_syntax in + let* block, proposer = context_init1 () in + let proposal = protos.(0) in + let* current_block_state = Incremental.begin_construction block in + let* op_in_current_block = Op.proposals (B block) proposer [proposal] in + let* current_block_state = + Incremental.validate_operation current_block_state op_in_current_block + in + let* op = Op.proposals (B block) proposer [proposal] in + let* _i = + Incremental.validate_operation + ~expect_failure:(conflict_already_proposed proposal __LOC__) + current_block_state + op + in + return_unit + (** {3 Proposals -- Positive test} A Proposals operation is valid when: @@ -1484,10 +1596,10 @@ let observe_proposals pre_state post_state op caller_loc = proposals) ; (* Check [Storage.Vote.Proposals_count] update. *) let* proposal_count_pre = - Context.Vote.recorded_proposal_count_for_delegate (B pre_state) source + Context.Vote.get_delegate_proposal_count (B pre_state) source in let* proposal_count_post = - Context.Vote.recorded_proposal_count_for_delegate (B post_state) source + Context.Vote.get_delegate_proposal_count (B post_state) source in let* () = Assert.equal_int @@ -1547,43 +1659,8 @@ let test_valid_proposals () = let* b4 = Block.bake b3 ~operation:op3 in observe_proposals b3 b4 op3 __LOC__ -(** {3 Proposals -- Incoming semantic change} *) - -(** Test that a Proposals operation can be replayed - (this will no longer be true in an upcoming commit). *) -let test_replay_proposals () = - let open Lwt_result_syntax in - let* bpre, proposer = context_init1 ~blocks_per_cycle:10l () in - let* operation = Op.proposals (B bpre) proposer [protos.(0)] in - let* bpost = Block.bake bpre ~operation in - let* () = observe_proposals bpre bpost operation __LOC__ in - (* We do not observe the effects of replayed operations because they - are different from fresh operations: since the proposals were - already recorded for the proposer, - [voting_infos.remaining_proposals] does not decrease, nor does - the total supporting weight increase. - - We simply check that [Block.bake] does not fail. *) - let* bpost = Block.bake bpost ~operation in - let* _bpost = Block.bake bpost ~operations:[operation; operation] in - return_unit - (** {3 Ballot -- Negative tests} *) -(** Test that a Ballot operation fails when its source is not a - registered delegate. *) -let test_ballot_unregistered_delegate () = - let open Lwt_result_syntax in - let* block, _delegate, proposal = context_init_exploration () in - let fresh_account = Account.new_account () in - assert_validate_ballot_fails - ~expected_error:(unregistered_delegate fresh_account.pkh) - ~voter:(Contract.Implicit fresh_account.pkh) - ~proposal - ~ballot:Vote.Yay - block - __LOC__ - (** Test that a Ballot operation fails when it is unsigned. *) let test_ballot_missing_signature () = let open Lwt_result_syntax in @@ -1608,14 +1685,14 @@ let test_ballot_invalid_signature () = (** Test that a Ballot operation fails when the period index provided in the operation is not the current voting period index. *) -let test_ballot_wrong_voting_period () = +let test_ballot_wrong_voting_period_index () = let open Lwt_result_syntax in let* block, voter = context_init1 () in let* current_period = Context.Vote.get_current_period (B block) in let current_index = current_period.voting_period.index in let op_index = Int32.succ current_index in assert_validate_ballot_fails - ~expected_error:(wrong_voting_period ~current_index ~op_index) + ~expected_error:(wrong_voting_period_index ~current_index ~op_index) ~voter ~proposal:protos.(0) ~ballot:Vote.Yay @@ -1625,13 +1702,13 @@ let test_ballot_wrong_voting_period () = (** Test that a Ballot operation fails when it occurs outside of an Exploration or Promotion voting period. *) -let test_unexpected_ballot () = +let test_ballot_wrong_voting_period_kind () = let open Lwt_result_syntax in let* block, voter = context_init1 () in let proposal = protos.(0) in let assert_ballot_fails_with_unexpected_ballot = assert_validate_ballot_fails - ~expected_error:unexpected_ballot + ~expected_error:wrong_voting_period_kind ~voter ~proposal ~ballot:Vote.Nay @@ -1664,28 +1741,29 @@ let test_unexpected_ballot () = (** Test that a Ballot operation fails when its proposal is not the current proposal. *) -let test_ballot_on_invalid_proposal () = +let test_ballot_for_wrong_proposal () = let open Lwt_result_syntax in - let* block, voter, _proposal = + let* block, voter, current_proposal = context_init_exploration ~proposal:protos.(0) () in + let op_proposal = protos.(1) in assert_validate_ballot_fails - ~expected_error:invalid_proposal + ~expected_error:(ballot_for_wrong_proposal ~current_proposal ~op_proposal) ~voter - ~proposal:protos.(1) + ~proposal:op_proposal ~ballot:Vote.Yay block __LOC__ (** Test that a Ballot operation fails when its source has already submitted a Ballot. *) -let test_duplicate_ballot () = +let test_already_submitted_a_ballot () = let open Lwt_result_syntax in let* block, voter, proposal = context_init_exploration () in let* operation = Op.ballot (B block) voter proposal Vote.Yay in let* block = Block.bake ~operation block in assert_validate_ballot_fails - ~expected_error:duplicate_ballot + ~expected_error:already_submitted_a_ballot ~voter ~proposal ~ballot:Vote.Nay @@ -1693,25 +1771,59 @@ let test_duplicate_ballot () = __LOC__ (** Test that a Ballot operation fails when its source is not in the - vote listings. *) -let test_unauthorized_ballot () = + vote listings (with the same error, no matter how far the source is + from being a delegate with voting rights). *) +let test_ballot_source_not_in_vote_listings () = let open Lwt_result_syntax in - let* block, funder, proposal = context_init_exploration () in - let account = Account.new_account () in - let voter = Contract.Implicit account.pkh in + let* block, funder, proposal = + (* The chosen [blocks_per_cycle] is an arbitrary value that we + will not reach with the blocks baked in this test. *) + context_init_exploration ~blocks_per_cycle:10l () + in + let fresh_account = Account.new_account () in + let voter = Contract.Implicit fresh_account.pkh in + let assert_fails_with_source_not_in_vote_listings block = + assert_validate_ballot_fails + ~expected_error:source_not_in_vote_listings + ~voter + ~proposal + ~ballot:Vote.Yay + block + in + (* Fail when the source has no contract in the storage. *) + let* () = assert_fails_with_source_not_in_vote_listings block __LOC__ in let* operation = Op.transaction (B block) funder voter Tez.one in let* block = Block.bake block ~operation in - let* operation = - Op.delegation ~force_reveal:true (B block) voter (Some account.pkh) - in + (* Fail when the contract's public key is unreavealed. *) + let* () = assert_fails_with_source_not_in_vote_listings block __LOC__ in + let* operation = Op.revelation (B block) fresh_account.pk in let* block = Block.bake block ~operation in - assert_validate_ballot_fails - ~expected_error:unauthorized_ballot - ~voter - ~proposal - ~ballot:Vote.Yay - block - __LOC__ + (* Fail when the source is not a delegate. *) + let* () = assert_fails_with_source_not_in_vote_listings block __LOC__ in + let* operation = Op.delegation (B block) voter (Some fresh_account.pkh) in + let* block = Block.bake block ~operation in + (* Fail when the source is a delegate, but not yet in the vote listings. *) + assert_fails_with_source_not_in_vote_listings block __LOC__ + +(** Test that a Ballot operation fails when its source has already + submitted a Ballot in a previously validated operation of the + current block. *) +let test_conflicting_ballot () = + let open Lwt_result_syntax in + let* block, voter, proposal = context_init_exploration () in + let* current_block_state = Incremental.begin_construction block in + let* op_in_current_block = Op.ballot (B block) voter proposal Vote.Yay in + let* current_block_state = + Incremental.validate_operation current_block_state op_in_current_block + in + let* op = Op.ballot (B block) voter proposal Vote.Nay in + let* _i = + Incremental.validate_operation + ~expect_failure:(conflicting_ballot __LOC__) + current_block_state + op + in + return_unit (** {3 Ballot -- Positive test} @@ -1808,6 +1920,8 @@ let observe_ballot pre_state post_state op caller_loc = let test_valid_ballot () = let open Lwt_result_syntax in + (* The chosen [blocks_per_cycle] is an arbitrary value that we will + not reach with the blocks baked in this test. *) let* block, delegates = context_init ~blocks_per_cycle:10l 4 () in let* proposer, voter1, voter2, voter3 = match delegates with @@ -1840,10 +1954,6 @@ let tests = "voting promotion, not enough quorum" `Quick (test_not_enough_quorum_in_promotion 432); - Tztest.tztest - "voting counting double proposal" - `Quick - test_multiple_identical_proposals_count_as_one; Tztest.tztest "voting proposal, with supermajority" `Quick @@ -1886,10 +1996,6 @@ let tests = test_voting_power_updated_each_voting_period; Tztest.tztest "voting period pretty print" `Quick test_voting_period_pp; (* Validity tests on Proposals *) - Tztest.tztest - "Proposals from unregistered delegate" - `Quick - test_proposals_unregistered_delegate; Tztest.tztest "Proposals missing signature" `Quick @@ -1899,12 +2005,22 @@ let tests = `Quick test_proposals_invalid_signature; Tztest.tztest - "Proposals wrong voting period" + "Proposals wrong voting period index" + `Quick + test_proposals_wrong_voting_period_index; + Tztest.tztest + "Proposals wrong voting period kind" + `Quick + test_proposals_wrong_voting_period_kind; + Tztest.tztest + "Proposals source not in vote listings" + `Quick + test_proposals_source_not_in_vote_listings; + Tztest.tztest "Empty proposals" `Quick test_empty_proposals; + Tztest.tztest + "Proposals contain a duplicate proposal" `Quick - test_proposals_wrong_voting_period; - Tztest.tztest "Unexpected proposals" `Quick test_unexpected_proposal; - Tztest.tztest "Unauthorized proposal" `Quick test_unauthorized_proposal; - Tztest.tztest "Empty proposal" `Quick test_empty_proposal; + test_proposals_contain_duplicate; Tztest.tztest "Operation has too many proposals" `Quick @@ -1913,13 +2029,20 @@ let tests = "Too many proposals (over two operations)" `Quick test_too_many_proposals; - Tztest.tztest "Valid Proposals operations" `Quick test_valid_proposals; - Tztest.tztest "Replay proposals" `Quick test_replay_proposals; - (* Validity tests on Ballot *) Tztest.tztest - "Ballot from unregistered delegate" + "A proposal had already been proposed" + `Quick + test_already_proposed; + Tztest.tztest + "Conflict: too many proposals in current block/mempool" `Quick - test_ballot_unregistered_delegate; + test_conflict_too_many_proposals; + Tztest.tztest + "Conflict: proposal already proposed in current block/mempool" + `Quick + test_conflict_already_proposed; + Tztest.tztest "Valid Proposals operations" `Quick test_valid_proposals; + (* Validity tests on Ballot *) Tztest.tztest "Ballot missing signature" `Quick @@ -1929,15 +2052,28 @@ let tests = `Quick test_ballot_invalid_signature; Tztest.tztest - "Ballot wrong voting period" + "Ballot wrong voting period index" + `Quick + test_ballot_wrong_voting_period_index; + Tztest.tztest + "Ballot wrong voting period kind" + `Quick + test_ballot_wrong_voting_period_kind; + Tztest.tztest + "Ballot for wrong proposal" + `Quick + test_ballot_for_wrong_proposal; + Tztest.tztest + "Delegate has already submitted a ballot" + `Quick + test_already_submitted_a_ballot; + Tztest.tztest + "Ballot source not in vote listings" `Quick - test_ballot_wrong_voting_period; - Tztest.tztest "Unexpected ballot" `Quick test_unexpected_ballot; + test_ballot_source_not_in_vote_listings; Tztest.tztest - "Ballot on invalid proposal" + "Conflicting ballot in current block/mempool" `Quick - test_ballot_on_invalid_proposal; - Tztest.tztest "Duplicate ballot" `Quick test_duplicate_ballot; - Tztest.tztest "Unauthorized ballot" `Quick test_unauthorized_ballot; + test_conflicting_ballot; Tztest.tztest "Valid Ballot operations" `Quick test_valid_ballot; ] diff --git a/src/proto_alpha/lib_protocol/validate_errors.ml b/src/proto_alpha/lib_protocol/validate_errors.ml index 0f4ab0674ff4751aa7bf5cf5a5dd68d1b21eacf3..52b0f78dbf077ba079d26ed91ce6be0e023f16e2 100644 --- a/src/proto_alpha/lib_protocol/validate_errors.ml +++ b/src/proto_alpha/lib_protocol/validate_errors.ml @@ -380,6 +380,352 @@ module Consensus = struct (fun endorser -> Conflicting_dal_slot_availability {endorser}) end +module Voting = struct + type error += + | (* Shared voting errors *) + Wrong_voting_period_index of { + expected : int32; + provided : int32; + } + | Wrong_voting_period_kind of { + current : Voting_period.kind; + expected : Voting_period.kind list; + } + | Source_not_in_vote_listings + | (* Proposals errors *) + Empty_proposals + | Proposals_contain_duplicate of {proposal : Protocol_hash.t} + | Too_many_proposals + | Already_proposed of {proposal : Protocol_hash.t} + | Conflict_too_many_proposals of { + max_allowed : int; + count_previous_blocks : int; + count_current_block : int; + count_operation : int; + conflicting_operations : Operation_hash.t list; + } + | Conflict_already_proposed of { + proposal : Protocol_hash.t; + conflicting_operation : Operation_hash.t; + } + | Conflicting_dictator_proposals of Operation_hash.t + | Testnet_dictator_multiple_proposals + | Testnet_dictator_conflicting_operation + | (* Ballot errors *) + Ballot_for_wrong_proposal of { + current : Protocol_hash.t; + submitted : Protocol_hash.t; + } + | Already_submitted_a_ballot + | Conflicting_ballot of {conflicting_operation : Operation_hash.t} + + let () = + (* Shared voting errors *) + register_error_kind + `Temporary + ~id:"validate_operation.wrong_voting_period_index" + ~title:"Wrong voting period index" + ~description: + "The voting operation contains a voting period index different from \ + the current one." + ~pp:(fun ppf (expected, provided) -> + Format.fprintf + ppf + "The voting operation is meant for voting period %ld, whereas the \ + current period is %ld." + provided + expected) + Data_encoding.( + obj2 (req "current_index" int32) (req "provided_index" int32)) + (function + | Wrong_voting_period_index {expected; provided} -> + Some (expected, provided) + | _ -> None) + (fun (expected, provided) -> + Wrong_voting_period_index {expected; provided}) ; + register_error_kind + `Temporary + ~id:"validate_operation.wrong_voting_period_kind" + ~title:"Wrong voting period kind" + ~description: + "The voting operation is incompatible the current voting period kind." + ~pp:(fun ppf (current, expected) -> + Format.fprintf + ppf + "The voting operation is only valid during a %a voting period, but \ + we are currently in a %a period." + (Format.pp_print_list + ~pp_sep:(fun fmt () -> Format.fprintf fmt " or ") + Voting_period.pp_kind) + expected + Voting_period.pp_kind + current) + Data_encoding.( + obj2 + (req "current" Voting_period.kind_encoding) + (req "expected" (list Voting_period.kind_encoding))) + (function + | Wrong_voting_period_kind {current; expected} -> + Some (current, expected) + | _ -> None) + (fun (current, expected) -> Wrong_voting_period_kind {current; expected}) ; + let description = "The delegate is not in the vote listings." in + register_error_kind + `Temporary + ~id:"validate_operation.source_not_in_vote_listings" + ~title:"Source not in vote listings" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function Source_not_in_vote_listings -> Some () | _ -> None) + (fun () -> Source_not_in_vote_listings) ; + + (* Proposals errors *) + let description = "Proposal list cannot be empty." in + register_error_kind + `Permanent + ~id:"validate_operation.empty_proposals" + ~title:"Empty proposals" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function Empty_proposals -> Some () | _ -> None) + (fun () -> Empty_proposals) ; + register_error_kind + `Permanent + ~id:"validate_operation.proposals_contain_duplicate" + ~title:"Proposals contain duplicate" + ~description:"The list of proposals contains a duplicate element." + ~pp:(fun ppf proposal -> + Format.fprintf + ppf + "The list of proposals contains multiple occurrences of the proposal \ + %a." + Protocol_hash.pp + proposal) + Data_encoding.(obj1 (req "proposal" Protocol_hash.encoding)) + (function + | Proposals_contain_duplicate {proposal} -> Some proposal | _ -> None) + (fun proposal -> Proposals_contain_duplicate {proposal}) ; + let description = + "The proposer exceeded the maximum number of allowed proposals." + in + register_error_kind + `Branch + ~id:"validate_operation.too_many_proposals" + ~title:"Too many proposals" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function Too_many_proposals -> Some () | _ -> None) + (fun () -> Too_many_proposals) ; + register_error_kind + `Branch + ~id:"validate_operation.already_proposed" + ~title:"Already proposed" + ~description: + "The delegate has already submitted one of the operation's proposals." + ~pp:(fun ppf proposal -> + Format.fprintf + ppf + "The delegate has already submitted the proposal %a." + Protocol_hash.pp + proposal) + Data_encoding.(obj1 (req "proposal" Protocol_hash.encoding)) + (function Already_proposed {proposal} -> Some proposal | _ -> None) + (fun proposal -> Already_proposed {proposal}) ; + register_error_kind + `Temporary + ~id:"validate_operation.conflict_too_many_proposals" + ~title:"Conflict too many proposals" + ~description: + "The delegate exceeded the maximum number of allowed proposals due to, \ + among others, previous Proposals operations in the current \ + block/mempool." + ~pp: + (fun ppf + ( max_allowed, + count_previous_blocks, + count_current_block, + count_operation, + conflicting_operations ) -> + Format.fprintf + ppf + "The delegate has submitted too many proposals (the maximum allowed \ + is %d): %d in previous blocks, %d in the considered operation, and \ + %d in the following validated operations of the current \ + block/mempool: %a." + max_allowed + count_previous_blocks + count_operation + count_current_block + (Format.pp_print_list Operation_hash.pp) + conflicting_operations) + Data_encoding.( + obj5 + (req "max_allowed" int8) + (req "count_previous_blocks" int8) + (req "count_current_block" int8) + (req "count_operation" int8) + (req "conflicting_operations" (list Operation_hash.encoding))) + (function + | Conflict_too_many_proposals + { + max_allowed; + count_previous_blocks; + count_current_block; + count_operation; + conflicting_operations; + } -> + Some + ( max_allowed, + count_previous_blocks, + count_current_block, + count_operation, + conflicting_operations ) + | _ -> None) + (fun ( max_allowed, + count_previous_blocks, + count_current_block, + count_operation, + conflicting_operations ) -> + Conflict_too_many_proposals + { + max_allowed; + count_previous_blocks; + count_current_block; + count_operation; + conflicting_operations; + }) ; + register_error_kind + `Temporary + ~id:"validate_operation.conflict_already_proposed" + ~title:"Conflict already proposed" + ~description: + "The delegate has already submitted one of the operation's proposals \ + in a previously validated operation of the current block or mempool." + ~pp:(fun ppf (proposal, conflicting_oph) -> + Format.fprintf + ppf + "The delegate has already proposed the protocol hash %a in the \ + previously validated operation %a of the current block or mempool." + Protocol_hash.pp + proposal + Operation_hash.pp + conflicting_oph) + Data_encoding.( + obj2 + (req "proposal" Protocol_hash.encoding) + (req "conflicting_operation" Operation_hash.encoding)) + (function + | Conflict_already_proposed {proposal; conflicting_operation} -> + Some (proposal, conflicting_operation) + | _ -> None) + (fun (proposal, conflicting_operation) -> + Conflict_already_proposed {proposal; conflicting_operation}) ; + register_error_kind + `Branch + ~id:"validate_operation.conflicting_dictator_proposals" + ~title:"Conflicting dictator proposals" + ~description: + "The current block/mempool already contains a testnest dictator \ + proposals operation, so it cannot have any other voting operation." + ~pp:(fun ppf dictator_operation -> + Format.fprintf + ppf + "The current block/mempool already contains the testnest dictator \ + proposals operation %a, so it cannot have any other voting \ + operation." + Operation_hash.pp + dictator_operation) + Data_encoding.(obj1 (req "dictator_operation" Operation_hash.encoding)) + (function Conflicting_dictator_proposals oph -> Some oph | _ -> None) + (fun oph -> Conflicting_dictator_proposals oph) ; + let description = + "A testnet dictator cannot submit more than one proposal at a time." + in + register_error_kind + `Permanent + ~id:"validate_operation.testnet_dictator_multiple_proposals" + ~title:"Testnet dictator multiple proposals" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function Testnet_dictator_multiple_proposals -> Some () | _ -> None) + (fun () -> Testnet_dictator_multiple_proposals) ; + let description = + "A testnet dictator proposals operation cannot be included in a block or \ + mempool that already contains any other voting operation." + in + register_error_kind + `Branch + ~id:"validate_operation.testnet_dictator_conflicting_operation" + ~title:"Testnet dictator conflicting operation" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function Testnet_dictator_conflicting_operation -> Some () | _ -> None) + (fun () -> Testnet_dictator_conflicting_operation) ; + + (* Ballot errors *) + register_error_kind + `Branch + ~id:"validate_operation.ballot_for_wrong_proposal" + ~title:"Ballot for wrong proposal" + ~description:"Ballot provided for a proposal that is not the current one." + ~pp:(fun ppf (current, submitted) -> + Format.fprintf + ppf + "Ballot provided for proposal %a whereas the current proposal is %a." + Protocol_hash.pp + submitted + Protocol_hash.pp + current) + Data_encoding.( + obj2 + (req "current_proposal" Protocol_hash.encoding) + (req "ballot_proposal" Protocol_hash.encoding)) + (function + | Ballot_for_wrong_proposal {current; submitted} -> + Some (current, submitted) + | _ -> None) + (fun (current, submitted) -> + Ballot_for_wrong_proposal {current; submitted}) ; + let description = + "The delegate has already submitted a ballot for the current voting \ + period." + in + register_error_kind + `Branch + ~id:"validate_operation.already_submitted_a_ballot" + ~title:"Already submitted a ballot" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function Already_submitted_a_ballot -> Some () | _ -> None) + (fun () -> Already_submitted_a_ballot) ; + register_error_kind + `Temporary + ~id:"validate_operation.conflicting_ballot" + ~title:"Conflicting ballot" + ~description: + "The delegate has already submitted a ballot in a previously validated \ + operation of the current block or mempool." + ~pp:(fun ppf conflicting_oph -> + Format.fprintf + ppf + "The delegate has already submitted a ballot in the previously \ + validated operation %a of the current block or mempool." + Operation_hash.pp + conflicting_oph) + Data_encoding.(obj1 (req "conflicting_operation" Operation_hash.encoding)) + (function + | Conflicting_ballot {conflicting_operation} -> + Some conflicting_operation + | _ -> None) + (fun conflicting_operation -> Conflicting_ballot {conflicting_operation}) +end + module Anonymous = struct type error += | Invalid_activation of {pkh : Ed25519.Public_key_hash.t} @@ -816,3 +1162,17 @@ module Manager = struct (function Sc_rollup_feature_disabled -> Some () | _ -> None) (fun () -> Sc_rollup_feature_disabled) end + +type error += Failing_noop_error + +let () = + let description = "A failing_noop operation can never be validated." in + register_error_kind + `Permanent + ~id:"validate_operation.failing_noop_error" + ~title:"Failing_noop error" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function Failing_noop_error -> Some () | _ -> None) + (fun () -> Failing_noop_error) diff --git a/src/proto_alpha/lib_protocol/validate_errors.mli b/src/proto_alpha/lib_protocol/validate_errors.mli index 424c8bd820bf6c212836a5005d95e80ddb52e27e..2ff8a6403301ec2ed3b335094b680d9882fc72ef 100644 --- a/src/proto_alpha/lib_protocol/validate_errors.mli +++ b/src/proto_alpha/lib_protocol/validate_errors.mli @@ -23,6 +23,8 @@ (* *) (*****************************************************************************) +open Alpha_context + (** Errors that may arise while validating a consensus operation. *) module Consensus : sig type consensus_operation_kind = @@ -36,23 +38,23 @@ module Consensus : sig | Consensus_operation_not_allowed | Consensus_operation_for_old_level of { kind : consensus_operation_kind; - expected : Alpha_context.Raw_level.t; - provided : Alpha_context.Raw_level.t; + expected : Raw_level.t; + provided : Raw_level.t; } | Consensus_operation_for_future_level of { kind : consensus_operation_kind; - expected : Alpha_context.Raw_level.t; - provided : Alpha_context.Raw_level.t; + expected : Raw_level.t; + provided : Raw_level.t; } | Consensus_operation_for_old_round of { kind : consensus_operation_kind; - expected : Alpha_context.Round.t; - provided : Alpha_context.Round.t; + expected : Round.t; + provided : Round.t; } | Consensus_operation_for_future_round of { kind : consensus_operation_kind; - expected : Alpha_context.Round.t; - provided : Alpha_context.Round.t; + expected : Round.t; + provided : Round.t; } | Wrong_consensus_operation_branch of { kind : consensus_operation_kind; @@ -67,8 +69,8 @@ module Consensus : sig | Unexpected_preendorsement_in_block | Unexpected_endorsement_in_block | Preendorsement_round_too_high of { - block_round : Alpha_context.Round.t; - provided : Alpha_context.Round.t; + block_round : Round.t; + provided : Round.t; } | Wrong_slot_used_for_consensus_operation of { kind : consensus_operation_kind; @@ -79,6 +81,47 @@ module Consensus : sig } end +(** Errors that may arise while validating a voting operation. *) +module Voting : sig + type error += + | (* Shared voting errors *) + Wrong_voting_period_index of { + expected : int32; + provided : int32; + } + | Wrong_voting_period_kind of { + current : Voting_period.kind; + expected : Voting_period.kind list; + } + | Source_not_in_vote_listings + | (* Proposals errors *) + Empty_proposals + | Proposals_contain_duplicate of {proposal : Protocol_hash.t} + | Too_many_proposals + | Already_proposed of {proposal : Protocol_hash.t} + | Conflict_too_many_proposals of { + max_allowed : int; + count_previous_blocks : int; + count_current_block : int; + count_operation : int; + conflicting_operations : Operation_hash.t list; + } + | Conflict_already_proposed of { + proposal : Protocol_hash.t; + conflicting_operation : Operation_hash.t; + } + | Conflicting_dictator_proposals of Operation_hash.t + | Testnet_dictator_multiple_proposals + | Testnet_dictator_conflicting_operation + | (* Ballot errors *) + Ballot_for_wrong_proposal of { + current : Protocol_hash.t; + submitted : Protocol_hash.t; + } + | Already_submitted_a_ballot + | Conflicting_ballot of {conflicting_operation : Operation_hash.t} +end + (** Errors that may arise while validating an anonymous operation. *) module Anonymous : sig type denunciation_kind = Preendorsement | Endorsement | Block @@ -89,11 +132,11 @@ module Anonymous : sig | Invalid_denunciation of denunciation_kind | Invalid_double_baking_evidence of { hash1 : Block_hash.t; - level1 : Alpha_context.Raw_level.t; - round1 : Alpha_context.Round.t; + level1 : Raw_level.t; + round1 : Round.t; hash2 : Block_hash.t; - level2 : Alpha_context.Raw_level.t; - round2 : Alpha_context.Round.t; + level2 : Raw_level.t; + round2 : Round.t; } | Inconsistent_denunciation of { kind : denunciation_kind; @@ -103,23 +146,23 @@ module Anonymous : sig | Already_denounced of { kind : denunciation_kind; delegate : Signature.Public_key_hash.t; - level : Alpha_context.Level.t; + level : Level.t; } | Conflicting_denunciation of { kind : denunciation_kind; delegate : Signature.Public_key_hash.t; - level : Alpha_context.Level.t; + level : Level.t; hash : Operation_hash.t; } | Too_early_denunciation of { kind : denunciation_kind; - level : Alpha_context.Raw_level.t; - current : Alpha_context.Raw_level.t; + level : Raw_level.t; + current : Raw_level.t; } | Outdated_denunciation of { kind : denunciation_kind; - level : Alpha_context.Raw_level.t; - last_cycle : Alpha_context.Cycle.t; + level : Raw_level.t; + last_cycle : Cycle.t; } | Conflicting_nonce_revelation end @@ -136,3 +179,5 @@ module Manager : sig | Tx_rollup_feature_disabled | Sc_rollup_feature_disabled end + +type error += Failing_noop_error diff --git a/src/proto_alpha/lib_protocol/validate_operation.ml b/src/proto_alpha/lib_protocol/validate_operation.ml index 6327d6ae871493887b1a68c7a14ee36ca03f34df..522b12506d020d54290e0c67fafc83a08279d4dc 100644 --- a/src/proto_alpha/lib_protocol/validate_operation.ml +++ b/src/proto_alpha/lib_protocol/validate_operation.ml @@ -130,6 +130,46 @@ let empty_consensus_state = dal_slot_availability_seen = Signature.Public_key_hash.Set.empty; } +(** Summary of previously validated Proposals operations by a given + proposer in the current block/mempool. *) +type proposer_history = { + count : int; + (** Total number of protocols submitted by the proposer in + previously validated operations. *) + operations : Operation_hash.t list; + (** Hashes of the previously validated Proposals operations from + the proposer. *) + proposed : Operation_hash.t Protocol_hash.Map.t; + (** A map indexed by the protocols that have been submitted by the + proposer in previously validated operations. Each protocol + points to the operation in which it was proposed. *) +} + +type voting_state = { + proposals_validated : proposer_history Signature.Public_key_hash.Map.t; + (** Summary of all Proposals operations validated in the current + block/mempool, indexed by the operation's source aka + proposer. *) + dictator_proposals_validated : Operation_hash.t option; + (** If a testnet dictator Proposals operation has been validated + in the current block/mempool, then its hash is recorded + here. Since such an operation can change the voting period + kind, it is mutually exclusive with any other voting operation + in a single block (otherwise we would loose the commutativity + of validated operation application: see + {!Validate_operation}). *) + ballots_validated : Operation_hash.t Signature.Public_key_hash.Map.t; + (** To each delegate that has submitted a ballot in a previously + validated operation, associates the hash of this operation. *) +} + +let empty_voting_state = + { + proposals_validated = Signature.Public_key_hash.Map.empty; + dictator_proposals_validated = None; + ballots_validated = Signature.Public_key_hash.Map.empty; + } + module Double_evidence = Map.Make (struct type t = Signature.Public_key_hash.t * Level.t @@ -222,7 +262,7 @@ let init_manager_state ctxt = type mode = Block | Mempool type validate_operation_info = { - ctxt : t; (** The context at the beginning of the block. *) + ctxt : t; (** The context at the beginning of the block or mempool. *) mode : mode; chain_id : Chain_id.t; (** Needed for signature checks. *) current_level : Level.t; @@ -232,6 +272,7 @@ type validate_operation_info = { type validate_operation_state = { consensus_state : consensus_state; + voting_state : voting_state; anonymous_state : anonymous_state; manager_state : manager_state; } @@ -251,6 +292,7 @@ let init_validate_operation_info ctxt mode chain_id let init_validate_operation_state ctxt = { consensus_state = empty_consensus_state; + voting_state = empty_voting_state; anonymous_state = empty_anonymous_state; manager_state = init_manager_state ctxt; } @@ -727,6 +769,422 @@ module Consensus = struct return (update_validity_state_dal_slot_availabitiy vs endorser) end +(** {2 Validation of voting operations} + + There are two kinds of voting operations: + + - Proposals: A delegate submits a list of protocol amendment + proposals. This operation is only accepted during a Proposal period + (see above). + + - Ballot: A delegate casts a vote for/against the current proposal + (or pass). This operation is only accepted during an Exploration + or Promotion period (see above). *) + +module Voting = struct + open Validate_errors.Voting + + let check_count_conflict ~count_previous_blocks ~count_operation + proposer_history = + let max_allowed = Constants.max_proposals_per_delegate in + let count_before_op = count_previous_blocks + proposer_history.count in + (* [count_before_op] should never have been increased above + [max_proposals_per_delegate]. *) + assert (Compare.Int.(count_before_op <= max_allowed)) ; + error_unless + Compare.Int.(count_before_op + count_operation <= max_allowed) + (Conflict_too_many_proposals + { + max_allowed; + count_previous_blocks; + count_current_block = proposer_history.count; + count_operation; + conflicting_operations = proposer_history.operations; + }) + + (** Check that a regular (ie. non-dictator) Proposals operation is + compatible with previously validated voting operations in the + current block/mempool, and update the [state] with this + operation. + + @return [Error Conflict_too_many_proposals] if the total number + of proposals by [proposer] in previously applied operations in + [ctxt], in previously validated operations in the current + block/mempool, and in the operation to validate, exceeds + {!Constants.max_proposals_per_delegate}. + + @return [Error Conflict_already_proposed] if one of the + operation's [proposals] has already been submitted by [proposer] + in the current block/mempool. + + @return [Error Conflicting_dictator_proposals] if the current + block/mempool already contains a testnet dictator Proposals + operation (see {!recfield:dictator_proposals_validated}). + + Note that this function is designed to be called in addition to + {!check_proposal_list_sanity} and {!check_count} further below, + not instead of them: that's why nothing is done when the + [proposer] is not in {!recfield:proposals_validated}. More + precisely, this function should be called {e after} the + aforementioned functions, whose potential errors + e.g. [Proposals_contain_duplicate] or [Too_many_proposals] should + take precedence because they are independent from the validation + [state]. *) + let check_proposals_conflicts_and_update_state state oph proposer proposals + ~count_in_ctxt ~proposals_length = + let open Tzresult_syntax in + let* new_proposer_history = + match + Signature.Public_key_hash.Map.find proposer state.proposals_validated + with + | None -> + let proposed = + List.fold_left + (fun acc proposal -> Protocol_hash.Map.add proposal oph acc) + Protocol_hash.Map.empty + proposals + in + return {count = proposals_length; operations = [oph]; proposed} + | Some proposer_history -> + let* () = + check_count_conflict + ~count_previous_blocks:count_in_ctxt + ~count_operation:proposals_length + proposer_history + in + let add_proposal proposed_map proposal = + match Protocol_hash.Map.find proposal proposer_history.proposed with + | Some conflicting_operation -> + error + (Conflict_already_proposed {proposal; conflicting_operation}) + | None -> ok (Protocol_hash.Map.add proposal oph proposed_map) + in + let* proposed = + List.fold_left_e add_proposal proposer_history.proposed proposals + in + return + { + count = proposer_history.count + proposals_length; + operations = oph :: proposer_history.operations; + proposed; + } + in + let* () = + match state.dictator_proposals_validated with + | None -> ok () + | Some dictator_oph -> error (Conflicting_dictator_proposals dictator_oph) + in + let proposals_validated = + Signature.Public_key_hash.Map.add + proposer + new_proposer_history + state.proposals_validated + in + return {state with proposals_validated} + + (** Check that a Proposals operation from a testnet dictator is + compatible with previously validated voting operations in the + current block/mempool (ie. that no other voting operation has + been validated), and update the [state] with this operation. + + @return [Error Testnet_dictator_conflicting_operation] if the + current block or mempool already contains any validated voting + operation. *) + let check_dictator_proposals_conflicts_and_update_state state oph = + let open Tzresult_syntax in + let* () = + error_unless + (Signature.Public_key_hash.Map.is_empty state.proposals_validated + && Option.is_none state.dictator_proposals_validated + && Signature.Public_key_hash.Map.is_empty state.ballots_validated) + Testnet_dictator_conflicting_operation + in + return {state with dictator_proposals_validated = Some oph} + + (** Check that a Ballot operation is compatible with previously + validated voting operations in the current block/mempool. + + @return [Error Conflicting_ballot] if the [delegate] has already + submitted a ballot in the current block/mempool. + + @return [Error Conflicting_dictator_proposals] if the current + block/mempool already contains a testnet dictator Proposals + operation (see {!recfield:dictator_proposals_validated}). *) + let check_ballot_conflicts state voter = + let open Tzresult_syntax in + let* () = + match + Signature.Public_key_hash.Map.find voter state.ballots_validated + with + | None -> ok () + | Some conflicting_operation -> + error (Conflicting_ballot {conflicting_operation}) + in + match state.dictator_proposals_validated with + | None -> ok () + | Some dictator_oph -> error (Conflicting_dictator_proposals dictator_oph) + + (** Update the [state] when a Ballot operation is validated. *) + let update_state_on_ballot state oph voter = + let ballots_validated = + Signature.Public_key_hash.Map.add voter oph state.ballots_validated + in + {state with ballots_validated} + + (** Check that [record_proposals] below will not fail. + + This function is designed to be exclusively called by + [validate_proposals] further down this file. + + @return [Error Multiple_proposals] if [proposals] has more than + one element. *) + let check_testnet_dictator_proposals chain_id proposals = + (* This assertion should be ensured by the fact that + {!is_testnet_dictator} cannot be [true] on mainnet, but we + double check it because it is critical. *) + assert (Chain_id.(chain_id <> Constants.mainnet_id)) ; + match proposals with + | [] | [_] -> + (* In [record_proposals] below, the call to + {!Vote.init_current_proposal} (in the singleton list case) + cannot fail because {!Vote.clear_current_proposal} is called + right before. + + The calls to + {!Voting_period.Testnet_dictator.overwrite_current_kind} may + usually fail when the voting period is not + initialized. However, this cannot happen because the current + function is only called in [validate_proposals] after a + successful call to {!Voting_period.get_current}. *) + ok () + | _ :: _ :: _ -> error Testnet_dictator_multiple_proposals + + let check_period_index ~expected period_index = + error_unless + Compare.Int32.(expected = period_index) + (Wrong_voting_period_index {expected; provided = period_index}) + + (** Check that the list of proposals is not empty and does not contain + duplicates. *) + let check_proposal_list_sanity proposals = + let open Tzresult_syntax in + let* () = + match proposals with [] -> error Empty_proposals | _ :: _ -> ok () + in + let* (_ : Protocol_hash.Set.t) = + List.fold_left_e + (fun previous_elements proposal -> + let* () = + error_when + (Protocol_hash.Set.mem proposal previous_elements) + (Proposals_contain_duplicate {proposal}) + in + return (Protocol_hash.Set.add proposal previous_elements)) + Protocol_hash.Set.empty + proposals + in + return_unit + + let check_period_kind_for_proposals current_period = + match current_period.Voting_period.kind with + | Proposal -> ok () + | (Exploration | Cooldown | Promotion | Adoption) as current -> + error (Wrong_voting_period_kind {current; expected = [Proposal]}) + + let check_in_listings ctxt source = + let open Lwt_tzresult_syntax in + let*! in_listings = Vote.in_listings ctxt source in + fail_unless in_listings Source_not_in_vote_listings + + let check_count ~count_in_ctxt ~proposals_length = + (* The proposal count of the proposer in the context should never + have been increased above [max_proposals_per_delegate]. *) + assert (Compare.Int.(count_in_ctxt <= Constants.max_proposals_per_delegate)) ; + error_unless + Compare.Int.( + count_in_ctxt + proposals_length <= Constants.max_proposals_per_delegate) + Too_many_proposals + + let check_already_proposed ctxt proposer proposals = + let open Lwt_tzresult_syntax in + List.iter_es + (fun proposal -> + let*! already_proposed = Vote.has_proposed ctxt proposer proposal in + fail_when already_proposed (Already_proposed {proposal})) + proposals + + let check_period_kind_for_ballot current_period = + match current_period.Voting_period.kind with + | Exploration | Promotion -> ok () + | (Cooldown | Proposal | Adoption) as current -> + error + (Wrong_voting_period_kind + {current; expected = [Exploration; Promotion]}) + + let check_current_proposal ctxt op_proposal = + let open Lwt_tzresult_syntax in + let* current_proposal = Vote.get_current_proposal ctxt in + fail_unless + (Protocol_hash.equal op_proposal current_proposal) + (Ballot_for_wrong_proposal + {current = current_proposal; submitted = op_proposal}) + + let check_source_has_not_already_voted ctxt source = + let open Lwt_tzresult_syntax in + let*! has_ballot = Vote.has_recorded_ballot ctxt source in + fail_when has_ballot Already_submitted_a_ballot + + (** Check that a Proposals operation can be safely applied. + + @return [Error Wrong_voting_period_index] if the operation's + period and the [context]'s current period do not have the same + index. + + @return [Error Empty_proposals] if the list of proposals is empty. + + @return [Error Proposals_contain_duplicate] if the list of + proposals contains a duplicate element. + + @return [Error Wrong_voting_period_kind] if the voting period is + not of the Proposal kind. + + @return [Error Source_not_in_vote_listings] if the source is not + in the vote listings. + + @return [Error Too_many_proposals] if the operation would make the + source's total number of proposals exceed + {!Constants.recorded_proposal_count_for_delegate}. + + @return [Error Already_proposed] if one of the proposals has + already been proposed by the source. + + @return [Error Conflict_too_many_proposals] if the total count of + proposals submitted by the source in previous blocks, in previously + validated operations of the current block/mempool, and in the + operation to validate, exceeds + {!Constants.max_proposals_per_delegate}. + + @return [Error Conflict_already_proposed] if one of the + operation's proposals has already been submitted by the source in + the current block/mempool. + + @return [Error Conflicting_dictator_proposals] if a testnet + dictator Proposals operation has already been validated in the + current block/mempool. + + @return [Error Testnet_dictator_multiple_proposals] if the source + is a testnet dictator and the operation contains more than one + proposal. + + @return [Error Testnet_dictator_conflicting_operation] if the + source is a testnet dictator and the current block or mempool + already contains any validated voting operation. + + @return [Error Operation.Missing_signature] or [Error + Operation.Invalid_signature] if the operation is unsigned or + incorrectly signed. *) + let validate_proposals vi vs ~should_check_signature oph + (operation : Kind.proposals operation) = + let open Lwt_tzresult_syntax in + let (Single (Proposals {source; period; proposals})) = + operation.protocol_data.contents + in + let* current_period = Voting_period.get_current vi.ctxt in + let*? () = check_period_index ~expected:current_period.index period in + let* voting_state = + if Amendment.is_testnet_dictator vi.ctxt vi.chain_id source then + let*? () = check_testnet_dictator_proposals vi.chain_id proposals in + Lwt.return + (check_dictator_proposals_conflicts_and_update_state + vs.voting_state + oph) + else + let*? () = check_proposal_list_sanity proposals in + let*? () = check_period_kind_for_proposals current_period in + let* () = check_in_listings vi.ctxt source in + let* count_in_ctxt = Vote.get_delegate_proposal_count vi.ctxt source in + let proposals_length = List.length proposals in + let*? () = check_count ~count_in_ctxt ~proposals_length in + let* () = check_already_proposed vi.ctxt source proposals in + Lwt.return + (check_proposals_conflicts_and_update_state + vs.voting_state + oph + source + proposals + ~count_in_ctxt + ~proposals_length) + in + (* The signature check is done last because it is more costly than + most checks. *) + let* () = + when_ should_check_signature (fun () -> + (* Retrieving the public key cannot fail. Indeed, we have + already checked that the delegate is in the vote listings + (or is a testnet dictator), which implies that it is a + manager with a revealed key. *) + let* public_key = Contract.get_manager_key vi.ctxt source in + Lwt.return + (Operation.check_signature public_key vi.chain_id operation)) + in + return {vs with voting_state} + + (** Check that a Ballot operation can be safely applied. + + @return [Error Conflicting_ballot] if the source has already + submitted a ballot in the current block/mempool. + + @return [Error Conflicting_dictator_proposals] if the current + block/mempool already contains a validated testnet dictator + Proposals operation. + + @return [Error Wrong_voting_period_index] if the operation's + period and the [context]'s current period do not have the same + index. + + @return [Error Wrong_voting_period_kind] if the voting period is + not of the Exploration or Promotion kind. + + @return [Error Ballot_for_wrong_proposal] if the operation's + proposal is different from the [context]'s current proposal. + + @return [Error Already_submitted_a_ballot] if the source has + already voted. + + @return [Error Source_not_in_vote_listings] if the source is not + in the vote listings. + + @return [Error Operation.Missing_signature] or [Error + Operation.Invalid_signature] if the operation is unsigned or + incorrectly signed. *) + let validate_ballot vi vs ~should_check_signature oph + (operation : Kind.ballot operation) = + let open Lwt_tzresult_syntax in + let (Single (Ballot {source; period; proposal; ballot = _})) = + operation.protocol_data.contents + in + let*? () = check_ballot_conflicts vs.voting_state source in + let* current_period = Voting_period.get_current vi.ctxt in + let*? () = check_period_index ~expected:current_period.index period in + let*? () = check_period_kind_for_ballot current_period in + let* () = check_current_proposal vi.ctxt proposal in + let* () = check_source_has_not_already_voted vi.ctxt source in + let* () = check_in_listings vi.ctxt source in + (* The signature check is done last because it is more costly than + most checks. *) + let* () = + when_ should_check_signature (fun () -> + (* Retrieving the public key cannot fail. Indeed, we have + already checked that the delegate is in the vote listings, + which implies that it is a manager with a revealed key. *) + let* public_key = Contract.get_manager_key vi.ctxt source in + Lwt.return + (Operation.check_signature public_key vi.chain_id operation)) + in + let voting_state = update_state_on_ballot vs.voting_state oph source in + return {vs with voting_state} +end + module Anonymous = struct open Validate_errors.Anonymous @@ -1489,7 +1947,7 @@ let begin_no_predecessor_info ctxt chain_id = let validate_operation (vi : validate_operation_info) (vs : validate_operation_state) ?(should_check_signature = true) oph (type kind) (operation : kind operation) = - let open Lwt_result_syntax in + let open Lwt_tzresult_syntax in let* vs = match operation.protocol_data.contents with | Single (Preendorsement _) -> @@ -1506,6 +1964,10 @@ let validate_operation (vi : validate_operation_info) vs ~should_check_signature operation + | Single (Proposals _) -> + Voting.validate_proposals vi vs ~should_check_signature oph operation + | Single (Ballot _) -> + Voting.validate_ballot vi vs ~should_check_signature oph operation | Single (Activate_account _ as contents) -> Anonymous.validate_activate_account vi vs oph contents | Single (Double_preendorsement_evidence _ as contents) -> @@ -1534,18 +1996,7 @@ let validate_operation (vi : validate_operation_info) source oph operation - | Single (Proposals _) | Single (Ballot _) | Single (Failing_noop _) -> - (* TODO: https://gitlab.com/tezos/tezos/-/issues/2603 - - There is no separate validation phase for non-manager - operations yet: all checks are currently done during - application in {!Apply}. - - When the validation of other operations is implemented, we - should also update - {!TMP_for_plugin.precheck_manager__do_nothing_on_non_manager_op} - (if has not been removed yet). *) - return vs + | Single (Failing_noop _) -> fail Validate_errors.Failing_noop_error in return (vs, Operation_validated_stamp) diff --git a/src/proto_alpha/lib_protocol/validate_operation.mli b/src/proto_alpha/lib_protocol/validate_operation.mli index 58bddc900af31ed2a57f8f562139a050ec918cc5..d4c61fb60aa9b17aa35b4bcd20d3e71d5f9ea78d 100644 --- a/src/proto_alpha/lib_protocol/validate_operation.mli +++ b/src/proto_alpha/lib_protocol/validate_operation.mli @@ -142,6 +142,11 @@ type stamp includes it being correctly signed: indeed, we can't take anything from a manager without having checked their signature. + For non-manager operations, any error during the operation + application causes the whole block to fail. Therefore, the + validation of such an operation must ensure that its application + will fully succeed. + @param should_check_signature indicates whether the signature check should happen. It defaults to [true] because the signature needs to be correct for the operation to be valid. This argument @@ -155,15 +160,7 @@ type stamp - The [run_operation] RPC provided by the plugin explicitly excludes signature checks: see its documentation in - [lib_plugin/RPC.Scripts.S.run_operation]. - - TODO: https://gitlab.com/tezos/tezos/-/issues/2603 - - This function currently does nothing for voting operations. - (instead, the validity of a voting operation is decided by calling - {!Apply.apply_operation} to check whether it returns an error). - We should specify and implement the validation of every kind of - operation. *) + [lib_plugin/RPC.Scripts.S.run_operation]. *) val validate_operation : validate_operation_info -> validate_operation_state -> diff --git a/src/proto_alpha/lib_protocol/vote_storage.ml b/src/proto_alpha/lib_protocol/vote_storage.ml index a37403dc565c4053e7999887fdbcc7093efba7d8..f2d110f8844b5d8ae978c42d7070784c7e1d3560 100644 --- a/src/proto_alpha/lib_protocol/vote_storage.ml +++ b/src/proto_alpha/lib_protocol/vote_storage.ml @@ -23,13 +23,17 @@ (* *) (*****************************************************************************) -let recorded_proposal_count_for_delegate ctxt proposer = +let get_delegate_proposal_count ctxt proposer = Storage.Vote.Proposals_count.find ctxt proposer >|=? Option.value ~default:0 -let record_proposal ctxt proposal proposer = - recorded_proposal_count_for_delegate ctxt proposer >>=? fun count -> - Storage.Vote.Proposals_count.add ctxt proposer (count + 1) >>= fun ctxt -> - Storage.Vote.Proposals.add ctxt (proposal, proposer) >|= ok +let set_delegate_proposal_count ctxt proposer count = + Storage.Vote.Proposals_count.add ctxt proposer count + +let has_proposed ctxt proposer proposal = + Storage.Vote.Proposals.mem ctxt (proposal, proposer) + +let add_proposal ctxt proposer proposal = + Storage.Vote.Proposals.add ctxt (proposal, proposer) let get_proposals ctxt = Storage.Vote.Proposals.fold @@ -248,14 +252,15 @@ let get_participation_ema = Storage.Vote.Participation_ema.get let set_participation_ema = Storage.Vote.Participation_ema.update +let current_proposal_exists = Storage.Vote.Current_proposal.mem + let get_current_proposal = Storage.Vote.Current_proposal.get let find_current_proposal = Storage.Vote.Current_proposal.find let init_current_proposal = Storage.Vote.Current_proposal.init -let clear_current_proposal ctxt = - Storage.Vote.Current_proposal.remove ctxt >|= ok +let clear_current_proposal = Storage.Vote.Current_proposal.remove let init ctxt ~start_position = (* participation EMA is in centile of a percentage *) diff --git a/src/proto_alpha/lib_protocol/vote_storage.mli b/src/proto_alpha/lib_protocol/vote_storage.mli index b814c8a54d9afc64a43f307e1db4c2ddc9a90509..69d0619f781ab0f0203cf160b5190e15f1ac54d3 100644 --- a/src/proto_alpha/lib_protocol/vote_storage.mli +++ b/src/proto_alpha/lib_protocol/vote_storage.mli @@ -26,15 +26,39 @@ (** Manages all the voting related storage in Storage.Vote. *) -(** Records a protocol proposal with the delegate that proposed it. *) -val record_proposal : +(** [get_delegate_proposal_count ctxt proposer] returns the number of + proposals already made by [proposer] in the current voting cycle. + + This number of proposals, aka [count], has its own storage bucket. + + @return [0] if the [count] of the proposer was not initialized. + + @return [Error Storage_error] if the deserialization of [count] + fails. *) +val get_delegate_proposal_count : + Raw_context.t -> Signature.public_key_hash -> int tzresult Lwt.t + +(** [set_delegate_proposal_count ctxt proposer count] sets + [proposer]'s number of submitted proposals to [count]. + + More precisely, the relevant storage bucket is allocated and + initialized to [count] if it didn't exist; otherwise it is simply + updated. *) +val set_delegate_proposal_count : + Raw_context.t -> Signature.public_key_hash -> int -> Raw_context.t Lwt.t + +(** [has_proposed ctxt proposer proposal] indicates whether the + [proposer] has already proposed the [proposal]. *) +val has_proposed : + Raw_context.t -> Signature.public_key_hash -> Protocol_hash.t -> bool Lwt.t + +(** [add_proposal ctxt proposer proposal] records the submission of + [proposal] by [proposer]. *) +val add_proposal : Raw_context.t -> + Signature.public_key_hash -> Protocol_hash.t -> - Signature.Public_key_hash.t -> - Raw_context.t tzresult Lwt.t - -val recorded_proposal_count_for_delegate : - Raw_context.t -> Signature.Public_key_hash.t -> int tzresult Lwt.t + Raw_context.t Lwt.t (** Computes for each proposal how many delegates proposed it. *) val get_proposals : Raw_context.t -> int64 Protocol_hash.Map.t tzresult Lwt.t @@ -127,15 +151,33 @@ val get_participation_ema : Raw_context.t -> int32 tzresult Lwt.t val set_participation_ema : Raw_context.t -> int32 -> Raw_context.t tzresult Lwt.t +(** Indicates whether there is a current proposal in the storage. *) +val current_proposal_exists : Raw_context.t -> bool Lwt.t + +(** Retrieves the current proposal. + + @return [Error Storage_error] if there is no current proposal, or + if the deserialization fails. *) val get_current_proposal : Raw_context.t -> Protocol_hash.t tzresult Lwt.t +(** Retrieves the current proposal. + + @return [None] if there is no current proposal. + + @return [Error Storage_error] if the deserialization fails. *) val find_current_proposal : Raw_context.t -> Protocol_hash.t option tzresult Lwt.t +(** Registers a current proposal. + + @return [Error (Storage_error Existing_key)] if there was already + a current proposal. *) val init_current_proposal : Raw_context.t -> Protocol_hash.t -> Raw_context.t tzresult Lwt.t -val clear_current_proposal : Raw_context.t -> Raw_context.t tzresult Lwt.t +(** Removes the current proposal. Does nothing if there was no current + proposal. *) +val clear_current_proposal : Raw_context.t -> Raw_context.t Lwt.t (** Sets the initial quorum to 80% and period kind to proposal. *) val init : diff --git a/src/proto_alpha/lib_protocol/voting_period_storage.ml b/src/proto_alpha/lib_protocol/voting_period_storage.ml index b15dd523009674747ccdf749920b91787538d063..cf080ff404d850b0a06bc591fc63bf4993b692f2 100644 --- a/src/proto_alpha/lib_protocol/voting_period_storage.ml +++ b/src/proto_alpha/lib_protocol/voting_period_storage.ml @@ -194,6 +194,9 @@ let get_rpc_succ_info ctxt = Voting_period_repr.{voting_period; position; remaining} module Testnet_dictator = struct + (* This error must never happen. It is deliberately unregistered so + that the execution fails loudly if [overwrite_current_kind] is + ever called on mainnet. *) type error += Forbidden_on_mainnet let overwrite_current_kind ctxt chain_id kind = diff --git a/src/proto_alpha/lib_protocol/voting_period_storage.mli b/src/proto_alpha/lib_protocol/voting_period_storage.mli index 06a6997aa1dbb2dcaf8e9579fee61a391cdfa3ee..2d8a4ea2d8a1b6cc28c2b57ad6ff49d5b728eba7 100644 --- a/src/proto_alpha/lib_protocol/voting_period_storage.mli +++ b/src/proto_alpha/lib_protocol/voting_period_storage.mli @@ -61,8 +61,13 @@ val get_rpc_current_info : val get_rpc_succ_info : Raw_context.t -> Voting_period_repr.info tzresult Lwt.t module Testnet_dictator : sig - (** Overwrites the kind of the current voting period WITHOUT incrementing the index. - Must ONLY be called by the testnet dictator on a testnet. *) + (** Overwrites the kind of the current voting period WITHOUT + incrementing the index. + + Must ONLY be called by the testnet dictator on a testnet. + + @return [Error Storage_error] if the current voting period is + not set or its deserialization fails. *) val overwrite_current_kind : Raw_context.t -> Chain_id.t -> diff --git a/tests_python/tests_alpha/test_basic.py b/tests_python/tests_alpha/test_basic.py index f40471aa5bfd98776a85889c8f153d2d5399e5e1..54bdf0887ad60dc2202616a8af6ee96a32b3cf23 100644 --- a/tests_python/tests_alpha/test_basic.py +++ b/tests_python/tests_alpha/test_basic.py @@ -152,7 +152,7 @@ class TestRawContext: '/chains/main/blocks/head/helpers/scripts/run_operation' ) with assert_run_failure( - 'The failing_noop operation cannot be executed by the protocol' + 'A failing_noop operation can never be validated' ): client.rpc('post', run_operation_path, data=run_json) diff --git a/tezt/lib_tezos/operation_core.ml b/tezt/lib_tezos/operation_core.ml index 528b838d2a09f31a5bcd38dc72d61f8ea75e7213..13a17cfa2f3a810bf98e7253f232ec0c46f4c004 100644 --- a/tezt/lib_tezos/operation_core.ml +++ b/tezt/lib_tezos/operation_core.ml @@ -149,7 +149,7 @@ let inject_operations ?(request = `Inject) ?(force = false) ?error t client : let* () = Process.check_error ~msg process in Lwt_list.map_s (fun op -> hash op client) t -let make_run_operation_input ?chain_id ?(sign_correctly = false) t client = +let make_run_operation_input ?chain_id t client = let* chain_id = match chain_id with | Some chain_id -> return chain_id @@ -157,11 +157,6 @@ let make_run_operation_input ?chain_id ?(sign_correctly = false) t client = in (* The [run_operation] RPC does not check the signature. *) let signature = Tezos_crypto.Signature.zero in - (* ...except that it is currently bugged, and it checks the - signature of non-manager operations - (https://gitlab.com/tezos/tezos/-/issues/3401). So we temporarily - provide the option to compute a correct signature. *) - let* signature = if sign_correctly then sign t client else return signature in return (`O [ diff --git a/tezt/lib_tezos/operation_core.mli b/tezt/lib_tezos/operation_core.mli index fc90f67e9c26ddf6077cad04f20d0a10e74e4365..3db2246e0e846a43b8aca0d27177bb931998bcb7 100644 --- a/tezt/lib_tezos/operation_core.mli +++ b/tezt/lib_tezos/operation_core.mli @@ -144,29 +144,13 @@ val inject_operations : The operation is signed with {!Tezos_crypto.Signature.zero}, because the [run_operation] RPC skips signature checks anyway. - @param sign_correctly If [true], use the correct signature of the - operation instead of [Signature.zero]. Defaults to [false]. This - parameter is temporary until the [run_operation] RPC is fixed to - actually skip signature checks for non-manager operations (see - https://gitlab.com/tezos/tezos/-/issues/3401) - @param chain_id Allows to manually provide the [chain_id]. If omitted, the [chain_id] is retrieved via RPC using the provided [client]. @param client The {!Client.t} argument is used to retrieve the [chain_id] when it is not provided. *) -val make_run_operation_input : - ?chain_id:string -> - (* FIXME: https://gitlab.com/tezos/tezos/-/issues/3401 - - Remove the [sign_correctly] argument once the [run_operation] RPC - is fixed to actually skip signature checks for non-manager - operations. *) - ?sign_correctly:bool -> - t -> - Client.t -> - JSON.u Lwt.t +val make_run_operation_input : ?chain_id:string -> t -> Client.t -> JSON.u Lwt.t module Consensus : sig (** A representation of a consensus operation. *) diff --git a/tezt/tests/run_operation_RPC.ml b/tezt/tests/run_operation_RPC.ml index fd8c8c0db394999f442f864daa73d6f11d654ffd..1e81839bf257029771d3848d69a5c8a11624083a 100644 --- a/tezt/tests/run_operation_RPC.ml +++ b/tezt/tests/run_operation_RPC.ml @@ -131,9 +131,7 @@ let test_run_proposals = [Protocol.demo_counter_hash] in let* op = Operation.Voting.operation ~client proposals_repr in - let* op_json = - Operation.make_run_operation_input ~sign_correctly:true op client - in + let* op_json = Operation.make_run_operation_input op client in Log.info "%s" (Ezjsonm.value_to_string ~minify:false op_json) ; Log.info "Call the [run_operation] RPC on this operation." ; let* _output =