From 855f4cddb1a12d05932b771583df34b5ff4245ad Mon Sep 17 00:00:00 2001 From: Thomas Athorne Date: Fri, 10 Jun 2022 18:32:19 +0100 Subject: [PATCH 1/2] Proto: SCORU: zero-tick commitment case --- .../lib_protocol/sc_rollup_errors.ml | 30 ++++++++++++++++--- .../lib_protocol/sc_rollup_game_repr.ml | 19 ++++++++---- .../lib_protocol/sc_rollup_game_repr.mli | 5 +++- .../lib_protocol/sc_rollup_stake_storage.ml | 27 ++++++++++++++--- ...f inbox in the rollup node (too_many_m.out | 5 +--- 5 files changed, 67 insertions(+), 19 deletions(-) diff --git a/src/proto_alpha/lib_protocol/sc_rollup_errors.ml b/src/proto_alpha/lib_protocol/sc_rollup_errors.ml index 19e181bef9db..c4b932e3ef0a 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_errors.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_errors.ml @@ -56,6 +56,7 @@ type error += | (* `Temporary *) Sc_rollup_invalid_outbox_message_index | (* `Temporary *) Sc_rollup_outbox_level_expired | (* `Temporary *) Sc_rollup_outbox_message_already_applied + | (* `Temporary *) Sc_rollup_state_change_on_zero_tick_commitment | (* `Temporary *) Sc_rollup_staker_funds_too_low of { staker : Signature.public_key_hash; @@ -73,11 +74,13 @@ type error += (Raw_level_repr.t * Raw_level_repr.t) let () = + let description = "Maximum number of available messages reached" in register_error_kind `Temporary ~id:"Sc_rollup_max_number_of_available_messages_reached" ~title:"Maximum number of available messages reached" - ~description:"Maximum number of available messages reached" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) Data_encoding.unit (function | Sc_rollup_max_number_of_available_messages_reached -> Some () @@ -144,11 +147,13 @@ let () = ]) (function Sc_rollup_staker_in_game x -> Some x | _ -> None) (fun x -> Sc_rollup_staker_in_game x) ; + let description = "Attempt to timeout game too early" in register_error_kind `Temporary ~id:"Sc_rollup_timeout_level_not_reached" ~title:"Attempt to timeout game too early" - ~description:"Attempt to timeout game too early" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) Data_encoding.unit (function Sc_rollup_timeout_level_not_reached -> Some () | _ -> None) (fun () -> Sc_rollup_timeout_level_not_reached) ; @@ -174,19 +179,25 @@ let () = Data_encoding.unit (function Sc_rollup_no_game -> Some () | _ -> None) (fun () -> Sc_rollup_no_game) ; + let description = "Attempt to play move but not staker's turn" in register_error_kind `Temporary ~id:"Sc_rollup_wrong_turn" ~title:"Attempt to play move but not staker's turn" - ~description:"Attempt to play move but not staker's turn" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) Data_encoding.unit (function Sc_rollup_wrong_turn -> Some () | _ -> None) (fun () -> Sc_rollup_wrong_turn) ; + let description = + "Maximum number of messages reached for commitment period" + in register_error_kind `Temporary ~id:"Sc_rollup_max_number_of_messages_reached_for_commitment_period" ~title:"Maximum number of messages reached for commitment period" - ~description:"Maximum number of messages reached for commitment period" + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) Data_encoding.unit (function | Sc_rollup_max_number_of_messages_reached_for_commitment_period -> @@ -376,6 +387,17 @@ let () = Data_encoding.empty (function Sc_rollup_outbox_message_already_applied -> Some () | _ -> None) (fun () -> Sc_rollup_outbox_message_already_applied) ; + let description = "Attempt to commit zero ticks with state change" in + register_error_kind + `Temporary + ~id:"Sc_rollup_state_change_on_zero_tick_commitment" + ~title:description + ~description + ~pp:(fun ppf () -> Format.fprintf ppf "%s" description) + Data_encoding.empty + (function + | Sc_rollup_state_change_on_zero_tick_commitment -> Some () | _ -> None) + (fun () -> Sc_rollup_state_change_on_zero_tick_commitment) ; register_error_kind `Temporary ~id:"Sc_rollup_staker_funds_too_low" diff --git a/src/proto_alpha/lib_protocol/sc_rollup_game_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_game_repr.ml index 2d0c8abe41e1..982e4832e571 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_game_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_game_repr.ml @@ -230,18 +230,25 @@ let initial inbox ~pvm_name ~(parent : Sc_rollup_commitment_repr.t) ~(child : Sc_rollup_commitment_repr.t) ~refuter ~defender = let ({alice; _} : Index.t) = Index.make refuter defender in let alice_to_play = Staker.equal alice refuter in - let tick = Sc_rollup_tick_repr.of_number_of_ticks child.number_of_ticks in + let open Sc_rollup_tick_repr in + let tick = of_number_of_ticks child.number_of_ticks in { turn = (if alice_to_play then Alice else Bob); inbox_snapshot = inbox; level = child.inbox_level; pvm_name; dissection = - [ - make_chunk (Some parent.compressed_state) Sc_rollup_tick_repr.initial; - make_chunk (Some child.compressed_state) tick; - make_chunk None (Sc_rollup_tick_repr.next tick); - ]; + (if equal tick initial then + [ + make_chunk (Some child.compressed_state) initial; + make_chunk None (next initial); + ] + else + [ + make_chunk (Some parent.compressed_state) initial; + make_chunk (Some child.compressed_state) tick; + make_chunk None (next tick); + ]); } type step = diff --git a/src/proto_alpha/lib_protocol/sc_rollup_game_repr.mli b/src/proto_alpha/lib_protocol/sc_rollup_game_repr.mli index 3092e5f93b4e..d5f73ed9c99f 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_game_repr.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_game_repr.mli @@ -162,7 +162,10 @@ module V1 : sig Invariants: ----------- - - [dissection] must contain at least 3 values + - [dissection] must contain at least 2 values (normally it will be 32 + values, but smaller if there isn't enough space for a dissection + that size. The initial game dissection will be 3 values except in + the case of a zero-tick commit when it will have 2 values.) - the first state hash value in [dissection] must not be [None] - [inbox_snapshot] never changes once the game is created *) diff --git a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml index 54b3b33d0720..b7a25fcae81d 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml @@ -169,17 +169,36 @@ let assert_commitment_period ctxt rollup commitment = in return ctxt +let assert_same_hash_as_predecessor ctxt rollup (commitment : Commitment.t) = + let open Lwt_tzresult_syntax in + let* pred, ctxt = + Commitment_storage.get_commitment_unsafe ctxt rollup commitment.predecessor + in + if + Sc_rollup_repr.State_hash.equal + pred.compressed_state + commitment.compressed_state + then return ctxt + else fail Sc_rollup_state_change_on_zero_tick_commitment + (** Check invariants on [inbox_level], enforcing overallocation of storage and regularity of block production. - The constants used by [assert_refine_conditions_met] must be chosen such - that the maximum cost of storage allocated by each staker is at most the size - of their deposit. + The constants used by [assert_refine_conditions_met] must be chosen such + that the maximum cost of storage allocated by each staker is at most the size + of their deposit. *) let assert_refine_conditions_met ctxt rollup lcc commitment = let open Lwt_tzresult_syntax in let* ctxt = assert_commitment_not_too_far_ahead ctxt rollup lcc commitment in - assert_commitment_period ctxt rollup commitment + let* ctxt = assert_commitment_period ctxt rollup commitment in + if + Int32.equal + (Sc_rollup_repr.Number_of_ticks.to_int32 + Commitment.(commitment.number_of_ticks)) + 0l + then assert_same_hash_as_predecessor ctxt rollup commitment + else return ctxt let get_commitment_stake_count ctxt rollup node = let open Lwt_tzresult_syntax in diff --git a/tezt/tests/expected/sc_rollup.ml/Alpha- observing the correct maintenance of inbox in the rollup node (too_many_m.out b/tezt/tests/expected/sc_rollup.ml/Alpha- observing the correct maintenance of inbox in the rollup node (too_many_m.out index 9d957bd08d7e..923def4442d9 100644 --- a/tezt/tests/expected/sc_rollup.ml/Alpha- observing the correct maintenance of inbox in the rollup node (too_many_m.out +++ b/tezt/tests/expected/sc_rollup.ml/Alpha- observing the correct maintenance of inbox in the rollup node (too_many_m.out @@ -834,7 +834,4 @@ This simulation failed: This operation FAILED. Error: - { "id": - "proto.alpha.Sc_rollup_max_number_of_messages_reached_for_commitment_period", - "description": "Maximum number of messages reached for commitment period", - "data": {} } + Maximum number of messages reached for commitment period -- GitLab From b413566a7223af0d1495f9db10e14413fe34b5b2 Mon Sep 17 00:00:00 2001 From: Thomas Athorne Date: Fri, 10 Jun 2022 18:32:51 +0100 Subject: [PATCH 2/2] Proto: SCORU: add test for zero-tick commitment check --- .../test/unit/test_sc_rollup_storage.ml | 51 +++++++++++++++++++ .../Alpha- consecutive commitments.out | 8 +-- tezt/tests/sc_rollup.ml | 2 +- 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml b/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml index 6b010c74b61d..165e49f55f23 100644 --- a/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml +++ b/src/proto_alpha/lib_protocol/test/unit/test_sc_rollup_storage.ml @@ -2340,6 +2340,53 @@ let test_concurrent_refinement_cement () = in assert_commitment_hash_equal ~loc:__LOC__ ctxt c1 c2 +let test_zero_tick_commitment_cannot_change_state () = + let* ctxt, rollup, genesis_hash, staker = + originate_rollup_and_deposit_with_one_staker () + in + let level = valid_inbox_level ctxt in + let commitment = + Commitment_repr. + { + predecessor = genesis_hash; + inbox_level = level 1l; + number_of_messages = number_of_messages_exn 3l; + number_of_ticks = number_of_ticks_exn 1232909l; + compressed_state = Sc_rollup_repr.State_hash.zero; + } + in + let* c1, _level, ctxt = + lift + @@ Sc_rollup_stake_storage.Internal_for_tests.refine_stake + ctxt + rollup + staker + commitment + in + let commitment = + Commitment_repr. + { + predecessor = c1; + inbox_level = level 2l; + number_of_messages = number_of_messages_exn 0l; + number_of_ticks = number_of_ticks_exn 0l; + compressed_state = + Sc_rollup_repr.State_hash.context_hash_to_state_hash + (Context_hash.hash_string ["wxyz"]); + } + in + let* () = + assert_fails_with + ~loc:__LOC__ + (Sc_rollup_stake_storage.Internal_for_tests.refine_stake + ctxt + rollup + staker + commitment) + Sc_rollup_errors.Sc_rollup_state_change_on_zero_tick_commitment + in + assert_true ctxt + let check_gas_consumed ~since ~until = let open Raw_context in let as_cost = Gas_limit_repr.cost_of_gas @@ gas_consumed ~since ~until in @@ -3059,6 +3106,10 @@ let tests = "Refinement operations are commutative (cement)" `Quick test_concurrent_refinement_cement; + Tztest.tztest + "A commitment with zero ticks shouldn't change the state" + `Quick + test_zero_tick_commitment_cannot_change_state; Tztest.tztest "Retrieval of in-memory message inbox consumes gas" `Quick diff --git a/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out b/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out index 0ea5d7784185..1d5e61040091 100644 --- a/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out +++ b/tezt/tests/expected/sc_rollup.ml/Alpha- consecutive commitments.out @@ -30,7 +30,7 @@ This sequence of operations was run: storage fees ........................... +ꜩ1.6565 -./tezos-client --wait none publish commitment from '[PUBLIC_KEY_HASH]' for sc rollup '[SC_ROLLUP_HASH]' with compressed state scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf at inbox level 32 and predecessor '[SC_ROLLUP_COMMITMENT_HASH]' and number of messages 0 and number of ticks 0 +./tezos-client --wait none publish commitment from '[PUBLIC_KEY_HASH]' for sc rollup '[SC_ROLLUP_HASH]' with compressed state scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf at inbox level 32 and predecessor '[SC_ROLLUP_COMMITMENT_HASH]' and number of messages 0 and number of ticks 1 Node is bootstrapped. Estimated gas: 6251.140 units (will add 100 for safety) Estimated storage: no bytes added @@ -55,7 +55,7 @@ This sequence of operations was run: inbox_level: 32 predecessor: [SC_ROLLUP_COMMITMENT_HASH] number_of_messages: 0 - number_of_ticks: 0 in the smart contract rollup at address [SC_ROLLUP_HASH] + number_of_ticks: 1 in the smart contract rollup at address [SC_ROLLUP_HASH] This smart contract rollup commitment publishing was successfully applied Consumed gas: 6251.140 Hash of commit: [SC_ROLLUP_COMMITMENT_HASH] @@ -65,7 +65,7 @@ This sequence of operations was run: Frozen_bonds([PUBLIC_KEY_HASH],[SC_ROLLUP_HASH]) ... +ꜩ10000 -./tezos-client --wait none publish commitment from '[PUBLIC_KEY_HASH]' for sc rollup '[SC_ROLLUP_HASH]' with compressed state scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf at inbox level 62 and predecessor '[SC_ROLLUP_COMMITMENT_HASH]' and number of messages 0 and number of ticks 0 +./tezos-client --wait none publish commitment from '[PUBLIC_KEY_HASH]' for sc rollup '[SC_ROLLUP_HASH]' with compressed state scs11VNjWyZw4Tgbvsom8epQbox86S2CKkE1UAZkXMM7Pj8MQMLzMf at inbox level 62 and predecessor '[SC_ROLLUP_COMMITMENT_HASH]' and number of messages 0 and number of ticks 1 Node is bootstrapped. Estimated gas: 4479.904 units (will add 100 for safety) Estimated storage: no bytes added @@ -90,7 +90,7 @@ This sequence of operations was run: inbox_level: 62 predecessor: [SC_ROLLUP_COMMITMENT_HASH] number_of_messages: 0 - number_of_ticks: 0 in the smart contract rollup at address [SC_ROLLUP_HASH] + number_of_ticks: 1 in the smart contract rollup at address [SC_ROLLUP_HASH] This smart contract rollup commitment publishing was successfully applied Consumed gas: 4479.904 Hash of commit: [SC_ROLLUP_COMMITMENT_HASH] diff --git a/tezt/tests/sc_rollup.ml b/tezt/tests/sc_rollup.ml index 4b86ffd010be..86c63f1174a0 100644 --- a/tezt/tests/sc_rollup.ml +++ b/tezt/tests/sc_rollup.ml @@ -1968,7 +1968,7 @@ let publish_dummy_commitment ~inbox_level ~predecessor ~sc_rollup ~src client = inbox_level; predecessor; number_of_messages = 0; - number_of_ticks = 0; + number_of_ticks = 1; } in -- GitLab