From 40472f456141508b8e35d8e664f167d161dde457 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Wed, 23 Mar 2022 20:46:15 +0100 Subject: [PATCH 01/13] Proto/Tests/Helpers: allow revelation helper to force gas limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the signature of the [revelation] test helper to craft reveal operations with arbitray gas limits (including 0). Co-authored-by: Germán Delbianco Co-authored-by: vbot --- .../lib_protocol/test/helpers/op.ml | 5 +++-- .../lib_protocol/test/helpers/op.mli | 20 +++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.ml b/src/proto_alpha/lib_protocol/test/helpers/op.ml index 4e02fb0c3780..ce521bd8acd2 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/op.ml @@ -277,7 +277,8 @@ let manager_operation ?counter ?(fee = Tez.zero) ?gas_limit ?storage_limit in Contents_list (Cons (op_reveal, Single op)) -let revelation ?(fee = Tez.zero) ctxt public_key = +let revelation ?(fee = Tez.zero) + ?(gas_limit = Gas.Arith.integral_of_int_exn 10000) ctxt public_key = let pkh = Signature.Public_key.hash public_key in let source = Contract.Implicit pkh in Context.Contract.counter ctxt source >>=? fun counter -> @@ -292,7 +293,7 @@ let revelation ?(fee = Tez.zero) ctxt public_key = fee; counter; operation = Reveal public_key; - gas_limit = Gas.Arith.integral_of_int_exn 10000; + gas_limit; storage_limit = Z.zero; })) in diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.mli b/src/proto_alpha/lib_protocol/test/helpers/op.mli index 1888cb9fd424..c3463f0c2a91 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/op.mli @@ -83,8 +83,24 @@ val set_deposits_limit : Tez.tez option -> Operation.packed tzresult Lwt.t +(** [revelation ?fee ?gas_limit ctxt pkh] Creates a new [Reveal] + {!manager_operation} to reveal a public key [pkh] applying to + current context [ctxt]. + + Optional arguments allow to override defaults: + + {ul {li [?fee:Tez.tez]: specify a fee, otherwise set to + [Tez.zero].} + + {li [?gas_limit:Gas.Arith.integral]: force a gas limit, + otherwise set to 10000 gas units.}} +*) val revelation : - ?fee:Tez.tez -> Context.t -> public_key -> Operation.packed tzresult Lwt.t + ?fee:Tez.tez -> + ?gas_limit:Gas.Arith.integral -> + Context.t -> + public_key -> + Operation.packed tzresult Lwt.t val failing_noop : Context.t -> public_key_hash -> string -> Operation.packed tzresult Lwt.t @@ -92,7 +108,7 @@ val failing_noop : (** [contract_origination ctxt source] Create a new contract origination operation, sign it with [source] and returns it alongside the contract address. The contract address is using the initial origination nonce with the - hash of the operation. If this operation is combine with [combine_operations] + hash of the operation. If this operation is combined with [combine_operations] then the contract address is false as the nonce is not based on the correct operation hash. *) val contract_origination : -- GitLab From 2d2d031a821d0c46f087797cb7860e132bba9a7e Mon Sep 17 00:00:00 2001 From: vbot Date: Mon, 28 Mar 2022 15:07:57 +0200 Subject: [PATCH 02/13] Proto/Tests/Helpers: reverse failure handlers in add_operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We invert [?expect_apply_failure?] and [?expect_failure] semantics in [add_operation] and propagate accordingly to all protocol integration tests. Co-authored-by: Germán Delbianco Co-authored-by: vbot --- .../lib_protocol/test/helpers/incremental.ml | 6 +- .../lib_protocol/test/helpers/incremental.mli | 2 +- .../lib_protocol/test/helpers/transfers.ml | 4 +- .../lib_protocol/test/helpers/transfers.mli | 2 +- .../integration/consensus/test_delegation.ml | 35 +++-- .../consensus/test_frozen_deposits.ml | 4 +- .../integration/michelson/test_sapling.ml | 4 +- .../michelson/test_ticket_accounting.ml | 2 +- .../michelson/test_ticket_operations_diff.ml | 2 +- .../operations/test_combined_operations.ml | 20 +-- .../integration/operations/test_reveal.ml | 4 +- .../integration/operations/test_sc_rollup.ml | 27 ++-- .../integration/operations/test_transfer.ml | 23 +-- .../integration/operations/test_tx_rollup.ml | 145 +++++++++--------- 14 files changed, 140 insertions(+), 140 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/incremental.ml b/src/proto_alpha/lib_protocol/test/helpers/incremental.ml index 5daa83bf2b01..3551de4e92c9 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/incremental.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/incremental.ml @@ -156,7 +156,7 @@ let detect_script_failure : in fun {contents} -> detect_script_failure contents -let add_operation ?expect_apply_failure ?expect_failure ?(check_size = true) st +let add_operation ?expect_failure ?expect_apply_failure ?(check_size = true) st op = let open Apply_results in (if check_size then @@ -169,7 +169,7 @@ let add_operation ?expect_apply_failure ?expect_failure ?(check_size = true) st operation_size Constants_repr.max_operation_data_length))) ; apply_operation st.state op >|= Environment.wrap_tzresult >>= fun result -> - match (expect_apply_failure, result) with + match (expect_failure, result) with | Some _, Ok _ -> failwith "Error expected while adding operation" | Some f, Error err -> f err >|=? fun () -> st | None, result -> ( @@ -177,7 +177,7 @@ let add_operation ?expect_apply_failure ?expect_failure ?(check_size = true) st match result with | state, (Operation_metadata result as metadata) -> detect_script_failure result |> fun result -> - (match expect_failure with + (match expect_apply_failure with | None -> Lwt.return result | Some f -> ( match result with diff --git a/src/proto_alpha/lib_protocol/test/helpers/incremental.mli b/src/proto_alpha/lib_protocol/test/helpers/incremental.mli index fb04df180674..f6dab21cb043 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/incremental.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/incremental.mli @@ -49,8 +49,8 @@ val begin_construction : incremental tzresult Lwt.t val add_operation : - ?expect_apply_failure:(error list -> unit tzresult Lwt.t) -> ?expect_failure:(error list -> unit tzresult Lwt.t) -> + ?expect_apply_failure:(error list -> unit tzresult Lwt.t) -> ?check_size:bool -> incremental -> Operation.packed -> diff --git a/src/proto_alpha/lib_protocol/test/helpers/transfers.ml b/src/proto_alpha/lib_protocol/test/helpers/transfers.ml index 78557fb56849..ac185cd069d3 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/transfers.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/transfers.ml @@ -28,7 +28,7 @@ open Alpha_context open Test_tez let transfer_and_check_balances ?(with_burn = false) ~loc b ?(fee = Tez.zero) - ?expect_failure src dst amount = + ?expect_apply_failure src dst amount = let open Lwt_result_syntax in let*? amount_fee = fee +? amount in let* bal_src = Context.Contract.balance (I b) src in @@ -42,7 +42,7 @@ let transfer_and_check_balances ?(with_burn = false) ~loc b ?(fee = Tez.zero) dst amount in - let* b = Incremental.add_operation ?expect_failure b op in + let* b = Incremental.add_operation ?expect_apply_failure b op in let* {parametric = {origination_size; cost_per_byte; _}; _} = Context.get_constants (I b) in diff --git a/src/proto_alpha/lib_protocol/test/helpers/transfers.mli b/src/proto_alpha/lib_protocol/test/helpers/transfers.mli index 3a3526fbaa65..86e17da6e2e4 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/transfers.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/transfers.mli @@ -45,7 +45,7 @@ val transfer_and_check_balances : loc:string -> Incremental.t -> ?fee:Tez.t -> - ?expect_failure:(error trace -> unit tzresult Lwt.t) -> + ?expect_apply_failure:(error trace -> unit tzresult Lwt.t) -> Contract.t -> Contract.t -> Tez.t -> diff --git a/src/proto_alpha/lib_protocol/test/integration/consensus/test_delegation.ml b/src/proto_alpha/lib_protocol/test/integration/consensus/test_delegation.ml index 41299b62d68a..086678fa2c54 100644 --- a/src/proto_alpha/lib_protocol/test/integration/consensus/test_delegation.ml +++ b/src/proto_alpha/lib_protocol/test/integration/consensus/test_delegation.ml @@ -91,7 +91,7 @@ let bootstrap_delegate_cannot_change ~fee () = if fee > balance0 then expect_too_low_balance_error i set_delegate else Incremental.add_operation - ~expect_failure:(expect_no_change_registered_delegate_pkh delegate0) + ~expect_apply_failure:(expect_no_change_registered_delegate_pkh delegate0) i set_delegate >>=? fun i -> @@ -114,7 +114,8 @@ let bootstrap_delegate_cannot_be_removed ~fee () = if fee > balance then expect_too_low_balance_error i set_delegate else Incremental.add_operation - ~expect_failure:(expect_no_change_registered_delegate_pkh manager.pkh) + ~expect_apply_failure: + (expect_no_change_registered_delegate_pkh manager.pkh) i set_delegate >>=? fun i -> @@ -202,7 +203,7 @@ let bootstrap_manager_already_registered_delegate ~fee () = if fee > balance then expect_too_low_balance_error i sec_reg else Incremental.add_operation - ~expect_failure:(function + ~expect_apply_failure:(function | Environment.Ecoproto_error Delegate_storage.Active_delegate :: _ -> return_unit | _ -> failwith "Delegate is already active and operation should fail.") @@ -236,7 +237,7 @@ let delegate_to_bootstrap_by_origination ~fee () = if fee > balance then expect_too_low_balance_error i op else if total_fee > balance && balance >= fee then (* origination did not proceed; fee has been debited *) - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error err :: _ -> Assert.test_error_encodings err ; let error_info = @@ -249,7 +250,7 @@ let delegate_to_bootstrap_by_origination ~fee () = "Test_delegation.delegate_to_bootstrap_by_origination was expected \ to fail but has not" in - Incremental.add_operation i ~expect_failure op >>=? fun i -> + Incremental.add_operation i ~expect_apply_failure op >>=? fun i -> (* fee was taken *) Assert.balance_was_debited ~loc:__LOC__ (I i) bootstrap balance fee >>=? fun () -> @@ -477,7 +478,7 @@ let test_unregistered_delegate_key_init_origination ~fee () = else (* origination did not proceed; fee has been debited *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key unregistered_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_pkh) i op >>=? fun i -> @@ -515,7 +516,7 @@ let test_unregistered_delegate_key_init_delegation ~fee () = (* fee has been debited; no delegate *) Incremental.add_operation i - ~expect_failure:(expect_unregistered_key unregistered_delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_delegate_pkh) delegate_op >>=? fun i -> Assert.balance_was_debited ~loc:__LOC__ (I i) impl_contract credit fee @@ -559,7 +560,7 @@ let test_unregistered_delegate_key_switch_delegation ~fee () = (* fee has been debited; no delegate *) Incremental.add_operation i - ~expect_failure:(expect_unregistered_key unregistered_delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_delegate_pkh) delegate_op >>=? fun i -> Assert.balance_was_debited ~loc:__LOC__ (I i) impl_contract credit fee @@ -594,7 +595,7 @@ let test_unregistered_delegate_key_init_origination_credit ~fee ~amount () = else (* origination not done, fee taken *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key unregistered_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_pkh) i op >>=? fun i -> @@ -634,7 +635,7 @@ let test_unregistered_delegate_key_init_delegation_credit ~fee ~amount () = else (* fee has been taken, no delegate for contract *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key unregistered_delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_delegate_pkh) i delegate_op >>=? fun i -> @@ -681,7 +682,7 @@ let test_unregistered_delegate_key_switch_delegation_credit ~fee ~amount () = else (* fee has been taken, delegate for contract has not changed *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key unregistered_delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_delegate_pkh) i delegate_op >>=? fun i -> @@ -722,7 +723,7 @@ let test_unregistered_delegate_key_init_origination_credit_debit ~fee ~amount () else (* fee taken, origination not processed *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key unregistered_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_pkh) i op >>=? fun i -> @@ -767,7 +768,7 @@ let test_unregistered_delegate_key_init_delegation_credit_debit ~amount ~fee () else (* fee has been taken, no delegate for contract *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key unregistered_delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_delegate_pkh) i delegate_op >>=? fun i -> @@ -818,7 +819,7 @@ let test_unregistered_delegate_key_switch_delegation_credit_debit ~fee ~amount else (* fee has been taken, delegate for contract has not changed *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key unregistered_delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key unregistered_delegate_pkh) i delegate_op >>=? fun i -> @@ -1186,7 +1187,7 @@ let test_unregistered_and_unrevealed_self_delegate_key_init_delegation ~fee () = else (* origination did not proceed; fee has been debited *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key delegate_pkh) i op >>=? fun i -> @@ -1209,7 +1210,7 @@ let test_unregistered_and_revealed_self_delegate_key_init_delegation ~fee () = else (* origination did not proceed; fee has been debited *) Incremental.add_operation - ~expect_failure:(expect_unregistered_key delegate_pkh) + ~expect_apply_failure:(expect_unregistered_key delegate_pkh) i op >>=? fun i -> @@ -1234,7 +1235,7 @@ let test_self_delegation_emptying_contract () = >>=? fun () -> (* The delegation operation should be applied and the fees debited but it is expected to fail in the apply-part. *) - Incremental.add_operation ~expect_failure:(fun _ -> return_unit) i op + Incremental.add_operation ~expect_apply_failure:(fun _ -> return_unit) i op >>=? fun i -> Context.Contract.is_manager_key_revealed (I i) contract >>=? function | false -> return_unit diff --git a/src/proto_alpha/lib_protocol/test/integration/consensus/test_frozen_deposits.ml b/src/proto_alpha/lib_protocol/test/integration/consensus/test_frozen_deposits.ml index e09c1b05b9d7..8b9e7b9b4769 100644 --- a/src/proto_alpha/lib_protocol/test/integration/consensus/test_frozen_deposits.ml +++ b/src/proto_alpha/lib_protocol/test/integration/consensus/test_frozen_deposits.ml @@ -197,7 +197,7 @@ let test_set_too_high_limit () = Int64.(div max_int 100L)) 1L) in - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error err :: _ -> Assert.test_error_encodings err ; let error_info = @@ -211,7 +211,7 @@ let test_set_too_high_limit () = Incremental.begin_construction genesis >>=? fun b -> Op.set_deposits_limit (B genesis) contract1 (Some max_limit) >>=? fun operation -> - Incremental.add_operation ~expect_failure b operation >>=? fun b -> + Incremental.add_operation ~expect_apply_failure b operation >>=? fun b -> Incremental.finalize_block b >>=? fun _ -> return_unit let test_unset_limit () = diff --git a/src/proto_alpha/lib_protocol/test/integration/michelson/test_sapling.ml b/src/proto_alpha/lib_protocol/test/integration/michelson/test_sapling.ml index da0e8d4b28b5..c6d9669667fe 100644 --- a/src/proto_alpha/lib_protocol/test/integration/michelson/test_sapling.ml +++ b/src/proto_alpha/lib_protocol/test/integration/michelson/test_sapling.ml @@ -729,7 +729,7 @@ module Interpreter_tests = struct Op.transaction ~fee (B b) src0 dst Tez.zero ~parameters >>=? fun operation -> Incremental.add_operation (* TODO make more precise *) - ~expect_failure:(fun _ -> return_unit) + ~expect_apply_failure:(fun _ -> return_unit) incr operation >>=? fun _incr -> @@ -764,7 +764,7 @@ module Interpreter_tests = struct Op.transaction ~fee (B b) src0 dst Tez.zero ~parameters >>=? fun operation -> Incremental.add_operation (* TODO make more precise *) - ~expect_failure:(fun _ -> return_unit) + ~expect_apply_failure:(fun _ -> return_unit) incr operation >>=? fun _incr -> return_unit diff --git a/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_accounting.ml b/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_accounting.ml index 214479f06e71..2cd5972b5c07 100644 --- a/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_accounting.ml +++ b/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_accounting.ml @@ -315,7 +315,7 @@ let originate_script block ~script ~storage ~src ~baker ~forges_tickets = in let* incr = Incremental.add_operation - ?expect_failure: + ?expect_apply_failure: (if forges_tickets then Some (fun _ -> return ()) else None) incr operation diff --git a/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_operations_diff.ml b/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_operations_diff.ml index 92117269d402..c38be69b0ca1 100644 --- a/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_operations_diff.ml +++ b/src/proto_alpha/lib_protocol/test/integration/michelson/test_ticket_operations_diff.ml @@ -199,7 +199,7 @@ let originate block ~script ~storage ~src ~baker ~forges_tickets = in let* incr = Incremental.add_operation - ?expect_failure: + ?expect_apply_failure: (if forges_tickets then Some (fun _ -> return ()) else None) incr operation diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_combined_operations.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_combined_operations.ml index da6145768a97..b36db3359d24 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_combined_operations.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_combined_operations.ml @@ -154,7 +154,7 @@ let test_multiple_origination_and_delegation () = Assert.balance_is ~loc:__LOC__ (I inc) c (Test_tez.of_int 10)) new_contracts -let expect_failure = function +let expect_apply_failure = function | Environment.Ecoproto_error err :: _ -> Assert.test_error_encodings err ; let error_info = @@ -178,7 +178,7 @@ let test_failing_operation_in_the_middle () = Incremental.begin_construction blk >>=? fun inc -> Context.Contract.balance (I inc) c1 >>=? fun c1_old_balance -> Context.Contract.balance (I inc) c2 >>=? fun c2_old_balance -> - Incremental.add_operation ~expect_failure inc operation >>=? fun inc -> + Incremental.add_operation ~expect_apply_failure inc operation >>=? fun inc -> let tickets = Incremental.rev_tickets inc in let open Apply_results in let tickets = @@ -223,7 +223,7 @@ let test_failing_operation_in_the_middle_with_fees () = Incremental.begin_construction blk >>=? fun inc -> Context.Contract.balance (I inc) c1 >>=? fun c1_old_balance -> Context.Contract.balance (I inc) c2 >>=? fun c2_old_balance -> - Incremental.add_operation ~expect_failure inc operation >>=? fun inc -> + Incremental.add_operation ~expect_apply_failure inc operation >>=? fun inc -> let tickets = Incremental.rev_tickets inc in let open Apply_results in let tickets = @@ -286,7 +286,7 @@ let test_wrong_signature_in_the_middle () = let operations = [op1; op2; op3] in Op.combine_operations ~spurious_operation ~source:c1 (I inc) operations >>=? fun operation -> - let expect_apply_failure = function + let expect_failure = function | Environment.Ecoproto_error err :: _ -> Assert.test_error_encodings err ; let error_info = @@ -300,7 +300,7 @@ let test_wrong_signature_in_the_middle () = "Packed operation has invalid source in the middle : operation \ expected to fail." in - Incremental.add_operation ~expect_apply_failure inc operation >>=? fun _inc -> + Incremental.add_operation ~expect_failure inc operation >>=? fun _inc -> return_unit let expect_inconsistent_counters list = @@ -370,17 +370,11 @@ let test_inconsistent_counters () = Incremental.add_operation inc op >>=? fun _ -> (* Gap in counter in the following op *) Op.batch_operations ~source:c1 (I inc) [op1; op2; op4] >>=? fun op -> - Incremental.add_operation - ~expect_apply_failure:expect_inconsistent_counters - inc - op + Incremental.add_operation ~expect_failure:expect_inconsistent_counters inc op >>=? fun _ -> (* Same counter used twice in the following op *) Op.batch_operations ~source:c1 (I inc) [op1; op2; op2'] >>=? fun op -> - Incremental.add_operation - ~expect_apply_failure:expect_inconsistent_counters - inc - op + Incremental.add_operation ~expect_failure:expect_inconsistent_counters inc op >>=? fun _ -> return_unit let tests = diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml index b81f17252142..992b91392b2f 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml @@ -109,9 +109,7 @@ let test_transfer_fees_emptying_after_reveal_batched () = >>=? fun transaction -> Op.batch_operations ~source:new_contract (I inc) [reveal; transaction] >>=? fun op -> - (* This operation is expected to fail at application time, not - during validation. *) - Incremental.add_operation ~expect_failure:(fun _ -> return_unit) inc op + Incremental.add_operation ~expect_apply_failure:(fun _ -> return_unit) inc op >>=? fun _inc -> return_unit let tests = diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml index 17e33c1d34d8..5cdfc49bccdc 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_sc_rollup.ml @@ -90,14 +90,13 @@ let test_disable_feature_flag () = let parameters_ty = Script.lazy_expr @@ Expr.from_string "unit" in Op.sc_rollup_origination (I i) contract kind "" parameters_ty in - - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error (Apply.Sc_rollup_feature_disabled as e) :: _ -> Assert.test_error_encodings e ; return_unit | _ -> failwith "It should have failed with [Sc_rollup_feature_disabled]" in - let*! _ = Incremental.add_operation ~expect_failure i op in + let*! _ = Incremental.add_operation ~expect_apply_failure i op in return_unit (** [test_sc_rollups_all_well_defined] checks that [Sc_rollups.all] @@ -228,7 +227,7 @@ let test_publish_fails_on_backtrack () = let* b = Incremental.finalize_block i in let* operation2 = Op.sc_rollup_publish (B b) contract rollup commitment2 in let* i = Incremental.begin_construction b in - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error (Sc_rollup_errors.Sc_rollup_staker_backtracked as e) :: _ -> @@ -236,7 +235,7 @@ let test_publish_fails_on_backtrack () = return_unit | _ -> failwith "It should have failed with [Sc_rollup_staker_backtracked]" in - let* _ = Incremental.add_operation ~expect_failure i operation2 in + let* _ = Incremental.add_operation ~expect_apply_failure i operation2 in return_unit (** [test_cement_fails_on_conflict] creates a rollup and then publishes @@ -267,18 +266,18 @@ let test_cement_fails_on_conflict () = let* i = Incremental.begin_construction b in let hash = Sc_rollup.Commitment.hash commitment1 in let* cement_op = Op.sc_rollup_cement (I i) contract1 rollup hash in - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error (Sc_rollup_errors.Sc_rollup_disputed as e) :: _ -> Assert.test_error_encodings e ; return_unit | _ -> failwith "It should have failed with [Sc_rollup_disputed]" in - - let* _ = Incremental.add_operation ~expect_failure i cement_op in + let* _ = Incremental.add_operation ~expect_apply_failure i cement_op in return_unit -let commit_and_cement_after_n_bloc ?expect_failure ctxt contract rollup n = +let commit_and_cement_after_n_bloc ?expect_apply_failure ctxt contract rollup n + = let* i = Incremental.begin_construction ctxt in let* commitment = dummy_commitment i rollup in let* operation = Op.sc_rollup_publish (B ctxt) contract rollup commitment in @@ -289,7 +288,7 @@ let commit_and_cement_after_n_bloc ?expect_failure ctxt contract rollup n = let* i = Incremental.begin_construction b in let hash = Sc_rollup.Commitment.hash commitment in let* cement_op = Op.sc_rollup_cement (I i) contract rollup hash in - let* _ = Incremental.add_operation ?expect_failure i cement_op in + let* _ = Incremental.add_operation ?expect_apply_failure i cement_op in return_unit (** [test_challenge_window_period_boundaries] checks that cementing a commitment @@ -303,7 +302,7 @@ let test_challenge_window_period_boundaries () = (* Should fail because the waiting period is not strictly greater than the challenge window period. *) let* () = - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error (Sc_rollup_errors.Sc_rollup_too_recent as e) :: _ -> Assert.test_error_encodings e ; @@ -311,7 +310,7 @@ let test_challenge_window_period_boundaries () = | _ -> failwith "It should have failed with [Sc_rollup_too_recent]" in commit_and_cement_after_n_bloc - ~expect_failure + ~expect_apply_failure ctxt contract rollup @@ -429,7 +428,7 @@ let test_atomic_batch_fails () = ~inclusion_proof:"xyz" ~atomic_transaction_batch:"xyz" in - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error (Sc_rollup_operations.Sc_rollup_invalid_atomic_batch as e) :: _ -> @@ -437,7 +436,7 @@ let test_atomic_batch_fails () = return_unit | _ -> failwith "For some reason in did not fail with the right error" in - let* _ = Incremental.add_operation ~expect_failure i batch_op in + let* _ = Incremental.add_operation ~expect_apply_failure i batch_op in return_unit diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml index 6a263f9ef124..1930ea3636e7 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml @@ -78,13 +78,13 @@ let two_over_n_of_balance incr contract n = (********************) -let single_transfer ?fee ?expect_failure amount = +let single_transfer ?fee ?expect_apply_failure amount = Context.init2 () >>=? fun (b, (contract_1, contract_2)) -> Incremental.begin_construction b >>=? fun b -> transfer_and_check_balances ~loc:__LOC__ ?fee - ?expect_failure + ?expect_apply_failure b contract_1 contract_2 @@ -101,7 +101,7 @@ let test_block_with_a_single_transfer_with_fee () = (** Single transfer without fee. *) let test_transfer_zero_tez () = - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error err :: _ -> Assert.test_error_encodings err ; let error_info = @@ -111,7 +111,7 @@ let test_transfer_zero_tez () = else failwith "unexpected error" | _ -> failwith "Empty transaction should fail" in - single_transfer ~expect_failure Tez.zero + single_transfer ~expect_apply_failure Tez.zero (** Transfer zero tez from an implicit contract. *) let test_transfer_zero_implicit () = @@ -383,7 +383,7 @@ let test_balance_too_low fee () = Context.Contract.balance (I i) contract_2 >>=? fun balance2 -> (* transfer the amount of tez that is bigger than the balance in the source contract *) Op.transaction ~fee (I i) contract_1 contract_2 max_tez >>=? fun op -> - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error err :: _ -> Assert.test_error_encodings err ; let error_info = @@ -395,11 +395,12 @@ let test_balance_too_low fee () = in (* the fee is higher than the balance then raise an error "Balance_too_low" *) if fee > balance1 then - Incremental.add_operation ~expect_failure i op >>= fun _res -> return_unit + Incremental.add_operation ~expect_apply_failure i op >>= fun _res -> + return_unit (* the fee is smaller than the balance, then the transfer is accepted but it is not processed, and fees are taken *) else - Incremental.add_operation ~expect_failure i op >>=? fun i -> + Incremental.add_operation ~expect_apply_failure i op >>=? fun i -> (* contract_1 loses the fees *) Assert.balance_was_debited ~loc:__LOC__ (I i) contract_1 balance1 fee >>=? fun () -> @@ -428,7 +429,7 @@ let test_balance_too_low_two_transfers fee () = Context.Contract.balance (I i) contract_3 >>=? fun balance3 -> Op.transaction ~fee (I i) contract_1 contract_3 two_third_of_balance >>=? fun operation -> - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error err :: _ -> Assert.test_error_encodings err ; let error_info = @@ -438,7 +439,7 @@ let test_balance_too_low_two_transfers fee () = else failwith "unexpected error" | _ -> failwith "balance too low should fail" in - Incremental.add_operation ~expect_failure i operation >>=? fun i -> + Incremental.add_operation ~expect_apply_failure i operation >>=? fun i -> (* contract_1 loses the fees *) Assert.balance_was_debited ~loc:__LOC__ (I i) contract_1 balance1 fee >>=? fun () -> @@ -597,14 +598,14 @@ let transfer_to_itself_with_no_such_entrypoint () = Context.init1 () >>=? fun (b, addr) -> Incremental.begin_construction b >>=? fun i -> Op.transaction (B b) addr addr Tez.one ~entrypoint >>=? fun transaction -> - let expect_failure = function + let expect_apply_failure = function | Environment.Ecoproto_error (Script_tc_errors.No_such_entrypoint _ as e) :: _ -> Assert.test_error_encodings e ; return () | _ -> failwith "no such entrypoint should fail" in - Incremental.add_operation ~expect_failure i transaction >>= fun _res -> + Incremental.add_operation ~expect_apply_failure i transaction >>= fun _res -> return () let tests = diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_tx_rollup.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_tx_rollup.ml index 0e4c09691ee8..cdf741070779 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_tx_rollup.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_tx_rollup.ml @@ -79,7 +79,7 @@ let test_disable_feature_flag () = Incremental.begin_construction b >>=? fun i -> Op.tx_rollup_origination (I i) contract >>=? fun (op, _tx_rollup) -> Incremental.add_operation - ~expect_apply_failure:(check_proto_error Apply.Tx_rollup_feature_disabled) + ~expect_failure:(check_proto_error Apply.Tx_rollup_feature_disabled) i op >>=? fun _i -> return_unit @@ -101,7 +101,7 @@ let test_sunset () = Incremental.begin_construction b >>=? fun i -> Op.tx_rollup_origination (I i) contract >>=? fun (op, _tx_rollup) -> Incremental.add_operation - ~expect_apply_failure:(check_proto_error Apply.Tx_rollup_feature_disabled) + ~expect_failure:(check_proto_error Apply.Tx_rollup_feature_disabled) i op >>=? fun _i -> return_unit @@ -841,7 +841,7 @@ let test_add_batch_with_limit () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Tx_rollup_errors.Submit_batch_burn_exceeded _ -> true | _ -> false)) @@ -910,7 +910,7 @@ let test_batch_too_big () = Op.tx_rollup_submit_batch (I i) contract tx_rollup contents >>=? fun op -> Incremental.add_operation i - ~expect_apply_failure: + ~expect_failure: (check_proto_error Tx_rollup_errors.Message_size_exceeds_limit) op >>=? fun _ -> return_unit @@ -963,7 +963,7 @@ let test_inbox_size_too_big () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Tx_rollup_errors.Inbox_size_would_exceed_limit _ -> true | _ -> false)) @@ -1016,7 +1016,7 @@ let test_inbox_count_too_big () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Inbox_count_would_exceed_limit rollup -> rollup = tx_rollup @@ -1037,7 +1037,7 @@ let test_inbox_count_too_big () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Inbox_count_would_exceed_limit rollup -> rollup = tx_rollup @@ -1124,7 +1124,7 @@ let test_valid_deposit_inexistant_rollup () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Script_interpreter.Runtime_contract_error _ -> true | _ -> false)) @@ -1151,7 +1151,7 @@ let test_invalid_deposit_not_ticket () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Script_interpreter.Bad_contract_parameter _ -> true | _ -> false)) @@ -1241,7 +1241,7 @@ let test_invalid_deposit_too_big_ticket () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Tx_rollup_errors_repr.Ticket_payload_size_limit_exceeded _ -> true | _ -> false)) @@ -1292,7 +1292,7 @@ let test_invalid_deposit_too_big_ticket_type () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Tx_rollup_errors_repr.Ticket_payload_size_limit_exceeded _ -> true | _ -> false)) @@ -1369,7 +1369,7 @@ let test_invalid_entrypoint () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Script_interpreter.Bad_contract_parameter _ -> true | _ -> false)) @@ -1396,7 +1396,7 @@ let test_invalid_l2_address () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Script_interpreter.Bad_contract_parameter _ -> true | _ -> false)) @@ -1422,7 +1422,7 @@ let test_valid_deposit_invalid_amount () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_runtime_error Script_interpreter_defs.Tx_rollup_invalid_transaction_amount) >>=? fun _ -> return_unit @@ -1445,7 +1445,7 @@ let test_deposit_too_many_tickets () = Incremental.add_operation i operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error Apply.Tx_rollup_invalid_transaction_ticket_amount) >>=? fun i -> ignore i ; @@ -1572,7 +1572,7 @@ let test_commitment_duplication () = Incremental.add_operation i op - ~expect_failure:(check_proto_error Tx_rollup_errors.Wrong_batch_count) + ~expect_apply_failure:(check_proto_error Tx_rollup_errors.Wrong_batch_count) >>=? fun i -> (* Submit the correct one *) Context.get_level (I i) >>?= fun level -> @@ -1654,7 +1654,8 @@ let test_commit_current_inbox () = Incremental.add_operation i operation - ~expect_failure:(check_proto_error Tx_rollup_errors.No_uncommitted_inbox) + ~expect_apply_failure: + (check_proto_error Tx_rollup_errors.No_uncommitted_inbox) >>=? fun i -> ignore i ; return_unit @@ -1863,7 +1864,7 @@ let test_commitment_predecessor () = Tx_rollup_errors.Commitment_too_early {provided = tx_level 10l; expected = tx_level 0l} in - Incremental.add_operation i op ~expect_failure:(check_proto_error error) + Incremental.add_operation i op ~expect_apply_failure:(check_proto_error error) >>=? fun _ -> (* Now we submit a real commitment *) Op.tx_rollup_commit (I i) contract1 tx_rollup commitment >>=? fun op -> @@ -1887,7 +1888,7 @@ let test_commitment_predecessor () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_predecessor_hash {provided = None; expected} -> expected = commitment.predecessor @@ -1902,7 +1903,7 @@ let test_commitment_predecessor () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_predecessor_hash {provided = _; expected} -> expected = commitment.predecessor @@ -1943,7 +1944,7 @@ let test_full_inbox () = Incremental.add_operation i op - ~expect_failure:(check_proto_error Tx_rollup_errors.Too_many_inboxes) + ~expect_apply_failure:(check_proto_error Tx_rollup_errors.Too_many_inboxes) >>=? fun i -> ignore i ; return () @@ -1965,7 +1966,7 @@ let test_bond_finalization () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Bond_does_not_exist a_pkh1 -> a_pkh1 = pkh1 | _ -> false) @@ -1978,7 +1979,7 @@ let test_bond_finalization () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Bond_in_use a_pkh1 -> a_pkh1 = pkh1 | _ -> false) @@ -2017,7 +2018,7 @@ let test_finalization_edge_cases () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error @@ Tx_rollup_errors.No_commitment_to_finalize) >>=? fun _i -> let message = "bogus" in @@ -2031,7 +2032,7 @@ let test_finalization_edge_cases () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error @@ Tx_rollup_errors.No_commitment_to_finalize) >>=? fun _i -> make_incomplete_commitment_for_batch (I i) (tx_level 0l) tx_rollup [] @@ -2045,7 +2046,7 @@ let test_finalization_edge_cases () = Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error @@ Tx_rollup_errors.No_commitment_to_finalize) >>=? fun _i -> Incremental.finalize_block i >>=? fun b -> @@ -2083,7 +2084,8 @@ let test_too_many_commitments () = Incremental.add_operation i op - ~expect_failure:(check_proto_error Tx_rollup_errors.Too_many_commitments) + ~expect_apply_failure: + (check_proto_error Tx_rollup_errors.Too_many_commitments) >>=? fun i -> (* Wait out the withdrawal period. *) bake_until i 12l >>=? fun i -> @@ -2408,7 +2410,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.Proof_produced_rejected_state) >>=? fun i -> Incremental.finalize_block i >>=? fun b -> @@ -2573,7 +2575,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Proof_produced_rejected_state -> true | _ -> false) @@ -2863,7 +2865,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.Proof_failed_to_reject) >>=? fun _ -> return_unit @@ -2904,7 +2906,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error (Tx_rollup_errors.Wrong_rejection_hash { @@ -2951,7 +2953,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error (Tx_rollup_errors.Cannot_reject_level {provided = level; accepted_range = None})) @@ -3001,7 +3003,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error (Tx_rollup_errors.Cannot_reject_level {provided = level; accepted_range = Some (level2, level2)})) @@ -3045,7 +3047,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error (Tx_rollup_errors.Wrong_message_path {expected = expected_root})) >>=? fun _ -> return_unit @@ -3077,7 +3079,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error (Tx_rollup_errors.Wrong_message_position {level; position = 1; length = 1})) @@ -3121,7 +3123,7 @@ module Rejection = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.Proof_failed_to_reject) >>=? fun i -> (* Check with a reasonable proof *) @@ -3207,7 +3209,7 @@ module Rejection = struct test_large_rejection 10_000 >>=? fun (i, op) -> Incremental.add_operation i - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.Proof_produced_rejected_state) op >>=? fun _ -> return_unit @@ -3282,7 +3284,7 @@ module Rejection = struct In other terms, the rejection created in this function must fail if [n_withdraw <= tx_rollup_max_withdrawals_per_batch] but also must succeed to reject if [n_withdraw > tx_rollup_max_withdrawals_per_batch]. *) - let test_reject_withdrawals_helper ?expect_failure n_withdraw = + let test_reject_withdrawals_helper ?expect_apply_failure n_withdraw = let sk, pk, addr = gen_l2_account () in init_with_deposit ~tx_rollup_hard_size_limit_per_message:20_000 addr >>=? fun (b, account, _, tx_rollup, store, ticket_hash) -> @@ -3373,7 +3375,8 @@ module Rejection = struct ~previous_message_result ~previous_message_result_path:Tx_rollup_commitment.Merkle.dummy_path >>=? fun op -> - Incremental.add_operation i op ?expect_failure >>=? fun _ -> return_unit + Incremental.add_operation i op ?expect_apply_failure >>=? fun _ -> + return_unit let test_reject_withdrawals_limit () = context_init1 () >>=? fun (b, _) -> @@ -3381,13 +3384,14 @@ module Rejection = struct let limit = l2_parameters.Tx_rollup_l2_apply.tx_rollup_max_withdrawals_per_batch in - let expect_failure = + let expect_apply_failure = check_proto_error Tx_rollup_errors.Proof_produced_rejected_state in (* It must not be rejected: (limit - 1) is below the limit *) - test_reject_withdrawals_helper ~expect_failure (limit - 1) >>=? fun () -> + test_reject_withdrawals_helper ~expect_apply_failure (limit - 1) + >>=? fun () -> (* It must not be rejected: limit is the limit :p. *) - test_reject_withdrawals_helper ~expect_failure limit >>=? fun () -> + test_reject_withdrawals_helper ~expect_apply_failure limit >>=? fun () -> (* It must be rejected: (limit + 1) is above the limit *) test_reject_withdrawals_helper (limit + 1) @@ -3648,12 +3652,12 @@ module Single_message_inbox = struct Op.tx_rollup_submit_batch (B b) account tx_rollup contents >>=? fun operation -> Block.bake b ~operation - let reject ?expect_failure b tx_rollup account level commitment = + let reject ?expect_apply_failure b tx_rollup account level commitment = Format.printf "Rejecting level %a (%s)\n" Tx_rollup_level.pp level - (if Option.is_some expect_failure then "x" else "√") ; + (if Option.is_some expect_apply_failure then "x" else "√") ; l2_parameters (B b) >>=? fun l2_parameters -> Rejection.valid_empty_proof l2_parameters >>= fun proof -> let message_position = 0 in @@ -3675,7 +3679,7 @@ module Single_message_inbox = struct ~previous_message_result_path:Tx_rollup_commitment.Merkle.dummy_path >>=? fun operation -> Incremental.begin_construction b >>=? fun i -> - Incremental.add_operation i operation ?expect_failure >>=? fun i -> + Incremental.add_operation i operation ?expect_apply_failure >>=? fun i -> Incremental.finalize_block i let make_commit predecessor_commit messages = @@ -3705,15 +3709,15 @@ module Single_message_inbox = struct Rejection.previous_message_result; ] - let commit ?expect_failure b tx_rollup account commit = + let commit ?expect_apply_failure b tx_rollup account commit = Format.printf "Commiting for level %a (%s)\n" Tx_rollup_level.pp commit.Tx_rollup_commitment.level - (if Option.is_some expect_failure then "x" else "√") ; + (if Option.is_some expect_apply_failure then "x" else "√") ; Incremental.begin_construction b >>=? fun i -> Op.tx_rollup_commit (B b) account tx_rollup commit >>=? fun operation -> - Incremental.add_operation i operation ?expect_failure >>=? fun i -> + Incremental.add_operation i operation ?expect_apply_failure >>=? fun i -> Incremental.finalize_block i end @@ -3770,7 +3774,7 @@ let test_state () = tx_rollup account1 commit1 - ~expect_failure:(check_proto_error Tx_rollup_errors.Invalid_committer) + ~expect_apply_failure:(check_proto_error Tx_rollup_errors.Invalid_committer) >>=? fun b -> (* Commit an incorrect commitment again. *) commit b tx_rollup account2 commit1 >>=? fun b -> @@ -3835,7 +3839,7 @@ let test_state () = account2 Tx_rollup_level.root commit1 - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f (function | Tx_rollup_errors.Cannot_reject_level _ -> true | _ -> false)) @@ -3851,7 +3855,7 @@ let test_state () = Incremental.add_operation i operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.No_commitment_to_finalize) >>=? fun i -> ignore i ; @@ -3906,7 +3910,7 @@ let test_state_with_deleted () = Incremental.add_operation i operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.Remove_commitment_too_early) >>=? fun _ -> (* Wait for some blocks, then remove *) @@ -4449,7 +4453,7 @@ module Withdraw = struct (* any non-empty list will fail *) >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.No_finalized_commitment_for_level {level; window = None} -> @@ -4495,7 +4499,7 @@ module Withdraw = struct [] >>=? fun operation -> Incremental.add_operation - ~expect_apply_failure: + ~expect_failure: (check_proto_error Tx_rollup_errors.No_withdrawals_to_dispatch) incr operation @@ -4546,7 +4550,7 @@ module Withdraw = struct [{ticket_info with amount = Tx_rollup_l2_qty.of_int64_exn 9L}] >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_rejection_hash {provided = _; expected = `Valid_path (_, 0)} -> @@ -4567,7 +4571,7 @@ module Withdraw = struct [ticket_info; ticket_info] >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_rejection_hash {provided = _; expected = `Valid_path (_, 0)} -> @@ -4588,7 +4592,7 @@ module Withdraw = struct [{ticket_info with ty = Script.lazy_expr @@ Expr.from_string "unit"}] >>=? fun operation -> Incremental.add_operation - ~expect_failure:(function + ~expect_apply_failure:(function | Environment.Ecoproto_error (Script_tc_errors.Invalid_constant (_, _, _)) :: _ -> @@ -4609,7 +4613,7 @@ module Withdraw = struct [{ticket_info with contents = Script.lazy_expr @@ Expr.from_string "2"}] >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_rejection_hash {provided = _; expected = `Valid_path (_, 0)} -> @@ -4630,7 +4634,7 @@ module Withdraw = struct [{ticket_info with ticketer = withdraw_contract}] >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_rejection_hash {provided = _; expected = `Valid_path (_, 0)} -> @@ -4695,7 +4699,7 @@ module Withdraw = struct >>=? fun operation -> Incremental.begin_construction block >>=? fun incr -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.Withdrawals_already_dispatched) incr operation @@ -4807,7 +4811,7 @@ module Withdraw = struct withdraw_op account1 block Z.one >>=? fun operation -> Incremental.begin_construction block >>=? fun incr -> Incremental.add_operation - ~expect_failure:(function + ~expect_apply_failure:(function | Environment.Ecoproto_error (Ticket_balance.Negative_ticket_balance {key = _; balance}) :: _ -> @@ -4917,7 +4921,7 @@ module Withdraw = struct [ticket_info] >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_rejection_hash {provided = _; expected = `Valid_path (_, idx)} -> @@ -4938,7 +4942,7 @@ module Withdraw = struct [ticket_info] >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_rejection_hash {provided = _; expected = `Valid_path (_, idx)} -> @@ -4961,7 +4965,7 @@ module Withdraw = struct [ticket_info] >>=? fun operation -> Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.Wrong_rejection_hash {provided = _; expected = `Valid_path (_, idx)} -> @@ -5030,7 +5034,7 @@ module Withdraw = struct >>=? fun operation -> (* try with correct withdraw but too late *) Incremental.add_operation - ~expect_failure: + ~expect_apply_failure: (check_proto_error_f @@ function | Tx_rollup_errors.No_finalized_commitment_for_level {level; window = None} -> @@ -5175,7 +5179,7 @@ module Withdraw = struct Incremental.add_operation i op - ~expect_failure: + ~expect_apply_failure: (check_proto_error Tx_rollup_errors.Proof_produced_rejected_state) >>=? fun _i -> return (i, message_result) @@ -5512,7 +5516,8 @@ module Withdraw = struct >>=? fun operation -> Incremental.begin_construction block >>=? fun incr -> Incremental.add_operation - ~expect_failure:(check_proto_error Apply.Forbidden_zero_ticket_quantity) + ~expect_apply_failure: + (check_proto_error Apply.Forbidden_zero_ticket_quantity) incr operation >>=? fun _incr -> return_unit @@ -5553,7 +5558,8 @@ module Withdraw = struct >>=? fun operation -> Incremental.begin_construction block >>=? fun incr -> Incremental.add_operation - ~expect_failure:(check_proto_error Apply.Forbidden_zero_ticket_quantity) + ~expect_apply_failure: + (check_proto_error Apply.Forbidden_zero_ticket_quantity) incr operation >>=? fun _incr -> return_unit @@ -5571,7 +5577,8 @@ module Withdraw = struct >>=? fun operation -> Incremental.begin_construction block >>=? fun incr -> Incremental.add_operation - ~expect_failure:(check_proto_error Apply.Forbidden_zero_ticket_quantity) + ~expect_apply_failure: + (check_proto_error Apply.Forbidden_zero_ticket_quantity) incr operation >>=? fun _incr -> return_unit -- GitLab From f57c4bb30b44b4484b08d8be4392c00548c497e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Mon, 28 Mar 2022 16:43:04 +0200 Subject: [PATCH 03/13] Proto/Tests/Helpers: add option to force reveal in managers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default behaviour to attach a reveal operation to any manager if the latter is not reveal hinders testing revelations themselves with integration tests. We make this behaviour depend on an optional `?force_reveal` argument to `manager_operation`, and we set it to true for backwards compatibility. The default value should be false, actually, but here we avoided breaking existing integration tests and leave fixing this as follow up in tezos/tezos#2979. We also patch [manager_operation] to enable/disable force reveal. We propagate this to all the operation constructors for single manager operations. Co-authored-by: Germán Delbianco Co-authored-by: vbot --- .../lib_protocol/test/helpers/op.ml | 188 +++++++++++------- .../lib_protocol/test/helpers/op.mli | 154 ++++++++++++-- 2 files changed, 254 insertions(+), 88 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.ml b/src/proto_alpha/lib_protocol/test/helpers/op.ml index ce521bd8acd2..69381d861f0b 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/op.ml @@ -219,8 +219,14 @@ let combine_operations ?public_key ?counter ?spurious_operation ~source ctxt Environment.wrap_tzresult @@ Operation.of_list operations >>?= fun operations -> return @@ sign account.sk ctxt operations -let manager_operation ?counter ?(fee = Tez.zero) ?gas_limit ?storage_limit - ?public_key ~source ctxt operation = +(* FIXME tezos/tezos#2979 + + The [force_reveal] option should default to false, but this + requires going over nearly all existing protocol operation + integration tests. Instead, we went for the minimal interference + path and left original behaviour as default. *) +let manager_operation ?(force_reveal = true) ?counter ?(fee = Tez.zero) + ?gas_limit ?storage_limit ?public_key ~source ctxt operation = (match counter with | Some counter -> return counter | None -> Context.Contract.counter ctxt source) @@ -238,44 +244,49 @@ let manager_operation ?counter ?(fee = Tez.zero) ?gas_limit ?storage_limit Context.Contract.manager ctxt source >>=? fun account -> let public_key = Option.value ~default:account.pk public_key in let counter = Z.succ counter in - Context.Contract.is_manager_key_revealed ctxt source >|=? function - | true -> - let op = - Manager_operation - { - source = Signature.Public_key.hash public_key; - fee; - counter; - operation; - gas_limit; - storage_limit; - } - in - Contents_list (Single op) - | false -> - let op_reveal = - Manager_operation - { - source = Signature.Public_key.hash public_key; - fee = Tez.zero; - counter; - operation = Reveal public_key; - gas_limit = Gas.Arith.integral_of_int_exn 10000; - storage_limit = Z.zero; - } - in - let op = - Manager_operation - { - source = Signature.Public_key.hash public_key; - fee; - counter = Z.succ counter; - operation; - gas_limit; - storage_limit; - } - in - Contents_list (Cons (op_reveal, Single op)) + Context.Contract.is_manager_key_revealed ctxt source >|=? fun revealed -> + (* If the manager is revealed or we are not forcing reveals, we + generate a singleton manager operation. *) + if revealed || not force_reveal then + let op = + Manager_operation + { + source = Signature.Public_key.hash public_key; + fee; + counter; + operation; + gas_limit; + storage_limit; + } + in + Contents_list (Single op) + (* Otherwise if the manager is unrevealed and we are + force_revaling managers by default, we pre-attach a revelation + for it. *) + else + let op_reveal = + Manager_operation + { + source = Signature.Public_key.hash public_key; + fee = Tez.zero; + counter; + operation = Reveal public_key; + gas_limit = Gas.Arith.integral_of_int_exn 10_000; + storage_limit = Z.zero; + } + in + let op = + Manager_operation + { + source = Signature.Public_key.hash public_key; + fee; + counter = Z.succ counter; + operation; + gas_limit; + storage_limit; + } + in + Contents_list (Cons (op_reveal, Single op)) let revelation ?(fee = Tez.zero) ?(gas_limit = Gas.Arith.integral_of_int_exn 10000) ctxt public_key = @@ -311,8 +322,8 @@ let originated_contract op = Contract.Originated (originated_contract_hash op) exception Impossible -let contract_origination_gen k ?counter ?delegate ~script ?public_key ?credit - ?fee ?gas_limit ?storage_limit ctxt source = +let contract_origination_gen k ?force_reveal ?counter ?delegate ~script + ?public_key ?credit ?fee ?gas_limit ?storage_limit ctxt source = Context.Contract.manager ctxt source >>=? fun account -> let default_credit = Tez.of_mutez @@ Int64.of_int 1000001 in let default_credit = @@ -321,6 +332,7 @@ let contract_origination_gen k ?counter ?delegate ~script ?public_key ?credit let credit = Option.value ~default:default_credit credit in let operation = Origination {delegate; script; credit} in manager_operation + ?force_reveal ?counter ?public_key ?fee @@ -337,11 +349,12 @@ let contract_origination = let contract_origination_hash = contract_origination_gen (fun op -> (op, originated_contract_hash op)) -let register_global_constant ?counter ?public_key ?fee ?gas_limit ?storage_limit - ctxt ~source ~value = +let register_global_constant ?force_reveal ?counter ?public_key ?fee ?gas_limit + ?storage_limit ctxt ~source ~value = Context.Contract.manager ctxt source >>=? fun account -> let operation = Register_global_constant {value} in manager_operation + ?force_reveal ?counter ?public_key ?fee @@ -359,17 +372,42 @@ let miss_signed_endorsement ?level ~endorsed_block ctxt = let delegate = Account.find_alternate real_delegate_pkh in endorsement ~delegate:(delegate.pkh, slots) ~level ~endorsed_block ctxt () -let transaction ?counter ?fee ?gas_limit ?storage_limit +let unsafe_transaction ?force_reveal ?counter ?fee ?gas_limit ?storage_limit ?(parameters = Script.unit_parameter) ?(entrypoint = Entrypoint.default) ctxt (src : Contract.t) (destination : Contract.t) (amount : Tez.t) = let top = Transaction {amount; parameters; destination; entrypoint} in - manager_operation ?counter ?fee ?gas_limit ?storage_limit ~source:src ctxt top + manager_operation + ?force_reveal + ?counter + ?fee + ?gas_limit + ?storage_limit + ~source:src + ctxt + top >>=? fun sop -> Context.Contract.manager ctxt src >|=? fun account -> sign account.sk ctxt sop -let delegation ?fee ctxt source dst = +let transaction ?force_reveal ?counter ?fee ?gas_limit ?storage_limit + ?parameters ?entrypoint ctxt (src : Contract.t) (dst : Contract.t) + (amount : Tez.t) = + unsafe_transaction + ?force_reveal + ?counter + ?fee + ?gas_limit + ?storage_limit + ?parameters + ?entrypoint + ctxt + src + dst + amount + +let delegation ?force_reveal ?fee ctxt source dst = let top = Delegation dst in manager_operation + ?force_reveal ?fee ~gas_limit:(Gas.Arith.integral_of_int_exn 1000) ~source @@ -379,9 +417,10 @@ let delegation ?fee ctxt source dst = Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt sop -let set_deposits_limit ?fee ctxt source limit = +let set_deposits_limit ?force_reveal ?fee ctxt source limit = let top = Set_deposits_limit limit in manager_operation + ?force_reveal ?fee ~gas_limit:(Gas.Arith.integral_of_int_exn 1000) ~source @@ -500,9 +539,10 @@ let originated_tx_rollup op = in (nonce, Tx_rollup.Internal_for_tests.originated_tx_rollup nonce) -let tx_rollup_origination ?counter ?fee ?gas_limit ?storage_limit ctxt - (src : Contract.t) = +let tx_rollup_origination ?force_reveal ?counter ?fee ?gas_limit ?storage_limit + ctxt (src : Contract.t) = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -515,9 +555,11 @@ let tx_rollup_origination ?counter ?fee ?gas_limit ?storage_limit ctxt let op = sign account.sk ctxt to_sign_op in (op, originated_tx_rollup op |> snd) -let tx_rollup_submit_batch ?counter ?fee ?burn_limit ?gas_limit ?storage_limit - ctxt (source : Contract.t) (tx_rollup : Tx_rollup.t) (content : string) = +let tx_rollup_submit_batch ?force_reveal ?counter ?fee ?burn_limit ?gas_limit + ?storage_limit ctxt (source : Contract.t) (tx_rollup : Tx_rollup.t) + (content : string) = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -529,10 +571,11 @@ let tx_rollup_submit_batch ?counter ?fee ?burn_limit ?gas_limit ?storage_limit Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt to_sign_op -let tx_rollup_commit ?counter ?fee ?gas_limit ?storage_limit ctxt +let tx_rollup_commit ?force_reveal ?counter ?fee ?gas_limit ?storage_limit ctxt (source : Contract.t) (tx_rollup : Tx_rollup.t) (commitment : Tx_rollup_commitment.Full.t) = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -544,9 +587,10 @@ let tx_rollup_commit ?counter ?fee ?gas_limit ?storage_limit ctxt Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt to_sign_op -let tx_rollup_return_bond ?counter ?fee ?gas_limit ?storage_limit ctxt - (source : Contract.t) (tx_rollup : Tx_rollup.t) = +let tx_rollup_return_bond ?force_reveal ?counter ?fee ?gas_limit ?storage_limit + ctxt (source : Contract.t) (tx_rollup : Tx_rollup.t) = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -558,9 +602,10 @@ let tx_rollup_return_bond ?counter ?fee ?gas_limit ?storage_limit ctxt Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt to_sign_op -let tx_rollup_finalize ?counter ?fee ?gas_limit ?storage_limit ctxt - (source : Contract.t) (tx_rollup : Tx_rollup.t) = +let tx_rollup_finalize ?force_reveal ?counter ?fee ?gas_limit ?storage_limit + ctxt (source : Contract.t) (tx_rollup : Tx_rollup.t) = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -572,9 +617,10 @@ let tx_rollup_finalize ?counter ?fee ?gas_limit ?storage_limit ctxt Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt to_sign_op -let tx_rollup_remove_commitment ?counter ?fee ?gas_limit ?storage_limit ctxt - (source : Contract.t) (tx_rollup : Tx_rollup.t) = +let tx_rollup_remove_commitment ?force_reveal ?counter ?fee ?gas_limit + ?storage_limit ctxt (source : Contract.t) (tx_rollup : Tx_rollup.t) = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -586,10 +632,11 @@ let tx_rollup_remove_commitment ?counter ?fee ?gas_limit ?storage_limit ctxt Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt to_sign_op -let tx_rollup_dispatch_tickets ?counter ?fee ?gas_limit ?storage_limit ctxt - ~(source : Contract.t) ~message_index ~message_result_path tx_rollup level - context_hash tickets_info = +let tx_rollup_dispatch_tickets ?force_reveal ?counter ?fee ?gas_limit + ?storage_limit ctxt ~(source : Contract.t) ~message_index + ~message_result_path tx_rollup level context_hash tickets_info = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -609,10 +656,11 @@ let tx_rollup_dispatch_tickets ?counter ?fee ?gas_limit ?storage_limit ctxt Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt to_sign_op -let transfer_ticket ?counter ?fee ?gas_limit ?storage_limit ctxt +let transfer_ticket ?force_reveal ?counter ?fee ?gas_limit ?storage_limit ctxt ~(source : Contract.t) ~contents ~ty ~ticketer amount ~destination entrypoint = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -624,7 +672,7 @@ let transfer_ticket ?counter ?fee ?gas_limit ?storage_limit ctxt Context.Contract.manager ctxt source >|=? fun account -> sign account.sk ctxt to_sign_op -let tx_rollup_reject ?counter ?fee ?gas_limit ?storage_limit ctxt +let tx_rollup_reject ?force_reveal ?counter ?fee ?gas_limit ?storage_limit ctxt (source : Contract.t) (tx_rollup : Tx_rollup.t) (level : Tx_rollup_level.t) (message : Tx_rollup_message.t) ~(message_position : int) ~(message_path : Tx_rollup_inbox.Merkle.path) ~message_result_hash @@ -632,6 +680,7 @@ let tx_rollup_reject ?counter ?fee ?gas_limit ?storage_limit ctxt ~(previous_message_result : Tx_rollup_message_result.t) ~previous_message_result_path = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -660,9 +709,10 @@ let originated_sc_rollup op = let nonce = Origination_nonce.Internal_for_tests.initial packed in Sc_rollup.Internal_for_tests.originated_sc_rollup nonce -let sc_rollup_origination ?counter ?fee ?gas_limit ?storage_limit ctxt - (src : Contract.t) kind boot_sector parameters_ty = +let sc_rollup_origination ?force_reveal ?counter ?fee ?gas_limit ?storage_limit + ctxt (src : Contract.t) kind boot_sector parameters_ty = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -675,9 +725,10 @@ let sc_rollup_origination ?counter ?fee ?gas_limit ?storage_limit ctxt let op = sign account.sk ctxt to_sign_op in originated_sc_rollup op |> fun addr -> (op, addr) -let sc_rollup_publish ?counter ?fee ?gas_limit ?storage_limit ctxt +let sc_rollup_publish ?force_reveal ?counter ?fee ?gas_limit ?storage_limit ctxt (src : Contract.t) rollup commitment = manager_operation + ?force_reveal ?counter ?fee ?gas_limit @@ -689,9 +740,10 @@ let sc_rollup_publish ?counter ?fee ?gas_limit ?storage_limit ctxt Context.Contract.manager ctxt src >|=? fun account -> sign account.sk ctxt to_sign_op -let sc_rollup_cement ?counter ?fee ?gas_limit ?storage_limit ctxt +let sc_rollup_cement ?force_reveal ?counter ?fee ?gas_limit ?storage_limit ctxt (src : Contract.t) rollup commitment = manager_operation + ?force_reveal ?counter ?fee ?gas_limit diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.mli b/src/proto_alpha/lib_protocol/test/helpers/op.mli index c3463f0c2a91..f27d2b56e448 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/op.mli @@ -57,6 +57,7 @@ val miss_signed_endorsement : Kind.endorsement Operation.t tzresult Lwt.t val transaction : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -69,7 +70,35 @@ val transaction : Tez.t -> Operation.packed tzresult Lwt.t +(** Same as [transaction], but with a more generic destination + parameter. It is said unsafe because it can construct transactions + that will always fail, such as + + {ul {li Transaction to the deposit entrypoint of a transaction + rollup, as these transactions are necessarily internals.}} + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) +val unsafe_transaction : + ?force_reveal:bool -> + ?counter:counter -> + ?fee:Tez.t -> + ?gas_limit:Fixed_point_repr.integral_tag Gas.Arith.t -> + ?storage_limit:counter -> + ?parameters: + Michelson_v1_primitives.prim Micheline.canonical Data_encoding.lazy_t -> + ?entrypoint:Entrypoint_repr.t -> + Context.t -> + Contract.t -> + Contract.t -> + Tez.t -> + packed_operation tzresult Lwt.t + val delegation : + ?force_reveal:bool -> ?fee:Tez.tez -> Context.t -> Contract.t -> @@ -77,6 +106,7 @@ val delegation : Operation.packed tzresult Lwt.t val set_deposits_limit : + ?force_reveal:bool -> ?fee:Tez.tez -> Context.t -> Contract.t -> @@ -110,8 +140,15 @@ val failing_noop : address. The contract address is using the initial origination nonce with the hash of the operation. If this operation is combined with [combine_operations] then the contract address is false as the nonce is not based on the correct - operation hash. *) + operation hash. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val contract_origination : + ?force_reveal:bool -> ?counter:Z.t -> ?delegate:public_key_hash -> script:Script.t -> @@ -125,6 +162,7 @@ val contract_origination : (Operation.packed * Contract.t) tzresult Lwt.t val contract_origination_hash : + ?force_reveal:bool -> ?counter:Z.t -> ?delegate:public_key_hash -> script:Script.t -> @@ -140,6 +178,7 @@ val contract_origination_hash : val originated_contract : Operation.packed -> Contract.t val register_global_constant : + ?force_reveal:bool -> ?counter:Z.t -> ?public_key:Signature.public_key -> ?fee:Tez.tez -> @@ -223,6 +262,7 @@ val dummy_script_cost : Tez.t tx rollup address is false as the nonce is not based on the correct operation hash. *) val tx_rollup_origination : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -233,8 +273,15 @@ val tx_rollup_origination : (** [tx_rollup_submit_batch ctxt source tx_rollup batch] submits [batch], an array of bytes that is expected to be a batch of L2 - transactions, to be appended in the inbox of [tx_rollup]. *) + transactions, to be appended in the inbox of [tx_rollup]. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val tx_rollup_submit_batch : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?burn_limit:Tez.tez -> @@ -246,9 +293,16 @@ val tx_rollup_submit_batch : string -> Operation.packed tzresult Lwt.t -(** [tx_rollup_commit ctxt source tx_rollup commitment] Commits to a tx - rollup state. *) +(** [tx_rollup_commit ctxt source tx_rollup commitment] Commits to a + tx. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val tx_rollup_commit : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -259,8 +313,16 @@ val tx_rollup_commit : Tx_rollup_commitment.Full.t -> Operation.packed tzresult Lwt.t -(** [tx_rollup_return_bond ctxt source tx_rollup] returns a commitment bond. *) +(** [tx_rollup_return_bond ctxt source tx_rollup] returns a commitment + bond. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val tx_rollup_return_bond : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -270,9 +332,16 @@ val tx_rollup_return_bond : Tx_rollup.t -> Operation.packed tzresult Lwt.t -(** [tx_rollup_finalize ctxt source tx_rollup] finalizes the most recent - final level of a rollup. *) +(** [tx_rollup_finalize ctxt source tx_rollup] finalizes the most + recent final level of a rollup. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val tx_rollup_finalize : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -285,6 +354,7 @@ val tx_rollup_finalize : (** [tx_rollup_remove_commitment ctxt source tx_rollup] tries to remove a commitment from the rollup context. *) val tx_rollup_remove_commitment : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -295,11 +365,18 @@ val tx_rollup_remove_commitment : Operation.packed tzresult Lwt.t (** [tx_rollup_dispatch_tickets ctxt ~source ~message_index tx_rollup - level context_hash tickets_info] sends all tickets from - [tickets_info] to the appropriate implicit accounts, as authorized - by the [message_index]th hash of the commitment of [tx_rollup] - posted for [level]. *) + level context_hash tickets_info] sends all tickets from + [tickets_info] to the appropriate implicit accounts, as authorized + by the [message_index]th hash of the commitment of [tx_rollup] + posted for [level]. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}} *) val tx_rollup_dispatch_tickets : + ?force_reveal:bool -> ?counter:counter -> ?fee:Tez.t -> ?gas_limit:Gas.Arith.integral -> @@ -331,8 +408,15 @@ val tx_rollup_dispatch_tickets : {li [destination:Contract.t]: the destination contract that should receive the ticket of the withdrawal} {li [Entrypoint_repr.t]: the entrypoint of the destination - contract to which the ticket should be sent}} *) + contract to which the ticket should be sent}} + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val transfer_ticket : + ?force_reveal:bool -> ?counter:counter -> ?fee:Tez.t -> ?gas_limit:Gas.Arith.integral -> @@ -348,8 +432,15 @@ val transfer_ticket : (packed_operation, tztrace) result Lwt.t (** [tx_rollup_reject ctxt source tx_rollup tx_rollup level message - index proof] Rejects a tx rollup commitment. *) + index proof] Rejects a tx rollup commitment. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val tx_rollup_reject : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -368,10 +459,18 @@ val tx_rollup_reject : previous_message_result_path:Tx_rollup_commitment.Merkle.path -> Operation.packed tzresult Lwt.t -(** [sc_rollup_origination ctxt source kind boot_sector] originates a new - smart contract rollup of some given [kind] booting using [boot_sector]. - The process is the same as in [tx_rollup_origination]. *) +(** [sc_rollup_origination ctxt source kind boot_sector] originates a + new smart contract rollup of some given [kind] booting using + [boot_sector]. The process is the same as in + [tx_rollup_origination]. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val sc_rollup_origination : + ?force_reveal:bool -> ?counter:counter -> ?fee:Tez.t -> ?gas_limit:Gas.Arith.integral -> @@ -383,9 +482,17 @@ val sc_rollup_origination : Script.lazy_expr -> (packed_operation * Sc_rollup.t) tzresult Lwt.t -(** [sc_rollup_publish ctxt source rollup commitment] tries to publish a - commitment to the SCORU. *) +(** [sc_rollup_publish ctxt source rollup commitment] tries to publish + a commitment to the SCORU. Optional arguments allow to override + defaults: + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val sc_rollup_publish : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> @@ -396,9 +503,16 @@ val sc_rollup_publish : Sc_rollup.Commitment.t -> Operation.packed tzresult Lwt.t -(** [sc_rollup_cement ctxt source rollup commitment] tries to cement the - specified commitment. *) +(** [sc_rollup_cement ctxt source rollup commitment] tries to cement + the specified commitment. + + Optional arguments allow to override defaults: + + {ul {li [?force_reveal:bool]: prepend the operation to reveal + [source]'s public key if the latter has not been revealed + yet. Enabled (set to [true]) by default.}}*) val sc_rollup_cement : + ?force_reveal:bool -> ?counter:Z.t -> ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> -- GitLab From 9c523cfcfeff1b1c446994d42e3496afde9fc47e Mon Sep 17 00:00:00 2001 From: vbot Date: Tue, 26 Apr 2022 16:31:19 +0200 Subject: [PATCH 04/13] Proto/Tests/Helpers: allow batch_operations to recompute counters --- .../lib_protocol/test/helpers/op.ml | 19 ++++++++++++++++++- .../lib_protocol/test/helpers/op.mli | 6 ++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.ml b/src/proto_alpha/lib_protocol/test/helpers/op.ml index 69381d861f0b..424b340897db 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/op.ml @@ -121,7 +121,8 @@ let preendorsement ?delegate ?slot ?level ?round ?block_payload_hash let sign ?watermark sk ctxt (Contents_list contents) = Operation.pack (sign ?watermark sk ctxt contents) -let batch_operations ~source ctxt (operations : packed_operation list) = +let batch_operations ?(recompute_counters = false) ~source ctxt + (operations : packed_operation list) = let operations = List.map (function @@ -130,6 +131,22 @@ let batch_operations ~source ctxt (operations : packed_operation list) = operations |> List.flatten in + (if recompute_counters then + Context.Contract.counter ctxt source >>=? fun counter -> + (* Update counters and transform into a contents_list *) + let _, rev_operations = + List.fold_left + (fun (counter, acc) -> function + | Contents (Manager_operation m) -> + ( Z.succ counter, + Contents (Manager_operation {m with counter}) :: acc ) + | x -> (counter, x :: acc)) + (Z.succ counter, []) + operations + in + return (List.rev rev_operations) + else return operations) + >>=? fun operations -> Context.Contract.manager ctxt source >>=? fun account -> Environment.wrap_tzresult @@ Operation.of_list operations >>?= fun operations -> return @@ sign account.sk ctxt operations diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.mli b/src/proto_alpha/lib_protocol/test/helpers/op.mli index f27d2b56e448..93eec11108c5 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/op.mli @@ -224,9 +224,11 @@ val combine_operations : packed_operation list -> packed_operation tzresult Lwt.t -(** Batch a list of (already signed) operations and (re-)sign with the [source]. - No revelation is inserted and the counters are kept as they are. *) +(** Batch a list of (already signed) operations and (re-)sign with the + [source]. No revelation is inserted and the counters are kept as + they are unless [recompute_counters] is set to [true] (defaults false). *) val batch_operations : + ?recompute_counters:bool -> source:Contract.t -> Context.t -> packed_operation list -> -- GitLab From a554eb63b3493b1c8b785f5081e65c917597c939 Mon Sep 17 00:00:00 2001 From: vbot Date: Tue, 26 Apr 2022 13:50:27 +0200 Subject: [PATCH 05/13] Proto: perform reveals at application MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #2386 We refactor [apply_operation] so that [Reveal pkh] manager operations don't take effect at pre-checking time (that is, in [precheck_manager_contents]), but rather when _really_ applying the manager operation in [apply_manager_contents]. This refactoring does not only restore the all-or-nothing semantic of manager operations batches, but also lays the ground for future developments in the Pipelining project %23, by removing effects from [precheck_manager_contents]. Co-authored-by: vbot Co-authored-by: Germán Delbianco --- docs/protocols/alpha.rst | 4 ++++ src/proto_alpha/lib_protocol/apply.ml | 12 +++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/protocols/alpha.rst b/docs/protocols/alpha.rst index 8f7424475124..1478803ac683 100644 --- a/docs/protocols/alpha.rst +++ b/docs/protocols/alpha.rst @@ -38,6 +38,10 @@ RPC Changes Bug Fixes --------- +- Restore *all-or-nothing* semantics of manager operation batches by + enforcing that failing reveal operations do not take effect (MR + :gl:`!5182`). + Minor Changes ------------- diff --git a/src/proto_alpha/lib_protocol/apply.ml b/src/proto_alpha/lib_protocol/apply.ml index 2c0d94f1b2fe..b95b7a54d188 100644 --- a/src/proto_alpha/lib_protocol/apply.ml +++ b/src/proto_alpha/lib_protocol/apply.ml @@ -2,6 +2,7 @@ (* *) (* Open Source License *) (* Copyright (c) 2018 Dynamic Ledger Solutions, Inc. *) +(* Copyright (c) 2019-2022 Nomadic Labs *) (* Copyright (c) 2022 Trili Tech, *) (* *) (* Permission is hereby granted, free of charge, to any person obtaining a *) @@ -1237,9 +1238,9 @@ let apply_external_manager_operation_content : ~gas_consumed_in_precheck >>=? fun (ctxt, before_operation, consume_deserialization_gas) -> match operation with - | Reveal _ -> + | Reveal pk -> + Contract.reveal_manager_key ctxt source pk >>=? fun ctxt -> return - (* No-op: action already performed by `precheck_manager_contents`. *) ( ctxt, (Reveal_result {consumed_gas = Gas.consumed ~since:before_operation ~until:ctxt} @@ -1905,7 +1906,12 @@ let precheck_manager_contents (type kind) ctxt (op : kind Kind.manager contents) risk getting different results if the operation has already been deserialized before (e.g. when retrieve in JSON format). *) (match operation with - | Reveal pk -> Contract.reveal_manager_key ctxt source pk + | Reveal _pk -> + (* TODO #2603 + Should pre-check/validate have a specific precondition for + individual reveals? + *) + return ctxt | Transaction {parameters; _} -> Lwt.return @@ record_trace Gas_quota_exceeded_init_deserialize -- GitLab From 784fc321415cd4571db12ebd82190cf85724e533 Mon Sep 17 00:00:00 2001 From: Diane Gallois-Wong Date: Thu, 19 May 2022 15:29:54 +0200 Subject: [PATCH 06/13] Proto: check the consistency of revealed keys during precheck MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - We refactor the business logic of `Contract_manager_storage.reveal_manager_key` to be able to recover the consistency check of the revealed signature as a part of the prechecking phase. This prevents forging revelation operations which pass prechecking but then result in apply failure. We fork this precondition out reveal_manager_key, as a new function `check_public_key`, intended to be used at precheck time, which asserts that the provided (pk, pkh) pair is consistent: that is, that hashing pk yields the provided pkh. Then, the logic of `reveal_manager_key` is reworked to take an optional flag `?check_consistency` which if true (or by default) calls check_public_key, and otherwise just take the manager and the pk arguments at face value (see tezos/tezos#3078). `reveal_manager_key` still checks that the manager provided is the expected one, and that the operation was not already revealed and otherwise fail according at application time. - We recover asserting the consistency of the (pk, pkh) pair to be revealed at prechecking time, preventing accepting inconsistent revel operations which will fail at application time. - We modify the integration test helpers to enable faking reveal operations, and we add two integration test asserting reveal operations cannot be forged by providing an inconsistent (pk, pkh) pairs. Co-authored-by: Diane Gallois-Wong Co-authored-by: Albin Coquereau Co-authored-by: Zay Dargaye Co-authored-by: Germán Delbianco Co-authored-by: vbot --- src/proto_alpha/lib_protocol/alpha_context.ml | 2 + .../lib_protocol/alpha_context.mli | 8 +- src/proto_alpha/lib_protocol/apply.ml | 19 +- .../lib_protocol/contract_manager_storage.ml | 39 ++-- .../lib_protocol/contract_manager_storage.mli | 17 ++ .../lib_protocol/test/helpers/op.ml | 15 +- .../lib_protocol/test/helpers/op.mli | 14 +- .../integration/operations/test_reveal.ml | 169 +++++++++++++++++- 8 files changed, 255 insertions(+), 28 deletions(-) diff --git a/src/proto_alpha/lib_protocol/alpha_context.ml b/src/proto_alpha/lib_protocol/alpha_context.ml index 24bed7a7eaa8..aa624eef44a8 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.ml +++ b/src/proto_alpha/lib_protocol/alpha_context.ml @@ -248,6 +248,8 @@ module Contract = struct let is_manager_key_revealed = Contract_manager_storage.is_manager_key_revealed + let check_public_key = Contract_manager_storage.check_public_key + let reveal_manager_key = Contract_manager_storage.reveal_manager_key let get_manager_key = Contract_manager_storage.get_manager_key diff --git a/src/proto_alpha/lib_protocol/alpha_context.mli b/src/proto_alpha/lib_protocol/alpha_context.mli index 5b9d279b7dae..9d3a52dc0a0f 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/alpha_context.mli @@ -1537,8 +1537,14 @@ module Contract : sig val is_manager_key_revealed : context -> public_key_hash -> bool tzresult Lwt.t + val check_public_key : public_key -> public_key_hash -> unit tzresult + val reveal_manager_key : - context -> public_key_hash -> public_key -> context tzresult Lwt.t + ?check_consistency:bool -> + context -> + public_key_hash -> + public_key -> + context tzresult Lwt.t val get_script_code : context -> t -> (context * Script.lazy_expr option) tzresult Lwt.t diff --git a/src/proto_alpha/lib_protocol/apply.ml b/src/proto_alpha/lib_protocol/apply.ml index b95b7a54d188..8f11fe030567 100644 --- a/src/proto_alpha/lib_protocol/apply.ml +++ b/src/proto_alpha/lib_protocol/apply.ml @@ -1239,7 +1239,17 @@ let apply_external_manager_operation_content : >>=? fun (ctxt, before_operation, consume_deserialization_gas) -> match operation with | Reveal pk -> - Contract.reveal_manager_key ctxt source pk >>=? fun ctxt -> + (* TODO tezos/tezos#3070 + + We have already asserted the consistency of the supplied public + key during precheck, so we avoid re-checking that precondition + with [?check_consistency=false]. This optional parameter is + temporary, to avoid breaking compatibility with external legacy + usage of [Contract.reveal_manager_key]. However, the pattern of + using [Contract.check_public_key] and this usage of + [Contract.reveal_manager_key] should become the standard. *) + Contract.reveal_manager_key ~check_consistency:false ctxt source pk + >>=? fun ctxt -> return ( ctxt, (Reveal_result @@ -1906,12 +1916,7 @@ let precheck_manager_contents (type kind) ctxt (op : kind Kind.manager contents) risk getting different results if the operation has already been deserialized before (e.g. when retrieve in JSON format). *) (match operation with - | Reveal _pk -> - (* TODO #2603 - Should pre-check/validate have a specific precondition for - individual reveals? - *) - return ctxt + | Reveal pk -> Contract.check_public_key pk source >>?= fun () -> return ctxt | Transaction {parameters; _} -> Lwt.return @@ record_trace Gas_quota_exceeded_init_deserialize diff --git a/src/proto_alpha/lib_protocol/contract_manager_storage.ml b/src/proto_alpha/lib_protocol/contract_manager_storage.ml index f1785234da5c..5c9d0e6d46ae 100644 --- a/src/proto_alpha/lib_protocol/contract_manager_storage.ml +++ b/src/proto_alpha/lib_protocol/contract_manager_storage.ml @@ -115,19 +115,38 @@ let is_manager_key_revealed c manager = | Some (Manager_repr.Hash _) -> return_false | Some (Manager_repr.Public_key _) -> return_true -let reveal_manager_key c manager public_key = +let check_public_key public_key expected_hash = + let provided_hash = Signature.Public_key.hash public_key in + error_unless + (Signature.Public_key_hash.equal provided_hash expected_hash) + (Inconsistent_hash {public_key; expected_hash; provided_hash}) + +let reveal_manager_key ?(check_consistency = true) c manager public_key = let contract = Contract_repr.Implicit manager in Storage.Contract.Manager.get c contract >>=? function | Public_key _ -> fail (Previously_revealed_key contract) - | Hash v -> - let actual_hash = Signature.Public_key.hash public_key in - if Signature.Public_key_hash.equal actual_hash v then - let v = Manager_repr.Public_key public_key in - Storage.Contract.Manager.update c contract v - else - fail - (Inconsistent_hash - {public_key; expected_hash = v; provided_hash = actual_hash}) + | Hash expected_hash -> + (* Ensure that the manager is equal to the retrieved hash. *) + error_unless + (Signature.Public_key_hash.equal manager expected_hash) + (Inconsistent_hash {public_key; expected_hash; provided_hash = manager}) + >>?= fun () -> + (* TODO tezos/tezos#3078 + + We keep the consistency check and the optional argument to + preserve the semantics of reveal_manager_key prior to + tezos/tezos!5182, when called outside the scope of + [apply_operation]. + + Inside appply.ml, it is used with + ?check_consistency=false. Ultimately this parameter should go + away, and the split check_publick_key / reveal_manager_key + pattern has to be exported to usage outside apply.ml *) + when_ check_consistency (fun () -> + Lwt.return @@ check_public_key public_key expected_hash) + >>=? fun () -> + let pk = Manager_repr.Public_key public_key in + Storage.Contract.Manager.update c contract pk let get_manager_key ?error ctxt pkh = let contract = Contract_repr.Implicit pkh in diff --git a/src/proto_alpha/lib_protocol/contract_manager_storage.mli b/src/proto_alpha/lib_protocol/contract_manager_storage.mli index 99d8cd19a064..6c3f74941dc3 100644 --- a/src/proto_alpha/lib_protocol/contract_manager_storage.mli +++ b/src/proto_alpha/lib_protocol/contract_manager_storage.mli @@ -46,7 +46,24 @@ val init : val is_manager_key_revealed : Raw_context.t -> Signature.Public_key_hash.t -> bool tzresult Lwt.t +(** [check_publick_key pk pkh] asserts that the provided [pk] is + consistent with the expected public key hash [pkh], otherwise + fails with an [Inconsistent_hash] error. *) +val check_public_key : + Signature.Public_key.t -> Signature.Public_key_hash.t -> unit tzresult + +(** [reveal_manager_key ?check_consistency ctxt manager pk] reveals + the public key [pk] for a given unrevealed [manager]. If the + optional [?check_consistency] flag is set (and it is set by + default), it will re-check the same consistency checks than + [check_public_key] above, otherwise it will assume [manager] is + indeed the hash of [pk]. It is expected to fail with + [Previously_revealed_key contract] if [manager] was already + revealed, and with [Inconsistent_hash] if the (unrevealed) [manager] + doesn't match the expected hash of the implicit contract associated + to [pk]. *) val reveal_manager_key : + ?check_consistency:bool -> Raw_context.t -> Signature.Public_key_hash.t -> Signature.Public_key.t -> diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.ml b/src/proto_alpha/lib_protocol/test/helpers/op.ml index 424b340897db..2f6da236b847 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/op.ml @@ -306,8 +306,17 @@ let manager_operation ?(force_reveal = true) ?counter ?(fee = Tez.zero) Contents_list (Cons (op_reveal, Single op)) let revelation ?(fee = Tez.zero) - ?(gas_limit = Gas.Arith.integral_of_int_exn 10000) ctxt public_key = - let pkh = Signature.Public_key.hash public_key in + ?(gas_limit = Gas.Arith.integral_of_int_exn 10000) ?(forge_pkh = None) ctxt + public_key = + (* If Some pkh is provided to ?forge_pkh we take that hash at face + value, otherwise we honestly compute the hash from + [public_key]. This is useful to test forging Reveal operations + (cf. tezos!5182). *) + let pkh = + match forge_pkh with + | Some pkh -> pkh + | None -> Signature.Public_key.hash public_key + in let source = Contract.Implicit pkh in Context.Contract.counter ctxt source >>=? fun counter -> Context.Contract.manager ctxt source >|=? fun account -> @@ -317,7 +326,7 @@ let revelation ?(fee = Tez.zero) (Single (Manager_operation { - source = Signature.Public_key.hash public_key; + source = pkh; fee; counter; operation = Reveal public_key; diff --git a/src/proto_alpha/lib_protocol/test/helpers/op.mli b/src/proto_alpha/lib_protocol/test/helpers/op.mli index 93eec11108c5..6d2bdec7248b 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/op.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/op.mli @@ -113,9 +113,9 @@ val set_deposits_limit : Tez.tez option -> Operation.packed tzresult Lwt.t -(** [revelation ?fee ?gas_limit ctxt pkh] Creates a new [Reveal] - {!manager_operation} to reveal a public key [pkh] applying to - current context [ctxt]. +(** [revelation ?fee ?gas_limit ?forge_pkh ctxt pkh] Creates a new + [Reveal] {!manager_operation} to reveal a public key [pkh] applying + to current context [ctxt]. Optional arguments allow to override defaults: @@ -123,11 +123,15 @@ val set_deposits_limit : [Tez.zero].} {li [?gas_limit:Gas.Arith.integral]: force a gas limit, - otherwise set to 10000 gas units.}} -*) + otherwise set to 10000 gas units.} + + {li [?forge_pkh]: use a + provided [pkh] as source, instead of hashing [pkh]. Useful for + forging non-honest reveal operations} *) val revelation : ?fee:Tez.tez -> ?gas_limit:Gas.Arith.integral -> + ?forge_pkh:public_key_hash option -> Context.t -> public_key -> Operation.packed tzresult Lwt.t diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml index 992b91392b2f..098320ad6d9f 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml @@ -1,7 +1,7 @@ (*****************************************************************************) (* *) (* Open Source License *) -(* Copyright (c) 2020 Nomadic Labs. *) +(* Copyright (c) 2020-2022 Nomadic Labs. *) (* *) (* Permission is hereby granted, free of charge, to any person obtaining a *) (* copy of this software and associated documentation files (the "Software"),*) @@ -32,7 +32,7 @@ Subject: On the reveal operation. *) -(** Test for the [Reveal] operation. *) +(** Protocol integration tests for the [Reveal] operation. *) open Protocol open Alpha_context @@ -112,6 +112,163 @@ let test_transfer_fees_emptying_after_reveal_batched () = Incremental.add_operation ~expect_apply_failure:(fun _ -> return_unit) inc op >>=? fun _inc -> return_unit +(* We assert that the changes introduced in !5182, splitting the + application of Reveal operations into a pre-checking and + an application phase, do not allow to forge dishonest revelations. *) +let test_reveal_with_fake_account () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, bootstrap) -> + (* Create two fresh, unrevealed, accounts a and b. *) + let account_a = Account.new_account () in + let a_pkh = account_a.pkh in + let a_contract = Contract.Implicit a_pkh in + let account_b = Account.new_account () in + let b_pkh = account_b.pkh in + let b_contract = Contract.Implicit b_pkh in + (* Assert a and b are fresh.*) + (* TODO tezos/tezos#2996 + + These preambles are too verbose and boilerplate. We should factor + out revealing fresh unrevealed accounts. *) + when_ (Signature.Public_key_hash.equal a_pkh b_pkh) (fun () -> + failwith + "Expected different pkhs: got %a %a" + Signature.Public_key_hash.pp + a_pkh + Signature.Public_key_hash.pp + b_pkh) + >>=? fun () -> + Op.transaction (B blk) bootstrap a_contract Tez.one >>=? fun oa -> + Op.transaction (B blk) bootstrap b_contract Tez.one >>=? fun ob -> + Op.batch_operations + ~recompute_counters:true + ~source:bootstrap + (B blk) + [oa; ob] + >>=? fun batch -> + Block.bake blk ~operation:batch >>=? fun b -> + (Context.Contract.is_manager_key_revealed (B blk) a_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + (Context.Contract.is_manager_key_revealed (B blk) b_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + (* get initial balance of account_a *) + Context.Contract.balance (B b) a_contract >>=? fun a_balance_before -> + (* We will attempt to forge a reveal with a fake account that + impersonates account_a but uses account_b's public and secret + keys, e.g. + + fake_a = Account.{pkh = account_a.pkh; pk = account_b.pk; sk = + account_b.sk} + + and we will attempt to reveal the public key of b with a's + pkh. This operation should fail without updating account_a's + balance *) + Op.revelation ~fee:Tez.one_mutez ~forge_pkh:(Some a_pkh) (B b) account_b.pk + >>=? fun operation -> + Incremental.begin_construction b >>=? fun i -> + Incremental.add_operation + ~expect_failure:(function + | [ + Environment.Ecoproto_error + (Contract_manager_storage.Inconsistent_hash _); + ] -> + return_unit + | errs -> + failwith + "Expected an Contract_manager_storage.Inconsistent_hash error but \ + got %a" + Error_monad.pp_print_trace + errs) + i + operation + >>=? fun i -> + Context.Contract.balance (I i) a_contract >>=? fun a_balance_after -> + unless (Tez.equal a_balance_after a_balance_before) (fun () -> + failwith + "Balance of contract_a should have not changed: expected %atz, got %atz" + Tez.pp + a_balance_before + Tez.pp + a_balance_after) + +(* On the following test, we create an account a, fund it, reveal it, + and get its balance. Then we attempt to forge a reveal for another + account b, using a's pkh. *) +let test_reveal_with_fake_account_already_revealed () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, bootstrap) -> + (* Create two fresh, unrevealed, accounts a and b. *) + let account_a = Account.new_account () in + let a_pkh = account_a.pkh in + let a_contract = Contract.Implicit a_pkh in + let account_b = Account.new_account () in + let b_pkh = account_b.pkh in + let b_contract = Contract.Implicit b_pkh in + (* Assert a and b are fresh.*) + (* TODO tezos/tezos#2996 + + These preambles are too verbose and boilerplate. We should factor + out revealing fresh unrevealed accounts. *) + when_ (Signature.Public_key_hash.equal a_pkh b_pkh) (fun () -> + failwith + "Expected different pkhs: got %a %a" + Signature.Public_key_hash.pp + a_pkh + Signature.Public_key_hash.pp + b_pkh) + >>=? fun () -> + Op.transaction (B blk) bootstrap a_contract Tez.one >>=? fun oa -> + Op.transaction (B blk) bootstrap b_contract Tez.one >>=? fun ob -> + Op.batch_operations + ~recompute_counters:true + ~source:bootstrap + (B blk) + [oa; ob] + >>=? fun batch -> + Block.bake blk ~operation:batch >>=? fun b -> + (Context.Contract.is_manager_key_revealed (B blk) a_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + (Context.Contract.is_manager_key_revealed (B blk) b_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + (* We first reveal a in a block *) + Op.revelation ~fee:Tez.one_mutez (B b) account_a.pk >>=? fun operation -> + Block.bake ~operation b >>=? fun b -> + Context.Contract.balance (B b) a_contract >>=? fun a_balance_before -> + (* Reveal the public key of b while impersonating account_a. This + operation should fail without updating account_a's balance *) + Op.revelation ~fee:Tez.one_mutez ~forge_pkh:(Some a_pkh) (B b) account_b.pk + >>=? fun operation -> + Incremental.begin_construction b >>=? fun i -> + Incremental.add_operation + ~expect_failure:(function + | [ + Environment.Ecoproto_error + (Contract_manager_storage.Inconsistent_hash _); + ] -> + return_unit + | errs -> + failwith + "Expected a Previously_revealed_key error but got %a" + Error_monad.pp_print_trace + errs) + i + operation + >>=? fun i -> + Context.Contract.balance (I i) a_contract >>=? fun a_balance_after -> + unless (Tez.equal a_balance_after a_balance_before) (fun () -> + failwith + "Balance of contract_a should have not changed: expected %atz, got %atz" + Tez.pp + a_balance_before + Tez.pp + a_balance_after) + let tests = [ Tztest.tztest "simple reveal" `Quick test_simple_reveal; @@ -124,4 +281,12 @@ let tests = "transfer fees emptying balance after reveal in batch" `Quick test_transfer_fees_emptying_after_reveal_batched; + Tztest.tztest + "cannot forge reveal with fake keys and signature" + `Quick + test_reveal_with_fake_account; + Tztest.tztest + "cannot re-reveal an account with fake keys and signature" + `Quick + test_reveal_with_fake_account_already_revealed; ] -- GitLab From 6222af29ed590061377d9dc4b5ee148063f02f92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Thu, 24 Mar 2022 15:37:32 +0100 Subject: [PATCH 07/13] Proto: properly backtrack and skip reveals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #2386. After having restored the _all-or-nothing_ semantics for manager operation batches, reveal operations are also backtracked. There is no need to unmark reveals as skipped anymore (tezos/tezos!857). We add a protocol integration test, `test_backtracked_reveal_in_batch`, to assert that backtracked reveals do not longer take effect. Co-authored-by: Germán Delbianco Co-authored-by: vbot --- src/proto_alpha/lib_protocol/apply.ml | 99 ++++++++----------- .../lib_protocol/contract_storage.mli | 4 +- .../integration/operations/test_reveal.ml | 51 ++++++++++ 3 files changed, 96 insertions(+), 58 deletions(-) diff --git a/src/proto_alpha/lib_protocol/apply.ml b/src/proto_alpha/lib_protocol/apply.ml index 8f11fe030567..6361fa887be6 100644 --- a/src/proto_alpha/lib_protocol/apply.ml +++ b/src/proto_alpha/lib_protocol/apply.ml @@ -2330,17 +2330,6 @@ let apply_manager_contents (type kind) ctxt mode chain_id | Error errors -> Lwt.return (Failure, Failed (manager_kind operation, errors), []) -let skipped_operation_result : - type kind. kind manager_operation -> kind manager_operation_result = - function - | operation -> ( - match operation with - | Reveal _ -> - Applied - (Reveal_result {consumed_gas = Gas.Arith.zero} - : kind successful_manager_operation_result) - | _ -> Skipped (manager_kind operation)) - let rec mark_skipped : type kind. payload_producer:Signature.Public_key_hash.t -> @@ -2358,7 +2347,7 @@ let rec mark_skipped : (Manager_operation_result { balance_updates; - operation_result = skipped_operation_result operation; + operation_result = Skipped (manager_kind operation); internal_operation_results = []; }) | PrecheckedCons @@ -2371,7 +2360,7 @@ let rec mark_skipped : ( Manager_operation_result { balance_updates; - operation_result = skipped_operation_result operation; + operation_result = Skipped (manager_kind operation); internal_operation_results = []; }, mark_skipped ~payload_producer level rest ) @@ -2536,54 +2525,50 @@ let rec apply_manager_contents_list_rec : (ctxt_result, Cons_result (result, results))) let mark_backtracked results = - let rec mark_contents_list : + let mark_results : + type kind. + kind Kind.manager contents_result -> kind Kind.manager contents_result = + fun results -> + let mark_manager_operation_result : + type kind. + kind manager_operation_result -> kind manager_operation_result = + function + | (Failed _ | Skipped _ | Backtracked _) as result -> result + | Applied result -> Backtracked (result, None) + in + let mark_internal_manager_operation_result : + type kind. + kind internal_manager_operation_result -> + kind internal_manager_operation_result = function + | (Failed _ | Skipped _ | Backtracked _) as result -> result + | Applied result -> Backtracked (result, None) + in + let mark_internal_operation_results + (Internal_manager_operation_result (kind, result)) = + Internal_manager_operation_result + (kind, mark_internal_manager_operation_result result) + in + match results with + | Manager_operation_result op -> + Manager_operation_result + { + balance_updates = op.balance_updates; + operation_result = mark_manager_operation_result op.operation_result; + internal_operation_results = + List.map + mark_internal_operation_results + op.internal_operation_results; + } + in + let rec traverse_apply_results : type kind. kind Kind.manager contents_result_list -> kind Kind.manager contents_result_list = function - | Single_result (Manager_operation_result op) -> - Single_result - (Manager_operation_result - { - balance_updates = op.balance_updates; - operation_result = - mark_manager_operation_result op.operation_result; - internal_operation_results = - List.map - mark_internal_operation_results - op.internal_operation_results; - }) - | Cons_result (Manager_operation_result op, rest) -> - Cons_result - ( Manager_operation_result - { - balance_updates = op.balance_updates; - operation_result = - mark_manager_operation_result op.operation_result; - internal_operation_results = - List.map - mark_internal_operation_results - op.internal_operation_results; - }, - mark_contents_list rest ) - and mark_internal_operation_results - (Internal_manager_operation_result (kind, result)) = - Internal_manager_operation_result - (kind, mark_internal_manager_operation_result result) - and mark_manager_operation_result : - type kind. kind manager_operation_result -> kind manager_operation_result - = function - | (Failed _ | Skipped _ | Backtracked _) as result -> result - | Applied (Reveal_result _) as result -> result - | Applied result -> Backtracked (result, None) - and mark_internal_manager_operation_result : - type kind. - kind internal_manager_operation_result -> - kind internal_manager_operation_result = function - | (Failed _ | Skipped _ | Backtracked _) as result -> result - | Applied result -> Backtracked (result, None) + | Single_result res -> Single_result (mark_results res) + | Cons_result (res, rest) -> + Cons_result (mark_results res, traverse_apply_results rest) in - mark_contents_list results - [@@coq_axiom_with_reason "non-top-level mutual recursion"] + traverse_apply_results results type apply_mode = | Application of { diff --git a/src/proto_alpha/lib_protocol/contract_storage.mli b/src/proto_alpha/lib_protocol/contract_storage.mli index ec287ace252e..2a18d321b47b 100644 --- a/src/proto_alpha/lib_protocol/contract_storage.mli +++ b/src/proto_alpha/lib_protocol/contract_storage.mli @@ -27,6 +27,8 @@ contracts. *) type error += + | (* `Temporary *) + Balance_too_low of Contract_repr.t * Tez_repr.t * Tez_repr.t | (* `Branch *) Counter_in_the_future of Contract_repr.t * Z.t * Z.t | (* `Temporary *) @@ -137,7 +139,7 @@ val spend_only_call_from_token : (** [raw_originate ctxt ~prepaid_bootstrap_storage contract ~script] originates the [contract] parameter. The [storage] space allocated by this origination is considered to be free of charge or to have been already paid - for by the user, if and only if [prepaid_bootstrap_storage] is [true]. In + for by the user, if and only if [prepaid_bootstrap_storage] is [true]. In particular, the amount of space allocated by this origination will be part of the consumed space to pay for returned by the next call to [Fees_storage.record_paid_storage_space ctxt contract], if and only if diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml index 098320ad6d9f..f0fcaf1d0137 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml @@ -269,6 +269,53 @@ let test_reveal_with_fake_account_already_revealed () = Tez.pp a_balance_after) +(* cf: #2386 + + On tezos/tezos!5182 we have reverted the behaviour implemented by + tezos/tezos!587, which explicitly avoided marking reveal operations + as backtracked to reflect the fact that a reveal in a failing batch + did still take effect (cf #338). + + We test that backtracked reveals stay backtracked. *) +let test_backtracked_reveal_in_batch () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + (* Create the contract *) + Op.transaction (B blk) c new_contract Tez.one >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + Incremental.begin_construction blk >>=? fun inc -> + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun op_reveal -> + Op.transaction + ~force_reveal:false + ~fee:Tez.zero + (I inc) + new_contract + new_contract + (Tez.of_mutez_exn 1_000_001L) + >>=? fun op_transfer -> + Op.batch_operations + ~recompute_counters:true + ~source:new_contract + (I inc) + [op_reveal; op_transfer] + >>=? fun batched_operation -> + let expect_apply_failure = function + | [Environment.Ecoproto_error (Contract_storage.Balance_too_low _)] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_apply_failure inc batched_operation + >>=? fun inc -> + (* We assert the manager key is still unrevealed, as the batch has failed *) + Context.Contract.is_manager_key_revealed (I inc) new_contract >>=? fun revelead -> + when_ revelead (fun () -> + failwith "Unexpected contract revelation: reveal was expected to fail") + let tests = [ Tztest.tztest "simple reveal" `Quick test_simple_reveal; @@ -289,4 +336,8 @@ let tests = "cannot re-reveal an account with fake keys and signature" `Quick test_reveal_with_fake_account_already_revealed; + Tztest.tztest + "a backtracked reveal in a batch doesn't take effect" + `Quick + test_backtracked_reveal_in_batch; ] -- GitLab From 779c76a3c2781821cd9d8309e067db0480358fd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Mon, 28 Mar 2022 15:53:59 +0200 Subject: [PATCH 08/13] Proto: add an existence check before revealing a manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We check that the manager exists before revealing it, this avoids attempting to reveal a manager which was depleted by preckeck. We then fix and tweak existing transfer and revelation integration tests, to assert an application failure on `add_operation` with the (now exported) `Empty_implicit_contract` protocol error. Co-authored-by: Germán Delbianco Co-authored-by: vbot --- src/proto_alpha/lib_protocol/apply.ml | 8 ++++ .../lib_protocol/contract_storage.mli | 2 + .../integration/operations/test_reveal.ml | 38 ++++++++++++++----- .../integration/operations/test_transfer.ml | 28 ++++++++++++-- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/src/proto_alpha/lib_protocol/apply.ml b/src/proto_alpha/lib_protocol/apply.ml index 6361fa887be6..460a3bd706a1 100644 --- a/src/proto_alpha/lib_protocol/apply.ml +++ b/src/proto_alpha/lib_protocol/apply.ml @@ -1239,6 +1239,14 @@ let apply_external_manager_operation_content : >>=? fun (ctxt, before_operation, consume_deserialization_gas) -> match operation with | Reveal pk -> + (* TODO #2603 + + Even if [precheck_manager_contents] has already asserted that + the implicit contract is allocated, we must re-do this check in + case the manager has been emptied while collecting fees. This + should be solved by forking out [validate_operation] from + [apply_operation]. *) + Contract.must_be_allocated ctxt source_contract >>=? fun () -> (* TODO tezos/tezos#3070 We have already asserted the consistency of the supplied public diff --git a/src/proto_alpha/lib_protocol/contract_storage.mli b/src/proto_alpha/lib_protocol/contract_storage.mli index 2a18d321b47b..f15e68debbc1 100644 --- a/src/proto_alpha/lib_protocol/contract_storage.mli +++ b/src/proto_alpha/lib_protocol/contract_storage.mli @@ -37,6 +37,8 @@ type error += Inconsistent_public_key of Signature.Public_key.t * Signature.Public_key.t | (* `Permanent *) Failure of string + | (* `Branch *) + Empty_implicit_contract of Signature.Public_key_hash.t (** [allocated ctxt contract] returns [true] if and only if the contract is stored in [Storage.Contract.Balance]. *) diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml index f0fcaf1d0137..8f50c4b63928 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml @@ -67,19 +67,28 @@ let test_empty_account_on_reveal () = Op.transaction (B blk) c new_contract amount >>=? fun operation -> Block.bake blk ~operation >>=? fun blk -> (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function - | true -> Stdlib.failwith "Unexpected revelation" + | true -> Stdlib.failwith "Unexpected revelation: expecting fresh pkh" | false -> ()) >>=? fun () -> (* Reveal the contract *) Op.revelation ~fee:amount (B blk) new_c.pk >>=? fun operation -> Incremental.begin_construction blk >>=? fun inc -> - Incremental.add_operation inc operation >>=? fun _ -> - Block.bake blk ~operation >>=? fun blk -> - Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + let expect_apply_failure = function + | [ + Environment.Ecoproto_error (Contract_storage.Empty_implicit_contract pkh); + ] + when pkh = new_c.pkh -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_apply_failure inc operation >>=? fun inc -> + Context.Contract.balance (I inc) new_contract >>=? fun balance -> + Assert.equal_tez ~loc:__LOC__ balance Tez.zero >>=? fun () -> + Context.Contract.is_manager_key_revealed (I inc) new_contract >|=? function | false -> () | true -> Stdlib.failwith "Empty account still exists and is revealed." -let test_not_enough_found_for_reveal () = +let test_not_enough_funds_for_reveal () = Context.init1 () >>=? fun (blk, c) -> let new_c = Account.new_account () in let new_contract = Alpha_context.Contract.Implicit new_c.pkh in @@ -109,8 +118,16 @@ let test_transfer_fees_emptying_after_reveal_batched () = >>=? fun transaction -> Op.batch_operations ~source:new_contract (I inc) [reveal; transaction] >>=? fun op -> - Incremental.add_operation ~expect_apply_failure:(fun _ -> return_unit) inc op - >>=? fun _inc -> return_unit + let expect_apply_failure = function + | [ + Environment.Ecoproto_error (Contract_storage.Empty_implicit_contract pkh); + ] + when pkh = new_c.pkh -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_apply_failure inc op >>=? fun _inc -> + return_unit (* We assert that the changes introduced in !5182, splitting the application of Reveal operations into a pre-checking and @@ -312,7 +329,8 @@ let test_backtracked_reveal_in_batch () = Incremental.add_operation ~expect_apply_failure inc batched_operation >>=? fun inc -> (* We assert the manager key is still unrevealed, as the batch has failed *) - Context.Contract.is_manager_key_revealed (I inc) new_contract >>=? fun revelead -> + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> when_ revelead (fun () -> failwith "Unexpected contract revelation: reveal was expected to fail") @@ -321,9 +339,9 @@ let tests = Tztest.tztest "simple reveal" `Quick test_simple_reveal; Tztest.tztest "empty account on reveal" `Quick test_empty_account_on_reveal; Tztest.tztest - "not enough found for reveal" + "not enough funds for reveal" `Quick - test_not_enough_found_for_reveal; + test_not_enough_funds_for_reveal; Tztest.tztest "transfer fees emptying balance after reveal in batch" `Quick diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml index 1930ea3636e7..5ae39c8608a5 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_transfer.ml @@ -183,15 +183,33 @@ let test_missing_transaction () = let test_transfer_zero_implicit_with_bal_src_as_fee () = Context.init1 () >>=? fun (b, dest) -> let account = Account.new_account () in + let src_pkh = account.Account.pkh in Incremental.begin_construction b >>=? fun i -> - let src = Contract.Implicit account.Account.pkh in + let src = Contract.Implicit src_pkh in Op.transaction (I i) dest src (Tez.of_mutez_exn 100L) >>=? fun op -> Incremental.add_operation i op >>=? fun i -> Context.Contract.balance (I i) src >>=? fun bal_src -> Assert.equal_tez ~loc:__LOC__ bal_src (Tez.of_mutez_exn 100L) >>=? fun () -> Op.transaction (I i) ~fee:bal_src src dest Tez.zero >>=? fun op -> - Incremental.add_operation i op >>= fun res -> - Assert.proto_error_with_info ~loc:__LOC__ res "Empty transaction" + (* Transferring zero tez should result in an application failure as + the implicit contract has been depleted. *) + let expect_apply_failure = function + | [ + Environment.Ecoproto_error (Contract_storage.Empty_implicit_contract pkh); + ] + when pkh = src_pkh -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_apply_failure i op >>=? fun inc -> + Context.Contract.balance (I inc) src >>=? fun balance -> + (* We assert that the failing operation was included and that the + fees were taken, effectively depleting the contract. *) + Assert.equal_tez ~loc:__LOC__ balance Tez.zero >>=? fun () -> + (* Empty contracts should be unrevealed *) + Context.Contract.is_manager_key_revealed (I inc) src >>=? fun revelead -> + when_ revelead (fun () -> + Stdlib.failwith "Empty account still exists and is revealed.") (** Transfer zero tez to an originated contract, with fee equals balance of src. *) let test_transfer_zero_to_originated_with_bal_src_as_fee () = @@ -205,6 +223,8 @@ let test_transfer_zero_to_originated_with_bal_src_as_fee () = >>=? fun (op, new_contract) -> Incremental.add_operation i op >>=? fun i -> Context.Contract.balance (I i) src >>=? fun bal_src -> + Op.revelation (I i) ~fee:Tez.zero account.pk >>=? fun op -> + Incremental.add_operation i op >>=? fun i -> Op.transaction (I i) ~fee:bal_src src new_contract Tez.zero >>=? fun op -> Assert.equal_tez ~loc:__LOC__ bal_src (Tez.of_mutez_exn 100L) >>=? fun () -> Incremental.add_operation i op >>=? fun i -> @@ -220,6 +240,8 @@ let test_transfer_one_to_implicit_with_bal_src_as_fee () = Incremental.add_operation i op >>=? fun i -> Context.Contract.balance (I i) src >>=? fun bal_src -> Assert.equal_tez ~loc:__LOC__ bal_src (Tez.of_mutez_exn 100L) >>=? fun () -> + Op.revelation (I i) ~fee:Tez.zero account.pk >>=? fun op -> + Incremental.add_operation i op >>=? fun i -> Op.transaction (I i) ~fee:bal_src src dest Tez.one >>=? fun op -> Incremental.add_operation i op >>= fun res -> Assert.proto_error_with_info ~loc:__LOC__ res "Balance too low" -- GitLab From dda9f26a72627514808b42227b4b08e5b4c1c5db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Tue, 26 Apr 2022 17:09:42 +0200 Subject: [PATCH 09/13] Proto: enforce reveal operations position as first of batches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #2774. We enforce that public key reveals should only appear at the beginning of a batch of manager operations. Co-authored-by: Germán Delbianco Co-authored-by: vbot Co-authored-by: Diane Gallois-Wong --- docs/protocols/alpha.rst | 3 + src/proto_alpha/lib_protocol/apply.ml | 91 +++++++++++++++++--------- src/proto_alpha/lib_protocol/apply.mli | 1 + 3 files changed, 63 insertions(+), 32 deletions(-) diff --git a/docs/protocols/alpha.rst b/docs/protocols/alpha.rst index 1478803ac683..c24900574fef 100644 --- a/docs/protocols/alpha.rst +++ b/docs/protocols/alpha.rst @@ -20,6 +20,9 @@ Rollups supporting execution of smart contracts. (MRs :gl:`!4933`, :gl:`!4812`) Breaking Changes ---------------- +- Reveal operations can only occur at the head of a manager operation + batch (MR :gl:`!5182`). + RPC Changes ----------- diff --git a/src/proto_alpha/lib_protocol/apply.ml b/src/proto_alpha/lib_protocol/apply.ml index 460a3bd706a1..7d7351d04ab4 100644 --- a/src/proto_alpha/lib_protocol/apply.ml +++ b/src/proto_alpha/lib_protocol/apply.ml @@ -126,6 +126,7 @@ type error += | Failing_noop_error | Zero_frozen_deposits of Signature.Public_key_hash.t | Forbidden_zero_ticket_quantity + | Incorrect_reveal_position let () = register_error_kind @@ -774,7 +775,20 @@ let () = "It is not allowed to use a zero amount ticket in this operation." Data_encoding.empty (function Forbidden_zero_ticket_quantity -> Some () | _ -> None) - (fun () -> Forbidden_zero_ticket_quantity) + (fun () -> Forbidden_zero_ticket_quantity) ; + register_error_kind + `Permanent + ~id:"operations.incorrect_reveal_position" + ~title:"Incorrect reveal position" + ~description:"Incorrect reveal position in batch" + ~pp:(fun ppf () -> + Format.fprintf + ppf + "Incorrect reveal operation position in batch: only allowed in first \ + position") + Data_encoding.empty + (function Incorrect_reveal_position -> Some () | _ -> None) + (fun () -> Incorrect_reveal_position) open Apply_results @@ -2427,42 +2441,55 @@ let precheck_manager_contents_list ctxt contents_list ~mempool_mode = rec_precheck_manager_contents_list ctxt contents_list let find_manager_public_key ctxt (op : _ Kind.manager contents_list) = - (* Currently, the [op] only contains one signature, so - all operations are required to be from the same manager. This may - change in the future, allowing several managers to group-sign a - sequence of transactions. *) - let check_same_manager (source, source_key) manager = - match manager with - | None -> ok (source, source_key) - | Some (manager, manager_key) -> - if Signature.Public_key_hash.equal source manager then - (* Consistency will be checked by - [reveal_manager_key] in [precheck_manager_contents]. *) - ok (source, Option.either manager_key source_key) - else error Inconsistent_sources + (* Currently, the [op] batch contains only one signature, so all + operations in the batch are required to originate from the same + manager. This may change in the future, in order to allow several + managers to group-sign a sequence of operations. *) + (* Invariants checked: + + - Reveal operations are only authorized in the first position element of a batch. + + - All sources in a batch must be equal. *) + (* Performs a sanity check and return the operation's (single) + source and a potential public key if the batch contains a reveal + operation in the head position. *) + let rec check_batch_tail_sanity : + type kind. + public_key_hash -> kind Kind.manager contents_list -> unit tzresult = + fun expected_source -> function + | Single (Manager_operation {operation = Reveal _key; _}) -> + error Incorrect_reveal_position + | Cons (Manager_operation {operation = Reveal _key; _}, _res) -> + error Incorrect_reveal_position + | Single (Manager_operation {source; _}) -> + error_unless + (Signature.Public_key_hash.equal expected_source source) + Inconsistent_sources + | Cons (Manager_operation {source; _}, rest) -> + error_unless + (Signature.Public_key_hash.equal expected_source source) + Inconsistent_sources + >>? fun () -> check_batch_tail_sanity source rest in - let rec find_source : + let check_batch : type kind. kind Kind.manager contents_list -> - (Signature.public_key_hash * Signature.public_key option) option -> - (Signature.public_key_hash * Signature.public_key option) tzresult = - fun contents_list manager -> - let source (type kind) = function[@coq_match_with_default] - | (Manager_operation {source; operation = Reveal key; _} : - kind Kind.manager contents) -> - (source, Some key) - | Manager_operation {source; _} -> (source, None) - in - match contents_list with - | Single op -> check_same_manager (source op) manager - | Cons (op, rest) -> - check_same_manager (source op) manager >>? fun manager -> - find_source rest (Some manager) + (public_key_hash * public_key option) tzresult = + fun op -> + match op with + | Single (Manager_operation {source; operation = Reveal key; _}) -> + ok (source, Some key) + | Single (Manager_operation {source; _}) -> ok (source, None) + | Cons (Manager_operation {source; operation = Reveal key; _}, rest) -> + check_batch_tail_sanity source rest >>? fun () -> ok (source, Some key) + | Cons (Manager_operation {source; _}, rest) -> + check_batch_tail_sanity source rest >>? fun () -> ok (source, None) in - find_source op None >>?= fun (source, source_key) -> - match source_key with - | Some key -> return key + check_batch op >>?= fun (source, revealed_key) -> + Contract.must_be_allocated ctxt (Contract.Implicit source) >>=? fun () -> + match revealed_key with | None -> Contract.get_manager_key ctxt source + | Some pk -> return pk let check_manager_signature ctxt chain_id (op : _ Kind.manager contents_list) raw_operation = diff --git a/src/proto_alpha/lib_protocol/apply.mli b/src/proto_alpha/lib_protocol/apply.mli index 3e3a0688450a..d48cca5585b5 100644 --- a/src/proto_alpha/lib_protocol/apply.mli +++ b/src/proto_alpha/lib_protocol/apply.mli @@ -44,6 +44,7 @@ type error += | Sc_rollup_feature_disabled | Inconsistent_counters | Forbidden_zero_ticket_quantity + | Incorrect_reveal_position val begin_partial_construction : context -> -- GitLab From 50bd7abdc029bf69f345f286c8a92ebe3613e789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Tue, 3 May 2022 13:26:54 +0200 Subject: [PATCH 10/13] Tezts: fix reveal batches tezts for Alpha MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix #2774 Repeated reveal operations in batches should result in Apply.Incorrect_reveal_position, Permanent errors, with protos > Jakarta. Co-authored-by: Germán Delbianco Co-authored-by: Albin Coquereau --- tezt/tests/manager_operations.ml | 47 ++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/tezt/tests/manager_operations.ml b/tezt/tests/manager_operations.ml index 6069fce3b69e..8dbe9b2e998b 100644 --- a/tezt/tests/manager_operations.ml +++ b/tezt/tests/manager_operations.ml @@ -1079,7 +1079,8 @@ end module Reveal = struct (* This auxiliary function forges and injects a batched operation - made of two revelations pk1 and pk2. The tx is signed by the given key. *) + made of two revelations pk1 and pk2. The transaction is signed by + the given key. *) let mk_reveal_twice {client; _} key pk1 pk2 = let* cpt = Operation.get_counter client ~source:key in let s1 = {key with Account.public_key = pk1} in @@ -1139,22 +1140,42 @@ module Reveal = struct in unit - let revealed_twice_in_batch = + let revealed_twice_in_batch ~supports decide_error = Protocol.register_test ~__FILE__ ~title:"Correct public key revealed twice in a batch" ~tags:["reveal"; "revelation"; "batch"] + ~supports @@ fun protocol -> let* nodes = Helpers.init ~protocol () in let* key = Helpers.init_fresh_account ~protocol nodes ~amount ~fee in Log.section "Make the revelation" ; let* _ = - Memchecks.with_branch_refused_checks ~__LOC__ nodes @@ fun () -> + decide_error nodes @@ fun () -> mk_reveal_twice nodes.main key key.public_key key.public_key in let* () = Memchecks.check_balance ~__LOC__ nodes.main key amount in Memchecks.check_revealed ~__LOC__ nodes.main key ~revealed:false + (* After the work in !5182, which enforces that reveal operations + can only be placed at the head of the batch, this test should + fail with a permanent, Apply.Incorrect_reveal_position error (see + #2774). For Ithaca and Jakarta, we leave the original behaviour + which resulted in an Branch Refused error. *) + let revealed_twice_in_batch protocols = + revealed_twice_in_batch + ~supports:(Protocol.Until_protocol 13) + (Memchecks.with_branch_refused_checks ~__LOC__) + protocols ; + revealed_twice_in_batch + ~supports:(Protocol.From_protocol 14) + (Memchecks.with_refused_checks ~__LOC__) + protocols + + (* After the work in !5182, which enforces that reveal operations + can only be placed at the head of the batch, this test should + fail with a permanent, Apply.Incorrect_reveal_position error (see + #2774). *) let revealed_twice_in_batch_bad_first_key = Protocol.register_test ~__FILE__ @@ -1175,17 +1196,18 @@ module Reveal = struct let* () = Memchecks.check_balance ~__LOC__ nodes.main key amount in Memchecks.check_revealed ~__LOC__ nodes.main key ~revealed:false - let revealed_twice_in_batch_bad_second_key = + let revealed_twice_in_batch_bad_second_key ~supports decide_error = Protocol.register_test ~__FILE__ ~title:"Two reveals in a batch. Second key is wrong" ~tags:["reveal"; "revelation"] + ~supports @@ fun protocol -> let* nodes = Helpers.init ~protocol () in let* key = Helpers.init_fresh_account ~protocol nodes ~amount ~fee in Log.section "Make the revelation" ; let* _ = - Memchecks.with_branch_refused_checks ~__LOC__ nodes @@ fun () -> + decide_error nodes @@ fun () -> mk_reveal_twice nodes.main key @@ -1195,6 +1217,21 @@ module Reveal = struct let* () = Memchecks.check_balance ~__LOC__ nodes.main key amount in Memchecks.check_revealed ~__LOC__ nodes.main key ~revealed:false + (* After the work in !5182, which enforces that reveal operations + can only be placed at the head of the batch, this test should + fail with a permanent, Apply.Incorrect_reveal_position error (see + #2774). For Ithaca and Jakarta, we leave the original behaviour + which resulted in an Branch Refused error. *) + let revealed_twice_in_batch_bad_second_key protocols = + revealed_twice_in_batch_bad_second_key + ~supports:(Protocol.Until_protocol 13) + (Memchecks.with_branch_refused_checks ~__LOC__) + protocols ; + revealed_twice_in_batch_bad_second_key + ~supports:(Protocol.From_protocol 14) + (Memchecks.with_refused_checks ~__LOC__) + protocols + let register ~protocols = simple_reveal_bad_pk protocols ; simple_reveal_not_a_pk protocols ; -- GitLab From 152b115498cb8b0ac336c1e652ad89bc04f44059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Mon, 2 May 2022 18:55:14 +0200 Subject: [PATCH 11/13] Proto/Tests: test re-revealing fails We test that a batch attempting to re-reveal a manager will fail. --- .../integration/operations/test_reveal.ml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml index 8f50c4b63928..9a6480a2c758 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml @@ -334,6 +334,56 @@ let test_backtracked_reveal_in_batch () = when_ revelead (fun () -> failwith "Unexpected contract revelation: reveal was expected to fail") +(* Asserts that re-revealing an already revealed manager will make the + whole batch fail. *) +let test_already_revealed_manager_in_batch () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + (* Create the contract *) + Op.transaction (B blk) c new_contract Tez.one >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expecting fresh pkh" + | false -> ()) + >>=? fun () -> + (* Reveal the contract *) + Op.revelation (B blk) new_c.pk >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (* We pack a correct batch of operations attempting to re-reveal the contract *) + Incremental.begin_construction blk >>=? fun inc -> + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun op_reveal -> + Op.transaction + ~force_reveal:false + ~fee:Tez.zero + (I inc) + new_contract + new_contract + (Tez.of_mutez_exn 1_000_001L) + >>=? fun op_transfer -> + Op.batch_operations + ~recompute_counters:true + ~source:new_contract + (B blk) + [op_reveal; op_transfer] + >>=? fun batched_operation -> + let expect_apply_failure = function + | [ + Environment.Ecoproto_error + (Contract_manager_storage.Previously_revealed_key _); + ] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_apply_failure inc batched_operation + >>=? fun inc -> + (* We assert the manager key is still revealed. *) + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> + unless revelead (fun () -> + Stdlib.failwith + "Unexpected unrevelation: failing batch shouldn't unreveal the manager") + let tests = [ Tztest.tztest "simple reveal" `Quick test_simple_reveal; @@ -358,4 +408,8 @@ let tests = "a backtracked reveal in a batch doesn't take effect" `Quick test_backtracked_reveal_in_batch; + Tztest.tztest + "cannot re-reveal a manager in a batch" + `Quick + test_already_revealed_manager_in_batch; ] -- GitLab From 5b1ebd76c5780bda4973a359f594861cdcb8def4 Mon Sep 17 00:00:00 2001 From: vbot Date: Tue, 22 Mar 2022 12:40:47 +0100 Subject: [PATCH 12/13] Proto/Tests: test that gas-exhausted reveals don't take effect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (See #2386) Adds an integration protocol test asserting that a reveal operations that fails because of gas exhaution doesn't take effect. Co-authored-by: vbot Co-authored-by: Germán Delbianco --- .../integration/operations/test_reveal.ml | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml index 9a6480a2c758..cbadd93f7fd0 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml @@ -384,6 +384,62 @@ let test_already_revealed_manager_in_batch () = Stdlib.failwith "Unexpected unrevelation: failing batch shouldn't unreveal the manager") +(* cf: #2386 + + We imitate the behaviour of + + https://tzkt.io/ooSocfx3xxzDo7eFyGu6ZDR1svzMrbaJtBikQanXXhwrqMuWfGz + + which provides evidence of a failing reveal with a gas exhaustion + error due to an incorrect gas limit of 0, which still takes effect + as witnessed by the subsequent (reveal-less) transfer + + https://tzkt.io/opBQQJQ5senPP5v8PfPFf4uVQqKRE5RVjbwx8uD4SqeRs2JGcVw + + This showcases a bad separation of concerns between pre-checking + and the application of manager operations per-se within + [Protocol.Apply.apply_operation]. The situation originated because + [precheck_manager_contents_lists] would reveal the manager by + calling [Protocol.Alpha_context.Contract.reveal_manager_key] before + [prepare_apply_manager_operation_content] has consumed the declared + gas. + + With !5182 we have fixed this situation by revealing the manager + contract at application time. The following test isolates the + failing reveal and asserts that the manager is not revealed after + the failing op. *) +let test_no_reveal_when_gas_exhausted () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + (* Fund the contract with a sufficient balance *) + Op.transaction (B blk) c new_contract (Tez.of_mutez_exn 1_000L) + >>=? fun operation -> + (* Create the contract *) + Block.bake blk ~operation >>=? fun blk -> + (* Assert that the account has not been revealed yet *) + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + (* We craft a new (bad) reveal operation with a 0 gas_limit *) + Op.revelation ~fee:Tez.zero ~gas_limit:Gas.Arith.zero (B blk) new_c.pk + >>=? fun op -> + Incremental.begin_construction blk >>=? fun inc -> + (* The application of this operation is expected to fail with a + {! Protocol.Raw_context.Operation_quota_exceeded} error *) + let expect_apply_failure = function + | [Environment.Ecoproto_error Raw_context.Operation_quota_exceeded] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_apply_failure inc op >>=? fun inc -> + (* We assert the manager key is still unrevealed, as the operation has failed *) + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> + when_ revelead (fun () -> + failwith "Unexpected revelation: reveal operation failed") + let tests = [ Tztest.tztest "simple reveal" `Quick test_simple_reveal; @@ -412,4 +468,8 @@ let tests = "cannot re-reveal a manager in a batch" `Quick test_already_revealed_manager_in_batch; + Tztest.tztest + "do not reveal when gas exhausted" + `Quick + test_no_reveal_when_gas_exhausted; ] -- GitLab From d6296de09dda53ff65683c7357cbba171ab441ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Delbianco?= Date: Mon, 28 Mar 2022 17:02:30 +0200 Subject: [PATCH 13/13] Proto/Tests: add tests for incorrectly placed and duplicated reveals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cf #2774 We test that reveal ops succeed only when placed first in the batch. We extend the test suite to duplicate reveals in batches to show they fail even if the first one is well placed: First, we test a batch consisting of duplicate (potentially) valid reveal operations. We assert the second reveal to fail again with an [Apply.Incorrect_reveal_position] error, and for the first reveal to be backtracked. Then, we test batches with duplicate reveals which follow a failing one and we assert again the second reveal fails skipped. We do this for the 3 different reasons a well-placed reveal might fail (as tested above): gas exhaustion, insolvency, and emptying the balance while revealing. Co-authored-by: Germán Delbianco Co-authored-by: vbot --- .../integration/operations/test_reveal.ml | 242 ++++++++++++++++++ 1 file changed, 242 insertions(+) diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml index cbadd93f7fd0..ee3b1c24183e 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_reveal.ml @@ -440,6 +440,228 @@ let test_no_reveal_when_gas_exhausted () = when_ revelead (fun () -> failwith "Unexpected revelation: reveal operation failed") +(* Fix #2774 + + We test that reveals can only succeed if they are placed at the + first position in a batch of manager operations, and that moreover + reveal operations occur uniquely in batches. + + - First, [test_reveal_incorrect_position_in_batch] asserts that a + [[transfer; reveal]] batch where a valid reveal follows another + valid op (different from a reveal, so here a transfer) fails with + an [Apply.Incorrect_reveal_position] error. + + - Second, we test a batch consisting of duplicate (potentially) + valid reveal operations. We assert the second reveal to fail again + with an [Apply.Incorrect_reveal_position] error, and for the first + reveal to be backtracked. + + - Then, we test batches with duplicate reveals which follow a + failing one and we assert again the second reveal fails skipped. We + do this for the 3 different reasons a well-placed reveal might fail + (as tested above): gas exhaustion, insolvency, and emptying the + balance while revealing. +*) +let test_reveal_incorrect_position_in_batch () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + (* Create the contract *) + Op.transaction (B blk) c new_contract Tez.one >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + Incremental.begin_construction blk >>=? fun inc -> + Op.transaction + ~force_reveal:false + ~fee:Tez.zero + (I inc) + new_contract + new_contract + (Tez.of_mutez_exn 1L) + >>=? fun op_transfer -> + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun op_reveal -> + Op.batch_operations + ~recompute_counters:true + ~source:new_contract + (I inc) + [op_transfer; op_reveal] + >>=? fun batched_operation -> + let expect_failure = function + | [Environment.Ecoproto_error Apply.Incorrect_reveal_position] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_failure inc batched_operation + >>=? fun inc -> + (* We assert the manager key is still unrevealed, as the operation has failed *) + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> + when_ revelead (fun () -> + failwith "Unexpected revelation: reveal operation was expected to fail") + +(* Test that a batch [reveal pk; reveal pk] where the first reveal + succeeds but the second one results in the second one failing, and + then first reveal being backtracked. *) +let test_duplicate_valid_reveals () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + (* Create the contract *) + Op.transaction (B blk) c new_contract Tez.one >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + Incremental.begin_construction blk >>=? fun inc -> + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun op_rev1 -> + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun op_rev2 -> + Op.batch_operations + ~recompute_counters:true + ~source:new_contract + (I inc) + [op_rev1; op_rev2] + >>=? fun batched_operation -> + let expect_failure = function + | [Environment.Ecoproto_error Apply.Incorrect_reveal_position] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_failure inc batched_operation + >>=? fun inc -> + (* We assert the manager key is still unrevealed, as the operation has failed *) + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> + when_ revelead (fun () -> + failwith "Unexpected contract revelation: backtracking expected") + +(* Test that a batch [failed_reveal pk; reveal pk] where the first + reveal fails with a gas exhaustion results in the second one + failing due to not being well-placed at the beginnning of the + batch. *) +let test_valid_reveal_after_gas_exhausted_one () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + (* Create the contract *) + Op.transaction (B blk) c new_contract Tez.one >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + Incremental.begin_construction blk >>=? fun inc -> + (* We first craft a (bad) reveal operation with a 0 gas_limit *) + Op.revelation ~fee:Tez.zero ~gas_limit:Gas.Arith.zero (B blk) new_c.pk + >>=? fun bad_reveal -> + (* While the second is a valid one *) + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun good_reveal -> + Op.batch_operations + ~recompute_counters:true + ~source:new_contract + (I inc) + [bad_reveal; good_reveal] + >>=? fun batched_operation -> + let expect_failure = function + | [Environment.Ecoproto_error Apply.Incorrect_reveal_position] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_failure inc batched_operation + >>=? fun inc -> + (* We assert the manager key is still unrevealed, as the batch has failed *) + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> + when_ revelead (fun () -> + failwith "Unexpected contract revelation: no valid reveal in batch") + +(* Test that a batch [failed_reveal pk; reveal pk; transfer] where the + first reveal fails with insufficient funds results in the second + one failing due to not being well-placed at the beginnning of the + batch. We add the trailing transfer to ensure covering all branches + of `check_batch_tail_sanity` in `find_manager_public_key` when + calling {!Apply.check_manager_signature} to verify the manager's pk + while processing the second reveal. *) +let test_valid_reveal_after_insolvent_one () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + (* Create the contract *) + Op.transaction (B blk) c new_contract Tez.one >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation: expected fresh pkh" + | false -> ()) + >>=? fun () -> + Incremental.begin_construction blk >>=? fun inc -> + (* We first craft an insolvent reveal operation *) + Op.revelation ~fee:ten_tez (B blk) new_c.pk >>=? fun bad_reveal -> + (* While the second is a free valid one *) + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun good_reveal -> + Op.transaction ~fee:Tez.zero (I inc) new_contract c Tez.one + >>=? fun transfer -> + Op.batch_operations + ~recompute_counters:true + ~source:new_contract + (I inc) + [bad_reveal; good_reveal; transfer] + >>=? fun batched_operation -> + let expect_failure = function + | [Environment.Ecoproto_error Apply.Incorrect_reveal_position] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_failure inc batched_operation + >>=? fun inc -> + (* We assert the manager key is still unrevealed, as the batch has failed *) + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> + when_ revelead (fun () -> + failwith "Unexpected contract revelation: no valid reveal in batch") + +(* Test that a batch [failed_reveal pk; reveal pk] where the first + reveal fails with insufficient funds results in the second one + failing due to not being well-placed at the beginnning of the + batch. *) +let test_valid_reveal_after_emptying_balance () = + Context.init1 ~consensus_threshold:0 () >>=? fun (blk, c) -> + let new_c = Account.new_account () in + let new_contract = Contract.Implicit new_c.pkh in + let amount = Tez.one_mutez in + (* Create the contract *) + Op.transaction (B blk) c new_contract amount >>=? fun operation -> + Block.bake blk ~operation >>=? fun blk -> + (Context.Contract.is_manager_key_revealed (B blk) new_contract >|=? function + | true -> Stdlib.failwith "Unexpected revelation" + | false -> ()) + >>=? fun () -> + Incremental.begin_construction blk >>=? fun inc -> + (* Reveal the contract, spending all its balance in fees *) + Op.revelation ~fee:amount (B blk) new_c.pk >>=? fun bad_reveal -> + (* While the second is a free valid one *) + Op.revelation ~fee:Tez.zero (I inc) new_c.pk >>=? fun good_reveal -> + Op.batch_operations + ~recompute_counters:true + ~source:new_contract + (I inc) + [bad_reveal; good_reveal] + >>=? fun batched_operation -> + let expect_failure = function + | [Environment.Ecoproto_error Apply.Incorrect_reveal_position] -> + return_unit + | _ -> assert false + in + Incremental.add_operation ~expect_failure inc batched_operation + >>=? fun inc -> + (* We assert the manager key is still unrevealed, as the batch has failed *) + Context.Contract.is_manager_key_revealed (I inc) new_contract + >>=? fun revelead -> + when_ revelead (fun () -> + failwith "Unexpected contract revelation: no valid reveal in batch") + let tests = [ Tztest.tztest "simple reveal" `Quick test_simple_reveal; @@ -472,4 +694,24 @@ let tests = "do not reveal when gas exhausted" `Quick test_no_reveal_when_gas_exhausted; + Tztest.tztest + "incorrect reveal position in batch" + `Quick + test_reveal_incorrect_position_in_batch; + Tztest.tztest + "cannot duplicate valid reveals in batch" + `Quick + test_duplicate_valid_reveals; + Tztest.tztest + "cannot batch a good reveal after a gas-exhausted one" + `Quick + test_valid_reveal_after_gas_exhausted_one; + Tztest.tztest + "cannot batch a good reveal after an insolvent one" + `Quick + test_valid_reveal_after_insolvent_one; + Tztest.tztest + "cannot batch a good reveal after one emptying account" + `Quick + test_valid_reveal_after_emptying_balance; ] -- GitLab