diff --git a/docs/protocols/alpha.rst b/docs/protocols/alpha.rst index 42fd99f70506b68c2f87a6586dfac50089e61b3b..401e40e25793149b4d6eb3d37bdb78adeda98ecd 100644 --- a/docs/protocols/alpha.rst +++ b/docs/protocols/alpha.rst @@ -63,6 +63,9 @@ Bug Fixes precheck even though they obviously do not have enough gas to apply the external operation, e.g. when `gas_limit = 0`. (MR :gl:`!5506`) +- Emptying an implicit account does not cost extra-gas anymore. (MR + :gl:`!5566`) + Minor Changes ------------- diff --git a/src/proto_alpha/lib_protocol/contract_storage.ml b/src/proto_alpha/lib_protocol/contract_storage.ml index 39c197670228d5474a39a2c2459e71f00c73dc1b..74da5c46a739aed821b1863cc8275bc5477cac9e 100644 --- a/src/proto_alpha/lib_protocol/contract_storage.ml +++ b/src/proto_alpha/lib_protocol/contract_storage.ml @@ -435,15 +435,17 @@ let delete c contract = (* For non implicit contract Big_map should be cleared *) failwith "Non implicit contracts cannot be removed" | Implicit _ -> + (* Implicit contract do not have: [Code], [Storage], + [Paid_storage_space] and [Used_storage_space]. We do not need + to delete them. Futhermore, these storages space are + carbonated, thus, require gas to be deleted (even when they + do not exist). An implicit contract deletion should not cost + extra gas. *) Contract_delegate_storage.unlink c contract >>=? fun c -> Storage.Contract.Spendable_balance.remove_existing c contract >>=? fun c -> Contract_manager_storage.remove_existing c contract >>=? fun c -> - Storage.Contract.Counter.remove_existing c contract >>=? fun c -> - Storage.Contract.Code.remove c contract >>=? fun (c, _, _) -> - Storage.Contract.Storage.remove c contract >>=? fun (c, _, _) -> - Storage.Contract.Paid_storage_space.remove c contract >>= fun c -> - Storage.Contract.Used_storage_space.remove c contract >|= ok + Storage.Contract.Counter.remove_existing c contract let allocated c contract = Storage.Contract.Spendable_balance.mem c contract diff --git a/src/proto_alpha/lib_protocol/test/integration/gas/test_gas_levels.ml b/src/proto_alpha/lib_protocol/test/integration/gas/test_gas_levels.ml index 568a88ba29551f45e5c0f6ee065d1ffc95103529..b9248c38be8ac0f3c0db228217fcf4411c27a00a 100644 --- a/src/proto_alpha/lib_protocol/test/integration/gas/test_gas_levels.ml +++ b/src/proto_alpha/lib_protocol/test/integration/gas/test_gas_levels.ml @@ -443,12 +443,12 @@ let test_block_mixed_operations () = >>=? fun (_block, consumed_gas, gas_limit_total) -> check_consumed_gas consumed_gas gas_limit_total -(** Test that emptying an account costs gas *) +(** Test that emptying an account does not cost extra-gas *) let test_emptying_account_gas () = let open Alpha_context in Context.init1 ~consensus_threshold:0 () >>=? fun (b, bootstrap) -> + let bootstrap_pkh = Context.Contract.pkh bootstrap in let {Account.pkh; pk; _} = Account.new_account () in - let {Account.pkh = pkh'; _} = Account.new_account () in let contract = Contract.Implicit pkh in let amount = Test_tez.of_int 10 in Op.transaction (B b) bootstrap contract amount >>=? fun op1 -> @@ -460,22 +460,22 @@ let test_emptying_account_gas () = (Gas.Arith.integral_of_int_exn Michelson_v1_gas.Internal_for_tests.int_cost_of_manager_operation) in - Op.delegation ~fee:amount ~gas_limit (B b) contract (Some pkh') >>=? fun op -> + Op.delegation ~fee:amount ~gas_limit (B b) contract (Some bootstrap_pkh) + >>=? fun op -> Incremental.begin_construction b >>=? fun i -> - (* The delegation operation should not be valid as the operation - effect would be to remove [contract] from the ledger and this - generates an extra gas cost. - - This semantics is not expected: see - https://gitlab.com/tezos/tezos/-/issues/3188 *) - Incremental.add_operation - ~expect_failure:(function - | [Environment.Ecoproto_error Raw_context.Operation_quota_exceeded] -> - return_unit - | _err -> assert false) - i - op - >>=? fun _i -> return_unit + (* The delegation operation should be valid as the operation effect + would be to remove [contract] and should not generate any extra + gas cost. *) + let expect_apply_failure = function + | [Environment.Ecoproto_error (Storage_error (Raw_context.Missing_key _))] + -> + (* The delegation is expected to fail in the apply part as the + contract was emptied when fees were retrieved. *) + return_unit + | err -> failwith "got unexpected error: %a" pp_print_trace err + in + Incremental.add_operation ~expect_apply_failure i op >>=? fun _i -> + return_unit let quick (what, how) = Tztest.tztest what `Quick how diff --git a/src/proto_alpha/lib_protocol/test/integration/operations/test_manager_operation_precheck.ml b/src/proto_alpha/lib_protocol/test/integration/operations/test_manager_operation_precheck.ml index f13c4796e8903cbc0e10cfd9aa3fd14de3d626e7..611f0bde4eb9170038a07f7634ef3a75d8c7305f 100644 --- a/src/proto_alpha/lib_protocol/test/integration/operations/test_manager_operation_precheck.ml +++ b/src/proto_alpha/lib_protocol/test/integration/operations/test_manager_operation_precheck.ml @@ -457,12 +457,26 @@ let emptying_undelegated_implicit_diagnostic (infos : infos) op = in apply_ko_diagnostic infos op expect_failure +(* Minimum gas cost to pass the precheck: + - cost_of_manager_operation for the generic part + - 100 (empiric) for the specific part (script decoding or hash costs) *) +let empiric_minimal_gas_cost_for_precheck = + Gas.Arith.integral_of_int_exn + (Michelson_v1_gas.Internal_for_tests.int_cost_of_manager_operation + 100) + let test_emptying_undelegated_implicit kind () = let open Lwt_result_syntax in let* infos = init_context () in let fee = Tez.one in + let gas_limit = Op.Custom_gas empiric_minimal_gas_cost_for_precheck in let* op = - select_op ~fee ~force_reveal:true ~source:infos.contract1 kind infos + select_op + ~fee + ~gas_limit + ~force_reveal:true + ~source:infos.contract1 + kind + infos in emptying_undelegated_implicit_diagnostic infos op