From d030ee6f58153f0535859348502e5a4911645b9e Mon Sep 17 00:00:00 2001 From: Thomas Letan Date: Tue, 10 May 2022 16:08:25 +0200 Subject: [PATCH 1/2] Proto,tests: Check operation size limit by default in [Incremental] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This test can be opt-out explicitly, and sometimes it’s OKay. But when designing protocol extensions that are explicitly limited by the size of Tezos operations (like Transaction Rollup was), it makes sense to systematically check the limits. The good news is: tests of TORU still pass with this change, modulo a slight change to allow to submit large batches of layer-2 operations in one layer-1 batch. --- src/proto_alpha/lib_protocol/test/helpers/incremental.ml | 2 +- .../test/integration/operations/test_tx_rollup.ml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/incremental.ml b/src/proto_alpha/lib_protocol/test/helpers/incremental.ml index 98e4722928cb..4657e9431d05 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/incremental.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/incremental.ml @@ -157,7 +157,7 @@ let detect_script_failure : in fun {contents} -> detect_script_failure contents -let add_operation ?expect_apply_failure ?expect_failure ?(check_size = false) st +let add_operation ?expect_apply_failure ?expect_failure ?(check_size = true) st op = let open Apply_results in (if check_size then 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 e5b8eb330795..9aa4d718d665 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 @@ -3492,7 +3492,7 @@ module Rejection = struct let ops = List.repeat message_count op1 in Op.combine_operations ~source:account (I i) ([op1; op2] @ ops) >>=? fun op -> - Incremental.add_operation i op >>=? fun i -> + Incremental.add_operation ~check_size:false i op >>=? fun i -> Incremental.finalize_block i >>=? fun b -> (* Prepare a commitment for the tx_level 0, only the very first message result hash will be a real value (in order to start the proof verification @@ -3559,7 +3559,7 @@ module Rejection = struct Incremental.begin_construction b >>=? fun i -> (* Finally, we reject the commitment and check that the size fits in a Tezos operation. *) - Incremental.add_operation ~check_size:true i op >>=? fun _i -> return_unit + Incremental.add_operation i op >>=? fun _i -> return_unit let tests = [ -- GitLab From f265c0e5ad1ef86b0707136f772f0eddfed91fc6 Mon Sep 17 00:00:00 2001 From: Thomas Letan Date: Tue, 10 May 2022 16:16:24 +0200 Subject: [PATCH 2/2] Proto,tests: Add an optional argument [check_size] to [Block.bake] The development of Transaction Rollups has proven that sometimes, it is mandatory to check that the arguments of a layer-1 operations actually fit in a Tezos operation. This patch improves the [Block.bake] helpers function to do just that. The [check_size] optional argument is set to [true] by default, but can be opt-out explicitly. --- .../lib_protocol/test/helpers/block.ml | 24 +++++++++++++++---- .../lib_protocol/test/helpers/block.mli | 6 ++++- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/helpers/block.ml b/src/proto_alpha/lib_protocol/test/helpers/block.ml index b9b21b799c24..3687342b17dd 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/block.ml +++ b/src/proto_alpha/lib_protocol/test/helpers/block.ml @@ -619,8 +619,8 @@ let get_construction_vstate ?(policy = By_round 0) ?timestamp () >|= Environment.wrap_tzresult -let apply_with_metadata ?(policy = By_round 0) ~baking_mode header - ?(operations = []) pred = +let apply_with_metadata ?(policy = By_round 0) ?(check_size = true) ~baking_mode + header ?(operations = []) pred = let open Environment.Error_monad in ( (match baking_mode with | Application -> @@ -639,6 +639,18 @@ let apply_with_metadata ?(policy = By_round 0) ~baking_mode header >>=? fun vstate -> List.fold_left_es (fun vstate op -> + (if check_size then + let operation_size = + Data_encoding.Binary.length Operation.encoding op + in + if operation_size > Constants_repr.max_operation_data_length then + raise + (invalid_arg + (Format.sprintf + "The operation size is %d, it exceeds the constant maximum \ + size %d" + operation_size + Constants_repr.max_operation_data_length))) ; apply_operation vstate op >|= Environment.wrap_tzresult >|=? fun (state, _result) -> state) vstate @@ -655,7 +667,7 @@ let apply header ?(operations = []) pred = >>=? fun (t, _metadata) -> return t let bake_with_metadata ?locked_round ?policy ?timestamp ?operation ?operations - ?payload_round ~baking_mode ?liquidity_baking_toggle_vote pred = + ?payload_round ?check_size ~baking_mode ?liquidity_baking_toggle_vote pred = let operations = match (operation, operations) with | (Some op, Some ops) -> Some (op :: ops) @@ -673,10 +685,11 @@ let bake_with_metadata ?locked_round ?policy ?timestamp ?operation ?operations pred >>=? fun header -> Forge.sign_header header >>=? fun header -> - apply_with_metadata ?policy ~baking_mode header ?operations pred + apply_with_metadata ?policy ?check_size ~baking_mode header ?operations pred let bake ?(baking_mode = Application) ?payload_round ?locked_round ?policy - ?timestamp ?operation ?operations ?liquidity_baking_toggle_vote pred = + ?timestamp ?operation ?operations ?liquidity_baking_toggle_vote ?check_size + pred = bake_with_metadata ?payload_round ~baking_mode @@ -686,6 +699,7 @@ let bake ?(baking_mode = Application) ?payload_round ?locked_round ?policy ?operation ?operations ?liquidity_baking_toggle_vote + ?check_size pred >>=? fun (t, (_metadata : block_header_metadata)) -> return t diff --git a/src/proto_alpha/lib_protocol/test/helpers/block.mli b/src/proto_alpha/lib_protocol/test/helpers/block.mli index ad43ff797e26..0c2f2fc4ec41 100644 --- a/src/proto_alpha/lib_protocol/test/helpers/block.mli +++ b/src/proto_alpha/lib_protocol/test/helpers/block.mli @@ -172,7 +172,10 @@ val apply : (** [bake b] returns a block [b'] which has as predecessor block [b]. - Optional parameter [policy] allows to pick the next baker in several ways. + Optional parameter [policy] allows to pick the next baker in + several ways. If [check_size] is [true] (the default case), then + the function checks that the operations passed as arguments satisfy + the size limit of Tezos operations, as defined in the protocol. This function bundles together [forge_header], [sign_header] and [apply]. These functions should be used instead of bake to craft unusual blocks for testing together with setters for properties of the headers. @@ -187,6 +190,7 @@ val bake : ?operation:Operation.packed -> ?operations:Operation.packed list -> ?liquidity_baking_toggle_vote:Liquidity_baking.liquidity_baking_toggle_vote -> + ?check_size:bool -> t -> t tzresult Lwt.t -- GitLab