From 20a2c94fd8ed30f3d32e6d90302aa3387bc1e374 Mon Sep 17 00:00:00 2001 From: Alain Mebsout Date: Tue, 4 Feb 2025 17:21:37 +0100 Subject: [PATCH 1/4] Test: check correct outbox execution for messages in case of reorg --- tezt/tests/sc_rollup.ml | 173 +++++++++++++++++++++++++++++++--------- 1 file changed, 136 insertions(+), 37 deletions(-) diff --git a/tezt/tests/sc_rollup.ml b/tezt/tests/sc_rollup.ml index 39815ede44b0..4f3e46b6887d 100644 --- a/tezt/tests/sc_rollup.ml +++ b/tezt/tests/sc_rollup.ml @@ -628,6 +628,54 @@ let sc_rollup_node_disconnects_scenario sc_rollup_node sc_rollup node client = Test.fail "Refutation loop did not process after reconnection"); ] +let setup_double_reorg node client = + let nodes_args = + Node.[Synchronisation_threshold 0; History_mode Archive; No_bootstrap_peers] + in + let* node2, client2 = Client.init_with_node ~nodes_args `Client () + and* node3, client3 = Client.init_with_node ~nodes_args `Client () in + let* () = Client.Admin.trust_address client ~peer:node2 + and* () = Client.Admin.trust_address client2 ~peer:node + and* () = Client.Admin.trust_address client ~peer:node3 + and* () = Client.Admin.trust_address client3 ~peer:node + and* () = Client.Admin.trust_address client2 ~peer:node3 + and* () = Client.Admin.trust_address client3 ~peer:node2 in + let* () = Client.Admin.connect_address client ~peer:node2 + and* () = Client.Admin.connect_address client ~peer:node3 + and* () = Client.Admin.connect_address client2 ~peer:node3 in + let divergence ~branch1 ~branch2 = + let* identity2 = Node.wait_for_identity node2 + and* identity3 = Node.wait_for_identity node3 in + let* () = Client.Admin.kick_peer client ~peer:identity2 + and* () = Client.Admin.kick_peer client2 ~peer:identity3 in + let* () = branch1 client in + let* level = Node.get_level node in + let* _ = Node.wait_for_level node3 level in + Log.info "Nodes 1 and 3 have branch1." ; + let* () = Client.Admin.kick_peer client ~peer:identity3 in + let* () = branch2 client2 in + (* One extra block in branch2 *) + let* () = Client.bake_for_and_wait client2 in + Log.info "Nodes 1 and 2 are following distinct branches." ; + (* Two extra blocks in node3 *) + let* () = Client.bake_for_and_wait client3 in + let* () = Client.bake_for_and_wait client3 in + Log.info "Nodes 1 and 2 and 3 are following distinct branches." ; + unit + in + let trigger_reorg () = + let* level2 = Node.get_level node2 in + let* level3 = Node.get_level node3 in + let* () = Client.Admin.connect_address client ~peer:node2 in + let* _ = Node.wait_for_level node level2 in + Log.info "Node 1 has switched to second branch." ; + let* () = Client.Admin.connect_address client ~peer:node3 in + let* _ = Node.wait_for_level node level3 in + Log.info "Node 1 has switched back to its first branch (with extra blocks)." ; + unit + in + return (divergence, trigger_reorg) + let sc_rollup_node_handles_chain_reorg sc_rollup_node sc_rollup node client = let num_messages = 1 in let nodes_args = @@ -4169,8 +4217,8 @@ let test_refutation_reward_and_punishment ~kind = *) let test_outbox_message_generic ?supports ?regression ?expected_error ?expected_l1_error ~earliness ?entrypoint ~init_storage ~storage_ty - ?outbox_parameters_ty ?boot_sector ~input_message ~expected_storage ~kind - ~message_kind ~auto_execute_outbox = + ?outbox_parameters_ty ?boot_sector ?(reorg = false) ~input_message + ~expected_storage ~kind ~message_kind ~auto_execute_outbox () = let commitment_period = 2 and challenge_window = 5 in let _outbox_level = 5 in let message_index = 0 in @@ -4182,6 +4230,7 @@ let test_outbox_message_generic ?supports ?regression ?expected_error Option.value ~default:"no_parameters_ty" outbox_parameters_ty in let auto_s = if auto_execute_outbox then ", auto_execute_outbox" else "" in + let reorg_s = if reorg then ", reorg" else "" in test_full_scenario ?supports ?regression @@ -4192,21 +4241,30 @@ let test_outbox_message_generic ?supports ?regression ?expected_error ~commitment_period ~challenge_window { - tags = ["outbox"; message_kind_s; entrypoint_s; outbox_parameters_ty_s]; + tags = + (["outbox"; message_kind_s; entrypoint_s; outbox_parameters_ty_s] + @ if reorg then ["reorg"] else []); variant = Some (Format.sprintf - "%s, entrypoint: %%%s, eager: %d, %s, %s%s" + "%s, entrypoint: %%%s, eager: %d, %s, %s%s%s" init_storage entrypoint_s earliness message_kind_s outbox_parameters_ty_s - auto_s); + auto_s + reorg_s); description = "output exec"; } ~uses:(fun _protocol -> [Constant.octez_codec]) @@ fun protocol rollup_node sc_rollup node client -> + let* reorg = + if reorg then + let* actions = setup_double_reorg node client in + return (Some actions) + else return None + in let src = Constant.bootstrap1.public_key_hash in let src2 = Constant.bootstrap2.public_key_hash in let* () = @@ -4279,7 +4337,7 @@ let test_outbox_message_generic ?supports ?regression ?expected_error string ~error_msg:"Invalid contract storage: expecting '%R', got '%L'.") in - let check_outbox_execution outbox_level status = + let check_outbox_execution outbox_level indexes status = let* _ = Sc_rollup_node.wait_sync rollup_node ~timeout:10. in let* executable_outboxes = Sc_rollup_node.RPC.call rollup_node @@ -4289,58 +4347,87 @@ let test_outbox_message_generic ?supports ?regression ?expected_error Sc_rollup_node.RPC.call rollup_node @@ Sc_rollup_rpc.get_local_outbox_pending_unexecutable () in - let get_nb l = + let get_indexes l = match List.assoc_opt outbox_level l with - | None -> 0 - | Some l -> List.length l + | None -> [] + | Some l -> List.map (fun x -> x.Sc_rollup_rpc.message_index) l in - let executable = get_nb executable_outboxes in - let unexecutable = get_nb unexecutable_outboxes in + let executable = get_indexes executable_outboxes in + let unexecutable = get_indexes unexecutable_outboxes in match (status, executable, unexecutable) with - | `Executable, 1, 0 -> unit - | `Unexecutable, 0, 1 -> unit + | `Executable, e, [] when e = indexes -> unit + | `Unexecutable, [], e when e = indexes -> unit | _ -> Test.fail - "Outbox level should be %s but there is %d executable, %d \ + "Outbox level should be [%a] %s but there is [%a] executable, [%a] \ unexecutable." + (Format.pp_print_list Format.pp_print_int) + indexes (match status with | `Executable -> "executable" | `Unexecutable -> "unexecutable") + (Format.pp_print_list Format.pp_print_int) executable + (Format.pp_print_list Format.pp_print_int) unexecutable in let perform_rollup_execution_and_cement source_address target_address = Log.info "Perform rollup execution and cement" ; - let* payload = input_message protocol target_address in + let trigger_output_message ?(extra_empty_messages = 0) client = + let* payload = input_message protocol target_address in + let* () = + match payload with + | `External payload -> + let extra = List.init extra_empty_messages (fun _ -> "") in + send_text_messages ~hooks ~format:`Hex client (extra @ [payload]) + | `Internal payload -> + let payload = "0x" ^ payload in + let* () = + Client.transfer + ~amount:Tez.(of_int 100) + ~burn_cap:Tez.(of_int 100) + ~storage_limit:100000 + ~giver:Constant.bootstrap1.alias + ~receiver:source_address + ~arg:(sf "Pair %s %S" payload sc_rollup) + client + in + Client.bake_for_and_wait client + in + let* _ = Sc_rollup_node.wait_sync rollup_node ~timeout:10. in + unit + in let* () = - match payload with - | `External payload -> - send_text_messages ~hooks ~format:`Hex client [payload] - | `Internal payload -> - let payload = "0x" ^ payload in - let* () = - Client.transfer - ~amount:Tez.(of_int 100) - ~burn_cap:Tez.(of_int 100) - ~storage_limit:100000 - ~giver:Constant.bootstrap1.alias - ~receiver:source_address - ~arg:(sf "Pair %s %S" payload sc_rollup) - client - in - Client.bake_for_and_wait client + match reorg with + | None -> trigger_output_message client + | Some (divergence, _) -> + divergence + ~branch1:trigger_output_message + ~branch2:(trigger_output_message ~extra_empty_messages:2) in let* outbox_level = Sc_rollup_node.wait_sync rollup_node ~timeout:10. in + let* () = + match reorg with + | None -> unit + | Some (_, trigger_reorg) -> + let* () = trigger_reorg () in + let* _ = Sc_rollup_node.wait_sync rollup_node ~timeout:10. in + unit + in let* {outbox; _} = Sc_rollup_node.RPC.call rollup_node - @@ Sc_rollup_rpc.get_global_block ~outbox:true () + @@ Sc_rollup_rpc.get_global_block + ~block:(string_of_int outbox_level) + ~outbox:true + () in let nb_outbox_transactions_in_block = - outbox |> List.map @@ fun (Sc_rollup_rpc.Transactions j) -> List.length j + outbox + |> List.map @@ fun (Sc_rollup_rpc.Transactions trs) -> List.length trs in Check.((nb_outbox_transactions_in_block = [1]) (list int)) ~error_msg:"Block has %L outbox transactions but expected %R" ; - let* () = check_outbox_execution outbox_level `Unexecutable in + let* () = check_outbox_execution outbox_level [0] `Unexecutable in let* _ = bake_until_lcc_updated ~timeout:100. @@ -4406,7 +4493,7 @@ let test_outbox_message_generic ?supports ?regression ?expected_error in if Option.is_some expected_error then unit else - let* () = check_outbox_execution outbox_level `Executable in + let* () = check_outbox_execution outbox_level [0] `Executable in if auto_execute_outbox then bake_until_execute_outbox_message ~at_least:1 @@ -4420,7 +4507,7 @@ let test_outbox_message_generic ?supports ?regression ?expected_error | Some {commitment_hash; proof} -> ( match expected_l1_error with | None -> - let* () = check_outbox_execution outbox_level `Executable in + let* () = check_outbox_execution outbox_level [0] `Executable in let*! () = Client.Sc_rollup.execute_outbox_message ~hooks @@ -4494,7 +4581,7 @@ let test_outbox_message_generic ?supports ?regression ?expected_error let test_outbox_message ?supports ?regression ?expected_error ?expected_l1_error ~earliness ?entrypoint ?(init_storage = "0") ?(storage_ty = "int") - ?(outbox_parameters = "37") ?outbox_parameters_ty ~kind ~message_kind + ?(outbox_parameters = "37") ?outbox_parameters_ty ?reorg ~kind ~message_kind ~auto_execute_outbox = let wrap payload = match message_kind with @@ -4558,6 +4645,7 @@ let test_outbox_message ?supports ?regression ?expected_error ?expected_l1_error ?entrypoint ?outbox_parameters_ty ?boot_sector + ?reorg ~init_storage ~storage_ty ~input_message @@ -4565,6 +4653,16 @@ let test_outbox_message ?supports ?regression ?expected_error ?expected_l1_error ~message_kind ~kind ~auto_execute_outbox + () + +let test_outbox_message_reorg protocols ~kind = + test_outbox_message + ~earliness:0 + ~message_kind:`External + ~kind + ~auto_execute_outbox:true + ~reorg:true + protocols let test_outbox_message protocols ~kind = let test @@ -7096,6 +7194,7 @@ let register_protocol_independent () = sc_rollup_node_batcher protocols ; test_rollup_node_inbox ~kind ~variant:"basic" basic_scenario protocols ; + test_outbox_message_reorg ~kind protocols ; test_gc "many_gc" ~kind -- GitLab From e76b55c7332fc87c8ea4ff840bc935a482b591d6 Mon Sep 17 00:00:00 2001 From: Alain Mebsout Date: Tue, 4 Feb 2025 17:21:24 +0100 Subject: [PATCH 2/4] Rollup node: fix outbox registration in case of reorg Fixes #7730 This is a dirty patch but we need to properly handle reorgs at the storage level. --- src/lib_smart_rollup_node/rollup_node_daemon.ml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/lib_smart_rollup_node/rollup_node_daemon.ml b/src/lib_smart_rollup_node/rollup_node_daemon.ml index 214d5ed847ba..96cf685bbd76 100644 --- a/src/lib_smart_rollup_node/rollup_node_daemon.ml +++ b/src/lib_smart_rollup_node/rollup_node_daemon.ml @@ -264,7 +264,16 @@ and update_l2_chain ({node_ctxt; _} as state) ~catching_up the predecessor. In this case, we don't update the head or notify the block. *) return_unit - else Node_context.set_l2_head node_ctxt l2_block + else + (* Reorg (reproposal) of a block already handled *) + (* TODO: https://gitlab.com/tezos/tezos/-/issues/7731 + This just overwrites the outbox messages with the correct ones for + the level but we need to properly handle reorgs in the storage. *) + let* ctxt = Node_context.checkout_context node_ctxt head.hash in + let* () = + register_outbox_messages state.plugin node_ctxt ctxt head.level + in + Node_context.set_l2_head node_ctxt l2_block | `New l2_block -> let* () = Node_context.set_l2_head node_ctxt l2_block in let stop_timestamp = Time.System.now () in -- GitLab From 759b11f75ed7a1a921d67c4d7444665ae37ccbf5 Mon Sep 17 00:00:00 2001 From: Alain Mebsout Date: Tue, 11 Feb 2025 14:34:40 +0100 Subject: [PATCH 3/4] Rollup node: don't go through publisher worker for executing outbox This allows to execute faster and potentially inject in the next round. --- src/lib_smart_rollup_node/publisher.ml | 14 +------------- src/lib_smart_rollup_node/publisher.mli | 3 --- .../publisher_worker_types.ml | 8 -------- .../publisher_worker_types.mli | 2 -- src/lib_smart_rollup_node/rollup_node_daemon.ml | 2 +- 5 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/lib_smart_rollup_node/publisher.ml b/src/lib_smart_rollup_node/publisher.ml index b5ee32901af9..589768e92ee9 100644 --- a/src/lib_smart_rollup_node/publisher.ml +++ b/src/lib_smart_rollup_node/publisher.ml @@ -579,9 +579,6 @@ let on_cement_commitments (node_ctxt : state) = let* cementable_commitments = cementable_commitments node_ctxt in List.iter_es (cement_commitment node_ctxt) cementable_commitments -let on_execute_outbox (node_ctxt : state) = - Outbox_execution.publish_executable_messages node_ctxt - module Types = struct type nonrec state = state @@ -617,7 +614,6 @@ module Handlers = struct match request with | Request.Publish -> protect @@ fun () -> on_publish_commitments state | Request.Cement -> protect @@ fun () -> on_cement_commitments state - | Request.Execute_outbox -> protect @@ fun () -> on_execute_outbox state type launch_error = error trace @@ -636,7 +632,6 @@ module Handlers = struct match r with | Request.Publish -> emit_and_return_errors errs | Request.Cement -> emit_and_return_errors errs - | Request.Execute_outbox -> emit_and_return_errors errs let on_completion _w r _ st = Commitment_event.Publisher.request_completed (Request.view r) st @@ -661,10 +656,7 @@ let start_in_mode mode = match mode with | Maintenance | Operator | Bailout -> true | Observer | Accuser | Batcher -> false - | Custom ops -> - purposes_matches_mode - (Custom ops) - [Operating; Cementing; Executing_outbox] + | Custom ops -> purposes_matches_mode (Custom ops) [Operating; Cementing] let init (node_ctxt : _ Node_context.t) = let open Lwt_result_syntax in @@ -711,10 +703,6 @@ let cement_commitments () = worker_add_request ~request:Request.Cement (fun node_ctxt -> Configuration.can_inject node_ctxt.config.mode Cement) -let execute_outbox () = - worker_add_request ~request:Request.Execute_outbox (fun node_ctxt -> - Configuration.can_inject node_ctxt.config.mode Execute_outbox_message) - let shutdown () = match worker () with | Error _ -> diff --git a/src/lib_smart_rollup_node/publisher.mli b/src/lib_smart_rollup_node/publisher.mli index 92f0edcb11f0..168d9c27164a 100644 --- a/src/lib_smart_rollup_node/publisher.mli +++ b/src/lib_smart_rollup_node/publisher.mli @@ -96,9 +96,6 @@ val publish_commitments : unit -> unit tzresult Lwt.t appropriate mode. *) val cement_commitments : unit -> unit tzresult Lwt.t -(** [execute_outbox] execute pending outbox messages on L1. *) -val execute_outbox : unit -> unit tzresult Lwt.t - (** Stop worker for publishing and cementing commitments. *) val shutdown : unit -> unit Lwt.t diff --git a/src/lib_smart_rollup_node/publisher_worker_types.ml b/src/lib_smart_rollup_node/publisher_worker_types.ml index 2084d99e0c1f..c10e697273e3 100644 --- a/src/lib_smart_rollup_node/publisher_worker_types.ml +++ b/src/lib_smart_rollup_node/publisher_worker_types.ml @@ -27,7 +27,6 @@ module Request = struct type ('a, 'b) t = | Publish : (unit, error trace) t | Cement : (unit, error trace) t - | Execute_outbox : (unit, error trace) t type view = View : _ t -> view @@ -49,17 +48,10 @@ module Request = struct (obj1 (req "request" (constant "cement"))) (function View Cement -> Some () | _ -> None) (fun () -> View Cement); - case - (Tag 2) - ~title:"Execute_outbox" - (obj1 (req "request" (constant "execute_outbox"))) - (function View Execute_outbox -> Some () | _ -> None) - (fun () -> View Execute_outbox); ] let pp ppf (View r) = match r with | Publish -> Format.pp_print_string ppf "publish" | Cement -> Format.pp_print_string ppf "cement" - | Execute_outbox -> Format.pp_print_string ppf "execute_outbox" end diff --git a/src/lib_smart_rollup_node/publisher_worker_types.mli b/src/lib_smart_rollup_node/publisher_worker_types.mli index eb43e0109428..eed1fdcad468 100644 --- a/src/lib_smart_rollup_node/publisher_worker_types.mli +++ b/src/lib_smart_rollup_node/publisher_worker_types.mli @@ -30,8 +30,6 @@ module Request : sig (** Request to publish new commitments in L1. *) | Cement : (unit, error trace) t (** Request to cement commitments in L1. *) - | Execute_outbox : (unit, error trace) t - (** Request to execute outbox messages on L1. *) type view = View : _ t -> view diff --git a/src/lib_smart_rollup_node/rollup_node_daemon.ml b/src/lib_smart_rollup_node/rollup_node_daemon.ml index 96cf685bbd76..c0f0939280d6 100644 --- a/src/lib_smart_rollup_node/rollup_node_daemon.ml +++ b/src/lib_smart_rollup_node/rollup_node_daemon.ml @@ -379,7 +379,7 @@ let on_layer_1_head ({node_ctxt; _} as state) (head : Layer1.header) = notify_synchronization node_ctxt head.level ; let* () = Publisher.publish_commitments () in let* () = Publisher.cement_commitments () in - let* () = Publisher.execute_outbox () in + let* () = Outbox_execution.publish_executable_messages node_ctxt in let*! () = Daemon_event.new_heads_processed reorg.new_chain in let* () = Batcher.produce_batches () in let* () = Dal_injection_queue.produce_dal_slots ~level:head.level in -- GitLab From 577b0b3f565cce73f1f8fa3d4e586584c6bf2d49 Mon Sep 17 00:00:00 2001 From: Alain Mebsout Date: Wed, 12 Feb 2025 08:58:29 +0100 Subject: [PATCH 4/4] Rollup node: emit event in case of reorg --- src/lib_smart_rollup_node/daemon_event.ml | 14 ++++++++++++++ src/lib_smart_rollup_node/daemon_event.mli | 4 ++++ src/lib_smart_rollup_node/rollup_node_daemon.ml | 1 + 3 files changed, 19 insertions(+) diff --git a/src/lib_smart_rollup_node/daemon_event.ml b/src/lib_smart_rollup_node/daemon_event.ml index 9a7db83058e7..5b7da9d2b0f6 100644 --- a/src/lib_smart_rollup_node/daemon_event.ml +++ b/src/lib_smart_rollup_node/daemon_event.ml @@ -62,6 +62,16 @@ module Simple = struct ("hash", Block_hash.encoding) ("level", Data_encoding.int32) + let reorg = + declare_3 + ~section + ~name:"smart_rollup_node_daemon_reorg" + ~msg:"Reorg of {number} blocks from level {from} to level {to}" + ~level:Notice + ("number", Data_encoding.int31) + ("from", Data_encoding.int32) + ("to", Data_encoding.int32) + let processing_heads_iteration = declare_3 ~section @@ -229,6 +239,10 @@ let new_heads_iteration event = function let processing_heads_iteration = new_heads_iteration Simple.processing_heads_iteration +let reorg = function + | [] -> Lwt.return_unit + | blocks -> new_heads_iteration Simple.reorg blocks + let new_heads_processed = new_heads_iteration Simple.new_heads_processed let new_heads_side_process_finished = diff --git a/src/lib_smart_rollup_node/daemon_event.mli b/src/lib_smart_rollup_node/daemon_event.mli index 25206267ca14..a0cf326169db 100644 --- a/src/lib_smart_rollup_node/daemon_event.mli +++ b/src/lib_smart_rollup_node/daemon_event.mli @@ -47,6 +47,10 @@ val new_head_degraded : Block_hash.t -> int32 -> unit Lwt.t going to be processed. *) val processing_heads_iteration : Layer1.head list -> unit Lwt.t +(** [reorg blocks] emits the event that the [blocks] are part of a + reorganization. *) +val reorg : Layer1.head list -> unit Lwt.t + (** [new_heads_processed heads] emits the event that the [heads] were processed. *) val new_heads_processed : Layer1.head list -> unit Lwt.t diff --git a/src/lib_smart_rollup_node/rollup_node_daemon.ml b/src/lib_smart_rollup_node/rollup_node_daemon.ml index c0f0939280d6..ed42e4d15de5 100644 --- a/src/lib_smart_rollup_node/rollup_node_daemon.ml +++ b/src/lib_smart_rollup_node/rollup_node_daemon.ml @@ -351,6 +351,7 @@ let on_layer_1_head ({node_ctxt; _} as state) (head : Layer1.header) = Node_context.get_tezos_reorg_for_new_head node_ctxt old_head stripped_head in let*? reorg = report_missing_data reorg in + let*! () = Daemon_event.reorg reorg.old_chain in (* TODO: https://gitlab.com/tezos/tezos/-/issues/3348 Rollback state information on reorganization, i.e. for reorg.old_chain. *) -- GitLab