From fcf7f04c3eaec5da15f259f9e39082a91acc86ce Mon Sep 17 00:00:00 2001 From: Thomas Letan Date: Wed, 19 Jun 2024 11:29:55 +0200 Subject: [PATCH] Proto: Consider past periods to decide the next commitment level ParisB has demonstrated the Smart Rollup commitment logic is not resilient to commitment period constant changes, forcing us to release Octez 20.1 to activate ParisC. The underlying logic error is well explained in [ParisC announcement], but in a nutshell, the reason can be summarized as follows: in ParisB, the new constants is used in place where the previous one should. [ParisC announcement]: https://research-development.nomadic-labs.com/parisC-announcement.html This patches addresses the root issue, but keeping track of past commitment periods (starting from ParisC). --- docs/protocols/alpha.rst | 4 + src/proto_alpha/lib_protocol/TEZOS_PROTOCOL | 2 +- src/proto_alpha/lib_protocol/dune | 8 +- src/proto_alpha/lib_protocol/init_storage.ml | 16 +++- .../lib_protocol/sc_rollup_repr.ml | 18 ++++ .../lib_protocol/sc_rollup_repr.mli | 20 +++++ .../lib_protocol/sc_rollup_stake_storage.ml | 14 ++- .../lib_protocol/sc_rollup_storage.ml | 48 ++++++++++ .../lib_protocol/sc_rollup_storage.mli | 16 ++++ src/proto_alpha/lib_protocol/storage.ml | 12 +++ src/proto_alpha/lib_protocol/storage.mli | 5 ++ tezt/tests/sc_rollup_migration.ml | 90 +++++++++++++++++++ 12 files changed, 236 insertions(+), 17 deletions(-) diff --git a/docs/protocols/alpha.rst b/docs/protocols/alpha.rst index f21a888a4f23..c5a91dd8947a 100644 --- a/docs/protocols/alpha.rst +++ b/docs/protocols/alpha.rst @@ -34,6 +34,10 @@ Smart Rollups of indiscriminately imposing them (e.g., imposing 2 weeks for the challenge window or 15 minutes for the commitment period). (MR :gl:`!13821`) +- Fixed the traversal logic of the commitments tree, by considering previous + commitment periods when computing what should be the level of a commitment + successor. (MR :gl:`!13841`) + Zero Knowledge Rollups (ongoing) -------------------------------- diff --git a/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL b/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL index d79b763f6783..feea36bc792c 100644 --- a/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL +++ b/src/proto_alpha/lib_protocol/TEZOS_PROTOCOL @@ -206,8 +206,8 @@ "Sc_rollup_commitment_storage", "Sc_rollup_outbox_storage", "Sc_rollup_whitelist_storage", - "Sc_rollup_stake_storage", "Sc_rollup_storage", + "Sc_rollup_stake_storage", "Dal_slot_storage", "Sc_rollup_refutation_storage", "Zk_rollup_errors", diff --git a/src/proto_alpha/lib_protocol/dune b/src/proto_alpha/lib_protocol/dune index 2813d1b0f374..fede92899f90 100644 --- a/src/proto_alpha/lib_protocol/dune +++ b/src/proto_alpha/lib_protocol/dune @@ -218,8 +218,8 @@ Sc_rollup_commitment_storage Sc_rollup_outbox_storage Sc_rollup_whitelist_storage - Sc_rollup_stake_storage Sc_rollup_storage + Sc_rollup_stake_storage Dal_slot_storage Sc_rollup_refutation_storage Zk_rollup_errors @@ -518,8 +518,8 @@ sc_rollup_commitment_storage.ml sc_rollup_commitment_storage.mli sc_rollup_outbox_storage.ml sc_rollup_outbox_storage.mli sc_rollup_whitelist_storage.ml sc_rollup_whitelist_storage.mli - sc_rollup_stake_storage.ml sc_rollup_stake_storage.mli sc_rollup_storage.ml sc_rollup_storage.mli + sc_rollup_stake_storage.ml sc_rollup_stake_storage.mli dal_slot_storage.ml dal_slot_storage.mli sc_rollup_refutation_storage.ml sc_rollup_refutation_storage.mli zk_rollup_errors.ml @@ -819,8 +819,8 @@ sc_rollup_commitment_storage.ml sc_rollup_commitment_storage.mli sc_rollup_outbox_storage.ml sc_rollup_outbox_storage.mli sc_rollup_whitelist_storage.ml sc_rollup_whitelist_storage.mli - sc_rollup_stake_storage.ml sc_rollup_stake_storage.mli sc_rollup_storage.ml sc_rollup_storage.mli + sc_rollup_stake_storage.ml sc_rollup_stake_storage.mli dal_slot_storage.ml dal_slot_storage.mli sc_rollup_refutation_storage.ml sc_rollup_refutation_storage.mli zk_rollup_errors.ml @@ -1104,8 +1104,8 @@ sc_rollup_commitment_storage.ml sc_rollup_commitment_storage.mli sc_rollup_outbox_storage.ml sc_rollup_outbox_storage.mli sc_rollup_whitelist_storage.ml sc_rollup_whitelist_storage.mli - sc_rollup_stake_storage.ml sc_rollup_stake_storage.mli sc_rollup_storage.ml sc_rollup_storage.mli + sc_rollup_stake_storage.ml sc_rollup_stake_storage.mli dal_slot_storage.ml dal_slot_storage.mli sc_rollup_refutation_storage.ml sc_rollup_refutation_storage.mli zk_rollup_errors.ml diff --git a/src/proto_alpha/lib_protocol/init_storage.ml b/src/proto_alpha/lib_protocol/init_storage.ml index 245659ae08f1..04a32a02a4db 100644 --- a/src/proto_alpha/lib_protocol/init_storage.ml +++ b/src/proto_alpha/lib_protocol/init_storage.ml @@ -112,9 +112,21 @@ let patch_script ctxt (address, hash, patched_code) = let prepare_first_block chain_id ctxt ~typecheck_smart_contract ~typecheck_smart_rollup ~level ~timestamp ~predecessor = let open Lwt_result_syntax in - let* previous_protocol, _previous_proto_constants, ctxt = + let* previous_protocol, previous_proto_constants, ctxt = Raw_context.prepare_first_block ~level ~timestamp chain_id ctxt in + let*? level = Raw_level_repr.of_int32 level in + (* To gracefully handle time block change, we keep a record of all commitment + periods used in the past. *) + let* ctxt = + match previous_proto_constants with + | Some previous_proto_constants -> + Sc_rollup_storage.save_commitment_period + ctxt + previous_proto_constants.sc_rollup.commitment_period_in_blocks + level + | None -> return ctxt + in let parametric = Raw_context.constants ctxt in let*! ctxt = let*! ctxt = @@ -128,7 +140,6 @@ let prepare_first_block chain_id ctxt ~typecheck_smart_contract match previous_protocol with | Genesis param -> (* This is the genesis protocol: initialise the state *) - let*? level = Raw_level_repr.of_int32 level in let* ctxt = Storage.Tenderbake.First_level_of_protocol.init ctxt level in @@ -191,7 +202,6 @@ let prepare_first_block chain_id ctxt ~typecheck_smart_contract if that is done, do not set Storage.Tenderbake.First_level_of_protocol. /!\ this storage is also use to add the smart rollup inbox migration message. see `sc_rollup_inbox_storage`. *) - let*? level = Raw_level_repr.of_int32 level in let* ctxt = Storage.Tenderbake.First_level_of_protocol.update ctxt level in diff --git a/src/proto_alpha/lib_protocol/sc_rollup_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_repr.ml index 9dae54444251..fa3b8cc82263 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_repr.ml @@ -106,3 +106,21 @@ module Number_of_ticks = struct | Some zero -> zero | None -> assert false (* unreachable case, since [min_int = 0l] *) end + +module Past_commitment_period = struct + type t = { + commitment_period_in_blocks : int; + next_protocol_activation : Raw_level_repr.t; + } + + let encoding = + Data_encoding.( + conv + (fun {commitment_period_in_blocks; next_protocol_activation} -> + (commitment_period_in_blocks, next_protocol_activation)) + (fun (commitment_period_in_blocks, next_protocol_activation) -> + {commitment_period_in_blocks; next_protocol_activation}) + @@ obj2 + (req "commitment_period" int31) + (req "next_protocol_activation" Raw_level_repr.encoding)) +end diff --git a/src/proto_alpha/lib_protocol/sc_rollup_repr.mli b/src/proto_alpha/lib_protocol/sc_rollup_repr.mli index d4d6aa0dcbfb..576fdcb8cd26 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_repr.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_repr.mli @@ -100,3 +100,23 @@ end (** The data model uses an index of these addresses. *) module Index : Storage_description.INDEX with type t = Address.t + +module Past_commitment_period : sig + type t = { + commitment_period_in_blocks : int; + (** The number of blocks separating two valid commitment for a given + particular Tezos protocol. *) + next_protocol_activation : Raw_level_repr.t; + (** The activation level of the next protocol, which is also the last + level during which the commitment period is valid. That is, if + + {math inbox_level + commitment_period_in_blocks <= next_protocol_activation} + + then [inbox_level + commitment_period_in_blocks] is the expected + level of the next commitment. + + Otherwise, the commitment period of the next protocol applies. *) + } + + val encoding : t Data_encoding.t +end 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 c25546aecb46..9e07240577ea 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_stake_storage.ml @@ -279,13 +279,12 @@ let assert_commitment_period ctxt rollup commitment = (* Commitments needs to be posted for inbox levels every [commitment_period]. Therefore, [commitment.inbox_level] must be [predecessor_commitment.inbox_level + commitment_period]. *) - let sc_rollup_commitment_period = - Constants_storage.sc_rollup_commitment_period_in_blocks ctxt + let* expected_level = + Sc_rollup_storage.next_commitment_level ctxt pred_level in let* () = fail_unless - Raw_level_repr.( - commitment.inbox_level = add pred_level sc_rollup_commitment_period) + Raw_level_repr.(commitment.inbox_level = expected_level) Sc_rollup_bad_inbox_level in return ctxt @@ -761,11 +760,8 @@ let cement_commitment ctxt rollup = let* old_lcc, old_lcc_level, ctxt = Commitment_storage.last_cemented_commitment_hash_with_level ctxt rollup in - let sc_rollup_commitment_period = - Constants_storage.sc_rollup_commitment_period_in_blocks ctxt - in - let new_lcc_level = - Raw_level_repr.add old_lcc_level sc_rollup_commitment_period + let* new_lcc_level = + Sc_rollup_storage.next_commitment_level ctxt old_lcc_level in (* Assert conditions to cement are met. *) let* ctxt, (new_lcc_commitment, new_lcc_commitment_hash), dangling_commitments diff --git a/src/proto_alpha/lib_protocol/sc_rollup_storage.ml b/src/proto_alpha/lib_protocol/sc_rollup_storage.ml index fe875fc93483..b5c2c16104a3 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_storage.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_storage.ml @@ -224,3 +224,51 @@ let must_exist ctxt rollup = let open Lwt_result_syntax in let* ctxt, exists = Store.Genesis_info.mem ctxt rollup in if exists then return ctxt else tzfail (Sc_rollup_does_not_exist rollup) + +let get_past_commitment_periods ctxt = + let open Lwt_result_syntax in + let+ cell = Store.Past_commitment_periods.find ctxt in + Option.value ~default:[] cell + +let save_commitment_period ctxt previous_commitment_period + next_protocol_activation = + let open Lwt_result_syntax in + let* past_commitments = get_past_commitment_periods ctxt in + let*! ctxt = + Store.Past_commitment_periods.add + ctxt + (past_commitments + @ [ + { + commitment_period_in_blocks = previous_commitment_period; + next_protocol_activation; + }; + ]) + (* We enforce the fact that the past commitment periods is sorted in + ascending order of [next_protocol_activation_level]. *) + in + return ctxt + +let next_commitment_level ctxt inbox_level = + let open Lwt_result_syntax in + (* This function assumes [past_commitments] is sorted in ascending order of + [next_protocol_activation_level]. This invariant is enforced by + [save_commitment_period]. *) + let+ past_commitments = get_past_commitment_periods ctxt in + let candidate_level = + List.find_map + (fun Sc_rollup_repr.Past_commitment_period. + {commitment_period_in_blocks; next_protocol_activation} -> + let candidate_level = + Raw_level_repr.add inbox_level commitment_period_in_blocks + in + if Raw_level_repr.(candidate_level <= next_protocol_activation) then + Some candidate_level + else None) + past_commitments + in + match candidate_level with + | Some candidate_level -> candidate_level + | None -> + Raw_level_repr.( + add inbox_level Raw_context.(sc_rollup ctxt).commitment_period_in_blocks) diff --git a/src/proto_alpha/lib_protocol/sc_rollup_storage.mli b/src/proto_alpha/lib_protocol/sc_rollup_storage.mli index 8da1651e5a5b..ca6ac76a88a0 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_storage.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_storage.mli @@ -100,3 +100,19 @@ val parameters_type : returned. *) val must_exist : Raw_context.t -> Sc_rollup_repr.t -> Raw_context.t tzresult Lwt.t + +(** [save_commitment_period ctxt previous_commitment_period next_protocol_activation] + records that before level [next_protocol_activation], the commitment period + to use is [previous_commitment_period]. *) +val save_commitment_period : + Raw_context.t -> int -> Raw_level_repr.t -> Raw_context.t tzresult Lwt.t + +(** [next_commitment_level ctxt inbox_level] returns the level that the next + commitment should target, taking into account the commitment periods of + past protocols as well as the current one. + + {b Precondition:} For [next_commitment_level] result to actually be a + valid level for a commitment requires that [inbox_level] is a valid level + for a commitment as well. *) +val next_commitment_level : + Raw_context.t -> Raw_level_repr.t -> Raw_level_repr.t tzresult Lwt.t diff --git a/src/proto_alpha/lib_protocol/storage.ml b/src/proto_alpha/lib_protocol/storage.ml index b1d0075db47f..bc307ee8fbcb 100644 --- a/src/proto_alpha/lib_protocol/storage.ml +++ b/src/proto_alpha/lib_protocol/storage.ml @@ -1830,6 +1830,18 @@ module Sc_rollup = struct let encoding = Sc_rollup_commitment_repr.genesis_info_encoding end) + module Past_commitment_periods = + Make_single_data_storage (Registered) (Raw_context) + (struct + let name = ["past_commitment_periods"] + end) + (struct + type t = Sc_rollup_repr.Past_commitment_period.t list + + let encoding = + Data_encoding.(list Sc_rollup_repr.Past_commitment_period.encoding) + end) + module Inbox = struct include Make_single_data_storage (Registered) (Raw_context) diff --git a/src/proto_alpha/lib_protocol/storage.mli b/src/proto_alpha/lib_protocol/storage.mli index 12fbd3bccadd..f48967588c42 100644 --- a/src/proto_alpha/lib_protocol/storage.mli +++ b/src/proto_alpha/lib_protocol/storage.mli @@ -850,6 +850,11 @@ module Sc_rollup : sig and type value = Sc_rollup_commitment_repr.genesis_info and type t := Raw_context.t + module Past_commitment_periods : + Single_data_storage + with type value = Sc_rollup_repr.Past_commitment_period.t list + and type t := Raw_context.t + module Inbox : Single_data_storage with type value = Sc_rollup_inbox_repr.t diff --git a/tezt/tests/sc_rollup_migration.ml b/tezt/tests/sc_rollup_migration.ml index 45110add747d..61b24f9d7c35 100644 --- a/tezt/tests/sc_rollup_migration.ml +++ b/tezt/tests/sc_rollup_migration.ml @@ -605,6 +605,95 @@ let test_rollup_node_catchup_migration ~kind ~migrate_from ~migrate_to = ~description () +let test_rollup_node_migration_constant_changes ~kind ~migrate_from ~migrate_to + = + let next_block client rollup_node = + let* () = Sc_rollup_helpers.send_messages 1 client in + let* _ = Sc_rollup_node.wait_sync ~timeout:10. rollup_node in + unit + in + let tags = ["constants_changes"] in + let description = + "node can deal with protocol migration with constant changes" + in + let commitment_period = 6 in + let challenge_window = 6 in + let scenario_prior ~sc_rollup:_ ~rollup_node _tezos_node tezos_client = + (* Bake enough blocks to compute *and* publish a commitment in the previous + protocol. *) + let* () = + repeat (commitment_period + 4) (fun () -> + next_block tezos_client rollup_node) + in + let* commitment_and_hash = + Sc_rollup_node.RPC.call + rollup_node + (Sc_rollup_rpc.get_global_last_stored_commitment ()) + in + (* Check that the first commitment has been indeed computed by the rollup + node. *) + Check.( + (commitment_and_hash.commitment.inbox_level = 2 + commitment_period) int) + ~error_msg: + "Unexpected last stored commitment, got %L but was expecting %R" ; + unit + in + let scenario_after ~sc_rollup ~rollup_node tezos_node tezos_client () = + (* Fetch the new commitment period, after constants update *) + let* {commitment_period_in_blocks = new_commitment_period; _} = + Sc_rollup_helpers.get_sc_rollup_constants tezos_client + in + (* Bake twice commitment periods. This should be enough for the rollup node + to compute two more commitments in the next protocol, and to decide to + cement the first one. *) + let* () = + repeat (2 * new_commitment_period) (fun () -> + next_block tezos_client rollup_node) + in + (* We first check the last computed commitment. *) + let* commitment_and_hash = + Sc_rollup_node.RPC.call + rollup_node + (Sc_rollup_rpc.get_global_last_stored_commitment ()) + in + Check.( + (commitment_and_hash.commitment.inbox_level + = 2 + commitment_period + (2 * new_commitment_period)) + int) + ~error_msg: + "Unexpected last stored commitment, got %L but was expecting %R" ; + (* We bake a few more blocks, to ensure the rollup node indeed injects a + cementation operation. *) + let* () = repeat 3 (fun () -> next_block tezos_client rollup_node) in + (* We check the LCC is indeed at the expected level. *) + let* commitment = + Node.RPC.call + tezos_node + RPC.( + get_chain_block_context_smart_rollups_smart_rollup_last_cemented_commitment_hash_with_level + sc_rollup) + in + Check.( + (JSON.(commitment |-> "level" |> as_int) + = 2 + commitment_period + new_commitment_period) + int) + ~error_msg: + "Unexpected last cemented commitment, got %L but was expecting %R" ; + unit + in + test_l2_migration_scenario + ~with_constant_change:true + ~tags + ~kind + ~commitment_period + ~challenge_window + ~migrate_from + ~migrate_to + ~scenario_prior + ~scenario_after + ~description + () + (* Test originate rollup in previous protocol and start node in a different protocol. *) let test_originate_before_migration ~kind ~migrate_from ~migrate_to = @@ -950,6 +1039,7 @@ let register_migration ~kind ~migrate_from ~migrate_to = test_cont_refute_pre_migration ~kind ~migrate_from ~migrate_to ; test_rollup_node_simple_migration ~kind ~migrate_from ~migrate_to ; test_rollup_node_catchup_migration ~kind ~migrate_from ~migrate_to ; + test_rollup_node_migration_constant_changes ~kind ~migrate_from ~migrate_to ; test_originate_before_migration ~kind ~migrate_from ~migrate_to ; test_migration_removes_dead_games ~kind ~migrate_from ~migrate_to -- GitLab