From fc524e93306e65350f167f13af53458b685cd5a5 Mon Sep 17 00:00:00 2001 From: "iguerNL@Functori" Date: Thu, 17 Mar 2022 16:44:22 +0100 Subject: [PATCH 1/2] Proto/Tx-rollup: bound the number of withdrawals per L2 message --- src/proto_alpha/bin_tx_rollup_node/daemon.ml | 6 +-- src/proto_alpha/bin_tx_rollup_node/state.ml | 15 +++++- src/proto_alpha/bin_tx_rollup_node/state.mli | 1 + src/proto_alpha/lib_client/mockup.ml | 14 ++++- .../lib_parameters/default_parameters.ml | 4 ++ .../lib_protocol/alpha_context.mli | 3 ++ .../lib_protocol/constants_repr.ml | 7 ++- .../lib_protocol/constants_repr.mli | 2 + .../lib_protocol/constants_storage.ml | 4 ++ .../lib_protocol/constants_storage.mli | 2 + src/proto_alpha/lib_protocol/raw_context.ml | 1 + .../test/unit/test_tx_rollup_l2_apply.ml | 51 +++++++++++-------- .../lib_protocol/tx_rollup_l2_apply.ml | 49 ++++++++++++++---- .../lib_protocol/tx_rollup_l2_apply.mli | 17 +++++-- tests_python/tests_alpha/test_mockup.py | 1 + tezt/_regressions/rpc/alpha.client.others.out | 1 + tezt/_regressions/rpc/alpha.light.others.out | 1 + tezt/_regressions/rpc/alpha.proxy.others.out | 1 + .../rpc/alpha.proxy_server.others.out | 1 + 19 files changed, 139 insertions(+), 42 deletions(-) diff --git a/src/proto_alpha/bin_tx_rollup_node/daemon.ml b/src/proto_alpha/bin_tx_rollup_node/daemon.ml index d7241266c1e1..07df2de32bee 100644 --- a/src/proto_alpha/bin_tx_rollup_node/daemon.ml +++ b/src/proto_alpha/bin_tx_rollup_node/daemon.ml @@ -38,12 +38,12 @@ let checkout_context (state : State.t) ctxt_hash = let+ context = Context.checkout state.context_index ctxt_hash in Option.to_result ~none:[Tx_rollup_cannot_checkout_context ctxt_hash] context -let interp_messages ctxt messages cumulated_size = +let interp_messages ctxt parameters messages cumulated_size = let open Lwt_syntax in let+ (ctxt, _ctxt_hash, rev_contents) = List.fold_left_s (fun (ctxt, ctxt_hash, acc) message -> - let+ apply_res = Apply.apply_message ctxt message in + let+ apply_res = Apply.apply_message ctxt parameters message in let (ctxt, ctxt_hash, result) = match apply_res with | Ok (ctxt, result) -> @@ -268,7 +268,7 @@ let process_messages_and_inboxes (state : State.t) | Some context -> return context in let*! (context, inbox) = - interp_messages predecessor_context messages cumulated_size + interp_messages predecessor_context state.parameters messages cumulated_size in match inbox with | None -> diff --git a/src/proto_alpha/bin_tx_rollup_node/state.ml b/src/proto_alpha/bin_tx_rollup_node/state.ml index 10ee3266903b..5fbbb0bc31bc 100644 --- a/src/proto_alpha/bin_tx_rollup_node/state.ml +++ b/src/proto_alpha/bin_tx_rollup_node/state.ml @@ -37,6 +37,7 @@ type t = { context_index : Context.index; rollup : Tx_rollup.t; rollup_origination : rollup_origination; + parameters : Protocol.Tx_rollup_l2_apply.parameters; } (* Stands for the manager operation pass, in which the rollup transactions are @@ -198,10 +199,22 @@ let init_context ~data_dir = let*! index = Context.init (Node_data.context_dir data_dir) in return index +let init_parameters cctxt = + let open Lwt_result_syntax in + let* {parametric; _} = + Protocol.Constants_services.all cctxt (cctxt#chain, cctxt#block) + in + return + { + Protocol.Tx_rollup_l2_apply.tx_rollup_max_withdrawals_per_batch = + parametric.tx_rollup_max_withdrawals_per_batch; + } + let init ~data_dir ~context ?rollup_genesis rollup = let open Lwt_result_syntax in let store_orig = init_store ~data_dir ~context ?rollup_genesis rollup in let context_index = init_context ~data_dir in let* (store, rollup_origination) = store_orig in let* context_index = context_index in - return {store; context_index; rollup; rollup_origination} + let* parameters = init_parameters context in + return {store; context_index; rollup; rollup_origination; parameters} diff --git a/src/proto_alpha/bin_tx_rollup_node/state.mli b/src/proto_alpha/bin_tx_rollup_node/state.mli index ece18c9290d1..bcab229514b3 100644 --- a/src/proto_alpha/bin_tx_rollup_node/state.mli +++ b/src/proto_alpha/bin_tx_rollup_node/state.mli @@ -38,6 +38,7 @@ type t = private { context_index : Context.index; rollup : Tx_rollup.t; rollup_origination : rollup_origination; + parameters : Protocol.Tx_rollup_l2_apply.parameters; } (** [init ~data_dir ~context ~rollup_genesis rollup] creates a new state for the diff --git a/src/proto_alpha/lib_client/mockup.ml b/src/proto_alpha/lib_client/mockup.ml index 703507fa2694..1954e48e5525 100644 --- a/src/proto_alpha/lib_client/mockup.ml +++ b/src/proto_alpha/lib_client/mockup.ml @@ -76,6 +76,7 @@ module Protocol_constants_overrides = struct tx_rollup_origination_size : int option; tx_rollup_hard_size_limit_per_inbox : int option; tx_rollup_hard_size_limit_per_message : int option; + tx_rollup_max_withdrawals_per_batch : int option; tx_rollup_commitment_bond : Tez.t option; tx_rollup_finality_period : int option; tx_rollup_max_unfinalized_levels : int option; @@ -140,6 +141,7 @@ module Protocol_constants_overrides = struct c.tx_rollup_origination_size, c.tx_rollup_hard_size_limit_per_inbox, c.tx_rollup_hard_size_limit_per_message, + c.tx_rollup_max_withdrawals_per_batch, c.tx_rollup_commitment_bond, c.tx_rollup_finality_period, c.tx_rollup_max_unfinalized_levels, @@ -193,6 +195,7 @@ module Protocol_constants_overrides = struct tx_rollup_origination_size, tx_rollup_hard_size_limit_per_inbox, tx_rollup_hard_size_limit_per_message, + tx_rollup_max_withdrawals_per_batch, tx_rollup_commitment_bond, tx_rollup_finality_period, tx_rollup_max_unfinalized_levels, @@ -244,6 +247,7 @@ module Protocol_constants_overrides = struct tx_rollup_origination_size; tx_rollup_hard_size_limit_per_inbox; tx_rollup_hard_size_limit_per_message; + tx_rollup_max_withdrawals_per_batch; tx_rollup_commitment_bond; tx_rollup_finality_period; tx_rollup_max_unfinalized_levels; @@ -313,13 +317,14 @@ module Protocol_constants_overrides = struct (opt "cache_sampler_state_cycles" int8)) (merge_objs (merge_objs - (obj9 + (obj10 (opt "tx_rollup_enable" Data_encoding.bool) (opt "tx_rollup_origination_size" int31) (opt "tx_rollup_hard_size_limit_per_inbox" int31) (opt "tx_rollup_hard_size_limit_per_message" int31) + (opt "tx_rollup_max_withdrawals_per_batch" int31) (opt "tx_rollup_commitment_bond" Tez.encoding) (opt "tx_rollup_finality_period" int31) (opt "tx_rollup_max_unfinalized_levels" int31) @@ -401,6 +406,8 @@ module Protocol_constants_overrides = struct Some parametric.tx_rollup_hard_size_limit_per_inbox; tx_rollup_hard_size_limit_per_message = Some parametric.tx_rollup_hard_size_limit_per_message; + tx_rollup_max_withdrawals_per_batch = + Some parametric.tx_rollup_max_withdrawals_per_batch; tx_rollup_commitment_bond = Some parametric.tx_rollup_commitment_bond; tx_rollup_finality_period = Some parametric.tx_rollup_finality_period; tx_rollup_max_unfinalized_levels = @@ -467,6 +474,7 @@ module Protocol_constants_overrides = struct tx_rollup_origination_size = None; tx_rollup_hard_size_limit_per_inbox = None; tx_rollup_hard_size_limit_per_message = None; + tx_rollup_max_withdrawals_per_batch = None; tx_rollup_commitment_bond = None; tx_rollup_finality_period = None; tx_rollup_max_unfinalized_levels = None; @@ -896,6 +904,10 @@ module Protocol_constants_overrides = struct Option.value ~default:c.tx_rollup_hard_size_limit_per_message o.tx_rollup_hard_size_limit_per_message; + tx_rollup_max_withdrawals_per_batch = + Option.value + ~default:c.tx_rollup_max_withdrawals_per_batch + o.tx_rollup_max_withdrawals_per_batch; tx_rollup_commitment_bond = Option.value ~default:c.tx_rollup_commitment_bond diff --git a/src/proto_alpha/lib_parameters/default_parameters.ml b/src/proto_alpha/lib_parameters/default_parameters.ml index 3743a583c9ff..875646612a45 100644 --- a/src/proto_alpha/lib_parameters/default_parameters.ml +++ b/src/proto_alpha/lib_parameters/default_parameters.ml @@ -108,6 +108,10 @@ let constants_mainnet = (* Transaction rollup’s size limits are expressed in number of bytes *) tx_rollup_hard_size_limit_per_inbox = 100_000; tx_rollup_hard_size_limit_per_message = 5_000; + (* We limit the number of withdraws per message to avoid costly + allocations/iterations in the accounting mechanism used for each + withdraw claiming in L1 and cleaned when removing a commitment. *) + tx_rollup_max_withdrawals_per_batch = 255; tx_rollup_commitment_bond = Tez.of_mutez_exn 10_000_000_000L; tx_rollup_finality_period = 2_000; tx_rollup_max_unfinalized_levels = 2_100; diff --git a/src/proto_alpha/lib_protocol/alpha_context.mli b/src/proto_alpha/lib_protocol/alpha_context.mli index de8ca9109773..cf947263bc82 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/alpha_context.mli @@ -795,6 +795,7 @@ module Constants : sig tx_rollup_origination_size : int; tx_rollup_hard_size_limit_per_inbox : int; tx_rollup_hard_size_limit_per_message : int; + tx_rollup_max_withdrawals_per_batch : int; tx_rollup_commitment_bond : Tez.t; tx_rollup_finality_period : int; tx_rollup_withdraw_period : int; @@ -895,6 +896,8 @@ module Constants : sig val tx_rollup_hard_size_limit_per_message : context -> int + val tx_rollup_max_withdrawals_per_batch : context -> int + val tx_rollup_commitment_bond : context -> Tez.t val tx_rollup_finality_period : context -> int diff --git a/src/proto_alpha/lib_protocol/constants_repr.ml b/src/proto_alpha/lib_protocol/constants_repr.ml index 51d7cd88fb68..c00b6e21c20f 100644 --- a/src/proto_alpha/lib_protocol/constants_repr.ml +++ b/src/proto_alpha/lib_protocol/constants_repr.ml @@ -163,6 +163,7 @@ type parametric = { tx_rollup_origination_size : int; tx_rollup_hard_size_limit_per_inbox : int; tx_rollup_hard_size_limit_per_message : int; + tx_rollup_max_withdrawals_per_batch : int; tx_rollup_commitment_bond : Tez_repr.t; tx_rollup_finality_period : int; tx_rollup_withdraw_period : int; @@ -220,6 +221,7 @@ let parametric_encoding = c.tx_rollup_origination_size, c.tx_rollup_hard_size_limit_per_inbox, c.tx_rollup_hard_size_limit_per_message, + c.tx_rollup_max_withdrawals_per_batch, c.tx_rollup_commitment_bond, c.tx_rollup_finality_period, c.tx_rollup_withdraw_period, @@ -271,6 +273,7 @@ let parametric_encoding = tx_rollup_origination_size, tx_rollup_hard_size_limit_per_inbox, tx_rollup_hard_size_limit_per_message, + tx_rollup_max_withdrawals_per_batch, tx_rollup_commitment_bond, tx_rollup_finality_period, tx_rollup_withdraw_period, @@ -323,6 +326,7 @@ let parametric_encoding = tx_rollup_origination_size; tx_rollup_hard_size_limit_per_inbox; tx_rollup_hard_size_limit_per_message; + tx_rollup_max_withdrawals_per_batch; tx_rollup_commitment_bond; tx_rollup_finality_period; tx_rollup_withdraw_period; @@ -389,11 +393,12 @@ let parametric_encoding = (req "cache_sampler_state_cycles" int8)) (merge_objs (merge_objs - (obj9 + (obj10 (req "tx_rollup_enable" bool) (req "tx_rollup_origination_size" int31) (req "tx_rollup_hard_size_limit_per_inbox" int31) (req "tx_rollup_hard_size_limit_per_message" int31) + (req "tx_rollup_max_withdrawals_per_batch" int31) (req "tx_rollup_commitment_bond" Tez_repr.encoding) (req "tx_rollup_finality_period" int31) (req "tx_rollup_withdraw_period" int31) diff --git a/src/proto_alpha/lib_protocol/constants_repr.mli b/src/proto_alpha/lib_protocol/constants_repr.mli index eb60dc7b2e63..88341a113902 100644 --- a/src/proto_alpha/lib_protocol/constants_repr.mli +++ b/src/proto_alpha/lib_protocol/constants_repr.mli @@ -128,6 +128,8 @@ type parametric = { tx_rollup_hard_size_limit_per_inbox : int; (* the maximum amount of bytes one batch can allocate in an inbox *) tx_rollup_hard_size_limit_per_message : int; + (* the maximum number of allowed "L2-to-L1" withdraws per batch *) + tx_rollup_max_withdrawals_per_batch : int; (* the amount of tez to bond a tx rollup commitment *) tx_rollup_commitment_bond : Tez_repr.t; (* the number of blocks before a tx rollup block is final *) diff --git a/src/proto_alpha/lib_protocol/constants_storage.ml b/src/proto_alpha/lib_protocol/constants_storage.ml index 361611fd2bc3..b431575429dd 100644 --- a/src/proto_alpha/lib_protocol/constants_storage.ml +++ b/src/proto_alpha/lib_protocol/constants_storage.ml @@ -162,6 +162,10 @@ let tx_rollup_hard_size_limit_per_message c = let constants = Raw_context.constants c in constants.tx_rollup_hard_size_limit_per_message +let tx_rollup_max_withdrawals_per_batch c = + let constants = Raw_context.constants c in + constants.tx_rollup_max_withdrawals_per_batch + let tx_rollup_commitment_bond c = let constants = Raw_context.constants c in constants.tx_rollup_commitment_bond diff --git a/src/proto_alpha/lib_protocol/constants_storage.mli b/src/proto_alpha/lib_protocol/constants_storage.mli index ad3fbb1f1006..0ac4f5b3e20e 100644 --- a/src/proto_alpha/lib_protocol/constants_storage.mli +++ b/src/proto_alpha/lib_protocol/constants_storage.mli @@ -92,6 +92,8 @@ val tx_rollup_hard_size_limit_per_inbox : Raw_context.t -> int val tx_rollup_hard_size_limit_per_message : Raw_context.t -> int +val tx_rollup_max_withdrawals_per_batch : Raw_context.t -> int + val tx_rollup_commitment_bond : Raw_context.t -> Tez_repr.t val tx_rollup_finality_period : Raw_context.t -> int diff --git a/src/proto_alpha/lib_protocol/raw_context.ml b/src/proto_alpha/lib_protocol/raw_context.ml index ed71e6e70be2..8ff9fe29f32a 100644 --- a/src/proto_alpha/lib_protocol/raw_context.ml +++ b/src/proto_alpha/lib_protocol/raw_context.ml @@ -904,6 +904,7 @@ let prepare_first_block ~level ~timestamp ctxt = tx_rollup_origination_size = 60_000; tx_rollup_hard_size_limit_per_inbox = 100_000; tx_rollup_hard_size_limit_per_message = 5_000; + tx_rollup_max_withdrawals_per_batch = 255; tx_rollup_commitment_bond = Tez_repr.of_mutez_exn 10_000_000_000L; tx_rollup_finality_period = 2_000; tx_rollup_withdraw_period; diff --git a/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml b/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml index 537e71ebb9a3..070143c9c94b 100644 --- a/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml +++ b/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml @@ -419,6 +419,14 @@ let test_returned_deposit () = | (Deposit_success _result, _) -> fail_msg "Did not expect overflowing deposit to be succesful" +let apply_l2_parameters : Protocol.Tx_rollup_l2_apply.parameters = + {maximum_withdraws_per_message = 5} + +let apply_l2_batch ctxt batch = + Batch_V1.apply_batch ctxt apply_l2_parameters batch + +let apply_l2_message ctxt msg = apply_message ctxt apply_l2_parameters msg + (** Test that all values used in a transaction creates indexes and they are packed in the final indexes. *) let test_indexes_creation () = @@ -493,7 +501,7 @@ let test_indexes_creation () = in let* (_ctxt, Batch_result {indexes; _}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let* () = @@ -550,7 +558,7 @@ let test_indexes_creation_bad () = in let* (ctxt, Batch_result {results; indexes}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in (* Only the indexes from the second transaction should exist, the first @@ -598,7 +606,7 @@ let test_simple_l2_transaction () = let batch = create_batch_v1 [transaction] [[sk1; sk2]] in let* (ctxt, Batch_result {results; _}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 |> snd in @@ -668,7 +676,7 @@ let test_l2_transaction_l2_addr_signer_good () = let signature = sign_transaction [sk1] transfer in let batch = batch signature [transfer] in let* (_ctxt, Batch_result {results; indexes = _}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 in match status with @@ -691,7 +699,7 @@ let test_l2_transaction_l2_addr_signer_bad () = let* () = expect_error ~msg_if_valid:"The check should fail with an unknown address" - (Batch_V1.apply_batch ctxt batch) + (apply_l2_batch ctxt batch) (Tx_rollup_l2_apply.Unknown_address addr1) in (* Now we add the index but the metadata is still missing *) @@ -699,7 +707,7 @@ let test_l2_transaction_l2_addr_signer_bad () = let* () = expect_error ~msg_if_valid:"The check should fail with unknown metadata" - (Batch_V1.apply_batch ctxt batch) + (apply_l2_batch ctxt batch) (Tx_rollup_l2_apply.Unallocated_metadata 0l) in (* Finally we add the metadata and the test pass *) @@ -709,7 +717,7 @@ let test_l2_transaction_l2_addr_signer_bad () = Ticket_ledger.credit ctxt tidx idx1 (Tx_rollup_l2_qty.of_int64_exn 100L) in let* (_ctxt, Batch_result {results; indexes = _}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 in match status with @@ -738,7 +746,7 @@ let test_simple_l1_transaction () = let batch = create_batch_v1 [transaction] [[sk1]] in let* (ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 |> snd in @@ -789,7 +797,7 @@ let test_l1_transaction_inexistant_ticket () = let batch = create_batch_v1 [transaction] [[sk1]] in let* (_ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in (* Expect no withdrawals *) @@ -829,7 +837,7 @@ let test_l1_transaction_inexistant_signer () = let batch = create_batch_v1 [transaction] [[sk_unknown]] in let* (_ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in (* Expect no withdrawals *) @@ -871,7 +879,7 @@ let test_l1_transaction_overdraft () = let batch = create_batch_v1 [transaction] [[sk1]] in let* (ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in (* Expect no withdrawals *) @@ -953,7 +961,7 @@ let test_l1_transaction_zero () = let batch = create_batch_v1 [transaction] [[sk1]] in let* (ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in (* Expect one zero-withdrawal *) @@ -1037,7 +1045,7 @@ let test_l1_transaction_partial () = let batch = create_batch_v1 [transaction] [[sk1]] in let* (ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in (* Expect one partial withdrawal *) @@ -1171,7 +1179,7 @@ let test_transaction_with_unknown_indexable () = let batch = batch signatures [transaction] in let* (ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 |> snd in @@ -1252,7 +1260,7 @@ let test_invalid_transaction () = let batch = create_batch_v1 [transaction] [[sk1; sk2]] in let* (ctxt, Batch_result {results; _}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 |> snd in @@ -1301,7 +1309,7 @@ let test_invalid_counter () = let batch = create_batch_v1 [transaction] [[sk1]] in let* (_ctxt, Batch_result {results; _}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 |> snd in @@ -1341,7 +1349,7 @@ let test_update_counter () = in let* (ctxt, Batch_result {results; _}, withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 |> snd in @@ -1439,7 +1447,7 @@ let test_apply_message_batch () = (V1 batch)) in - let* (_ctxt, result) = apply_message ctxt msg in + let* (_ctxt, result) = apply_l2_message ctxt msg in match result with | (Message_result.Batch_V1_result _, []) -> @@ -1512,7 +1520,7 @@ let test_apply_message_batch_withdrawals () = (V1 batch)) in - let* (ctxt, result) = apply_message ctxt msg in + let* (ctxt, result) = apply_l2_message ctxt msg in match result with | ( Message_result.Batch_V1_result @@ -1625,7 +1633,7 @@ let test_apply_message_deposit () = (Tx_rollup_l2_qty.of_int64_exn amount) in - let* (_ctxt, result) = apply_message ctxt msg in + let* (_ctxt, result) = apply_l2_message ctxt msg in match result with | (Message_result.Deposit_result _, []) -> @@ -1637,7 +1645,6 @@ let test_apply_message_deposit () = let test_transfer_to_self () = let open Context_l2.Syntax in let* (ctxt, _, accounts) = with_initial_setup [ticket1] [[(ticket1, 10L)]] in - let (sk1, pk1, addr1, _idx1, _) = nth_exn accounts 0 in let transaction = [transfer ~signer:(signer_pk pk1) ~dest:addr1 ~ticket:ticket1 1L] @@ -1645,7 +1652,7 @@ let test_transfer_to_self () = let batch = create_batch_v1 [transaction] [[sk1]] in let* (_ctxt, Batch_result {results; _}, _withdrawals) = - Batch_V1.apply_batch ctxt batch + apply_l2_batch ctxt batch in let status = nth_exn results 0 in diff --git a/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.ml b/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.ml index 3a89a64b3b36..802df653f703 100644 --- a/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.ml +++ b/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.ml @@ -45,6 +45,7 @@ type error += | Unknown_address of Tx_rollup_l2_address.t | Invalid_self_transfer | Invalid_zero_transfer + | Maximum_withdraws_per_message_exceeded of {current : int; maximum : int} let () = let open Data_encoding in @@ -162,7 +163,21 @@ let () = ~description:"A transfer's amount must be greater than zero." empty (function Invalid_zero_transfer -> Some () | _ -> None) - (function () -> Invalid_zero_transfer) + (function () -> Invalid_zero_transfer) ; + (* Maximum_withdraws_per_message_exceeded *) + register_error_kind + `Permanent + ~id:"tx_rollup_maximum_withdraws_per_message_exceeded" + ~title:"Maximum tx-rollup withdraws per message exceeded" + ~description: + "The maximum number of withdraws allowed per tx-rollup message exceeded" + (obj2 (req "current" int31) (req "limit" int31)) + (function + | Maximum_withdraws_per_message_exceeded {current; maximum} -> + Some (current, maximum) + | _ -> None) + (fun (current, maximum) -> + Maximum_withdraws_per_message_exceeded {current; maximum}) module Address_indexes = Map.Make (Tx_rollup_l2_address) module Ticket_indexes = Map.Make (Ticket_hash) @@ -323,6 +338,11 @@ module Message_result = struct tup2 message_result_encoding (list Tx_rollup_withdraw.encoding)) end +type parameters = { + (* Maximum number of allowed L2-to-L1 withdraws per batch *) + tx_rollup_max_withdrawals_per_batch : int; +} + module Make (Context : CONTEXT) = struct open Context open Syntax @@ -746,10 +766,11 @@ module Make (Context : CONTEXT) = struct let apply_batch : ctxt -> + parameters -> (Indexable.unknown, Indexable.unknown) t -> (ctxt * Message_result.Batch_V1.t * Tx_rollup_withdraw.withdrawal list) m = - fun ctxt batch -> + fun ctxt parameters batch -> let* (ctxt, indexes, batch) = check_signature ctxt batch in let {contents; _} = batch in let* (ctxt, indexes, rev_results, withdrawals) = @@ -767,11 +788,17 @@ module Make (Context : CONTEXT) = struct (ctxt, indexes, [], []) contents in - let results = List.rev rev_results in - return - ( ctxt, - Message_result.Batch_V1.Batch_result {results; indexes}, - withdrawals ) + let limit = parameters.tx_rollup_max_withdrawals_per_batch in + if Compare.List_length_with.(withdrawals > limit) then + fail + (Maximum_withdraws_per_message_exceeded + {current = List.length withdrawals; maximum = limit}) + else + let results = List.rev rev_results in + return + ( ctxt, + Message_result.Batch_V1.Batch_result {results; indexes}, + withdrawals ) end let apply_deposit : @@ -804,9 +831,9 @@ module Make (Context : CONTEXT) = struct in return (initial_ctxt, Deposit_failure reason, Some withdrawal)) - let apply_message : ctxt -> Tx_rollup_message.t -> (ctxt * Message_result.t) m - = - fun ctxt msg -> + let apply_message : + ctxt -> parameters -> Tx_rollup_message.t -> (ctxt * Message_result.t) m = + fun ctxt parameters msg -> let open Tx_rollup_message in match msg with | Deposit deposit -> @@ -819,7 +846,7 @@ module Make (Context : CONTEXT) = struct match batch with | Some (V1 batch) -> let* (ctxt, result, withdrawals) = - Batch_V1.apply_batch ctxt batch + Batch_V1.apply_batch ctxt parameters batch in return (ctxt, (Batch_V1_result result, withdrawals)) | None -> fail Invalid_batch_encoding) diff --git a/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.mli b/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.mli index 1f922a43e6bc..f8f6bdb7eb9f 100644 --- a/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.mli +++ b/src/proto_alpha/lib_protocol/tx_rollup_l2_apply.mli @@ -64,6 +64,7 @@ type error += | Unknown_address of Tx_rollup_l2_address.t | Invalid_self_transfer | Invalid_zero_transfer + | Maximum_withdraws_per_message_exceeded of {current : int; maximum : int} module Address_indexes : Map.S with type key = Tx_rollup_l2_address.t @@ -122,6 +123,12 @@ module Message_result : sig val encoding : t Data_encoding.t end +(** The record of parameters used during the application of messages. *) +type parameters = { + (* Maximum number of allowed L2-to-L1 withdraws per batch *) + tx_rollup_max_withdrawals_per_batch : int; +} + module Make (Context : CONTEXT) : sig open Context @@ -133,7 +140,8 @@ module Make (Context : CONTEXT) : sig module Batch_V1 : sig open Tx_rollup_l2_batch.V1 - (** [apply_batch ctxt batch] interprets the batch {Tx_rollup_l2_batch.V1.t}. + (** [apply_batch ctxt parameters batch] interprets the batch + {Tx_rollup_l2_batch.V1.t}. By construction, a failing transaction will not affect the [ctxt] and other transactions will still be interpreted. @@ -151,6 +159,7 @@ module Make (Context : CONTEXT) : sig *) val apply_batch : ctxt -> + parameters -> (Indexable.unknown, Indexable.unknown) t -> (ctxt * Message_result.Batch_V1.t * Tx_rollup_withdraw.withdrawal list) m @@ -204,7 +213,8 @@ module Make (Context : CONTEXT) : sig * Tx_rollup_withdraw.withdrawal option) m - (** [apply_message ctxt message] interpets the [message] in the [ctxt]. + (** [apply_message ctxt parameters message] interprets the [message] in the + [ctxt]. That is, @@ -221,7 +231,8 @@ module Make (Context : CONTEXT) : sig The list of withdrawals in the message result followed the ordering of the contents in the message. *) - val apply_message : ctxt -> Tx_rollup_message.t -> (ctxt * Message_result.t) m + val apply_message : + ctxt -> parameters -> Tx_rollup_message.t -> (ctxt * Message_result.t) m end module Internal_for_tests : sig diff --git a/tests_python/tests_alpha/test_mockup.py b/tests_python/tests_alpha/test_mockup.py index 07e742a2c0b4..8797d055309d 100644 --- a/tests_python/tests_alpha/test_mockup.py +++ b/tests_python/tests_alpha/test_mockup.py @@ -664,6 +664,7 @@ def _test_create_mockup_init_show_roundtrip( "tx_rollup_withdraw_period": 123456, "tx_rollup_max_unfinalized_levels": 2100, "tx_rollup_max_messages_per_inbox": 1010, + 'tx_rollup_max_withdrawals_per_batch': 255, "tx_rollup_max_finalized_levels": 60_100, "tx_rollup_cost_per_byte_ema_factor": 321, "tx_rollup_max_ticket_payload_size": 10_240, diff --git a/tezt/_regressions/rpc/alpha.client.others.out b/tezt/_regressions/rpc/alpha.client.others.out index 842ec3b9649d..79289afef72b 100644 --- a/tezt/_regressions/rpc/alpha.client.others.out +++ b/tezt/_regressions/rpc/alpha.client.others.out @@ -32,6 +32,7 @@ tezt/_regressions/rpc/alpha.client.others.out "tx_rollup_enable": false, "tx_rollup_origination_size": 60000, "tx_rollup_hard_size_limit_per_inbox": 100000, "tx_rollup_hard_size_limit_per_message": 5000, + "tx_rollup_max_withdrawals_per_batch": 255, "tx_rollup_commitment_bond": "10000000000", "tx_rollup_finality_period": 2000, "tx_rollup_withdraw_period": 60000, "tx_rollup_max_unfinalized_levels": 2100, diff --git a/tezt/_regressions/rpc/alpha.light.others.out b/tezt/_regressions/rpc/alpha.light.others.out index b844d3f937bb..58f933c90d94 100644 --- a/tezt/_regressions/rpc/alpha.light.others.out +++ b/tezt/_regressions/rpc/alpha.light.others.out @@ -32,6 +32,7 @@ tezt/_regressions/rpc/alpha.light.others.out "tx_rollup_enable": false, "tx_rollup_origination_size": 60000, "tx_rollup_hard_size_limit_per_inbox": 100000, "tx_rollup_hard_size_limit_per_message": 5000, + "tx_rollup_max_withdrawals_per_batch": 255, "tx_rollup_commitment_bond": "10000000000", "tx_rollup_finality_period": 2000, "tx_rollup_withdraw_period": 60000, "tx_rollup_max_unfinalized_levels": 2100, diff --git a/tezt/_regressions/rpc/alpha.proxy.others.out b/tezt/_regressions/rpc/alpha.proxy.others.out index 248e7b17758c..d581518fc2ab 100644 --- a/tezt/_regressions/rpc/alpha.proxy.others.out +++ b/tezt/_regressions/rpc/alpha.proxy.others.out @@ -32,6 +32,7 @@ tezt/_regressions/rpc/alpha.proxy.others.out "tx_rollup_enable": false, "tx_rollup_origination_size": 60000, "tx_rollup_hard_size_limit_per_inbox": 100000, "tx_rollup_hard_size_limit_per_message": 5000, + "tx_rollup_max_withdrawals_per_batch": 255, "tx_rollup_commitment_bond": "10000000000", "tx_rollup_finality_period": 2000, "tx_rollup_withdraw_period": 60000, "tx_rollup_max_unfinalized_levels": 2100, diff --git a/tezt/_regressions/rpc/alpha.proxy_server.others.out b/tezt/_regressions/rpc/alpha.proxy_server.others.out index 1c21198355c9..3434898536f6 100644 --- a/tezt/_regressions/rpc/alpha.proxy_server.others.out +++ b/tezt/_regressions/rpc/alpha.proxy_server.others.out @@ -32,6 +32,7 @@ tezt/_regressions/rpc/alpha.proxy_server.others.out "tx_rollup_enable": false, "tx_rollup_origination_size": 60000, "tx_rollup_hard_size_limit_per_inbox": 100000, "tx_rollup_hard_size_limit_per_message": 5000, + "tx_rollup_max_withdrawals_per_batch": 255, "tx_rollup_commitment_bond": "10000000000", "tx_rollup_finality_period": 2000, "tx_rollup_withdraw_period": 60000, "tx_rollup_max_unfinalized_levels": 2100, -- GitLab From c9dcc5550ef496f8ceb5537b7f44fee494a2422e Mon Sep 17 00:00:00 2001 From: "iguerNL@Functori" Date: Fri, 18 Mar 2022 11:28:39 +0100 Subject: [PATCH 2/2] Tests/Tx_rollup: add units tests for maximum withdrawals per batch --- .../test/unit/test_tx_rollup_l2_apply.ml | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml b/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml index 070143c9c94b..afa0144e7f65 100644 --- a/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml +++ b/src/proto_alpha/lib_protocol/test/unit/test_tx_rollup_l2_apply.ml @@ -420,7 +420,7 @@ let test_returned_deposit () = fail_msg "Did not expect overflowing deposit to be succesful" let apply_l2_parameters : Protocol.Tx_rollup_l2_apply.parameters = - {maximum_withdraws_per_message = 5} + {tx_rollup_max_withdrawals_per_batch = 5} let apply_l2_batch ctxt batch = Batch_V1.apply_batch ctxt apply_l2_parameters batch @@ -779,6 +779,71 @@ let test_simple_l1_transaction () = | (Transaction_success, _) -> fail_msg "Expected exactly one withdrawal" | (Transaction_failure _, _) -> fail_msg "The transaction should be a success" +let rec repeat n f acc = if n <= 0 then acc else repeat (n - 1) f (f n acc) + +(** This function crafts a batch containing [nb_withdraws]. Then, it applies it + on an L2 context, and checks the status, depending on the value of + [should_succeed] *) +let helper_test_withdrawal_limits_per_batch nb_withdraws ~should_succeed = + let open Context_l2.Syntax in + (* create sufficiently many accounts *) + let accounts = repeat nb_withdraws (fun _i l -> [(ticket1, 2L)] :: l) [] in + let* (ctxt, _tidxs, accounts) = + with_initial_setup [ticket1] ([] :: accounts) + in + (* destination of withdrawals *) + let (_skD, _pkD, _addrD, _idxD, pkhD) = nth_exn accounts 0 in + (* transfer 1 ticket from [nb_withdraws] accounts to the dest *) + let (transactions, sks) = + repeat + nb_withdraws + (fun i (transactions, sks) -> + let (sk, pk, _addr, _idx, _pkh) = nth_exn accounts i in + let withdraw = + withdraw ~signer:(signer_pk pk) ~dest:pkhD ~ticket:ticket1 1L + in + (withdraw :: transactions, sk :: sks)) + ([], []) + in + let batch = create_batch_v1 [transactions] [sks] in + (* apply the batch, and handle the success and error cases *) + Irmin_storage.Syntax.catch + (apply_l2_batch ctxt batch) + (fun _success -> + if should_succeed then return_unit + else fail_msg "The transaction should fail") + (fun error -> + let expected_error = "Maximum tx-rollup withdraws per message exceeded" in + let ({title = error_title; _} as _error_info) = + Error_monad.find_info_of_error (Environment.wrap_tzerror error) + in + if should_succeed then + fail_msg + ("The transaction should be a success, but failed: " ^ error_title) + else if error_title <> expected_error then + fail_msg + @@ Format.sprintf + "Expected error %s but got %s" + expected_error + error_title + else return_unit) + +(* Three tests that use the helper above *) +let nb_withdrawals_per_batch_below_limit () = + helper_test_withdrawal_limits_per_batch + (apply_l2_parameters.tx_rollup_max_withdrawals_per_batch - 1) + ~should_succeed:true + +let nb_withdrawals_per_batch_equals_limit () = + helper_test_withdrawal_limits_per_batch + apply_l2_parameters.tx_rollup_max_withdrawals_per_batch + ~should_succeed:true + +let nb_withdrawals_per_batch_above_limit () = + helper_test_withdrawal_limits_per_batch + (apply_l2_parameters.tx_rollup_max_withdrawals_per_batch + 1) + ~should_succeed:false + (** Test that [Missing_ticket] is raised if a transfer is attempted to a ticket absent from the rollup. *) let test_l1_transaction_inexistant_ticket () = @@ -1696,4 +1761,10 @@ let tests = test_apply_message_batch_withdrawals ); ("apply deposit from message", test_apply_message_deposit); ("test transfer to self fail", test_transfer_to_self); + ( "nb withdrawals per batch below limit", + nb_withdrawals_per_batch_below_limit ); + ( "nb withdrawals per batch equals limit", + nb_withdrawals_per_batch_equals_limit ); + ( "nb withdrawals per batch above limit", + nb_withdrawals_per_batch_above_limit ); ] -- GitLab