diff --git a/CHANGES.rst b/CHANGES.rst index e40a1df39e0e830ed44881c361779693f5bba623..bab299c66a0b211f8f381bf3c1271f8dd50d9664 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -267,6 +267,9 @@ Smart Rollup node ``/local/outbox/pending`` and ``/local/outbox/pending/executable``. (MR :gl:`!17279`) +- Fix issue with reorgs and outbox messages execution (e.g. withdrawals on + Etherlink) when messages are not in the new chain. (MR :gl:`!17961`) + Smart Rollup WASM Debugger -------------------------- diff --git a/src/lib_smart_rollup_node/commitment_event.ml b/src/lib_smart_rollup_node/commitment_event.ml index 16dd6acb112a0fec3335ca9b136bebbd5850ce33..8c0b9a88e122efafface35e1c5a6179ff9d2c418 100644 --- a/src/lib_smart_rollup_node/commitment_event.ml +++ b/src/lib_smart_rollup_node/commitment_event.ml @@ -141,6 +141,17 @@ module Simple = struct ("error", trace_encoding) ~pp3:Outbox_message.pp_summary + let outbox_message_missing = + declare_2 + ~section + ~name:"smart_rollup_node_outbox_message_missing" + ~msg: + "Outbox level {outbox_level} does not contain any message at index \ + {message_index}. Please report this error." + ~level:Error + ("outbox_level", Data_encoding.int32) + ("message_index", Data_encoding.int31) + module Publisher = struct let section = section @ ["publisher"] @@ -203,6 +214,9 @@ let outbox_message_execution_failed ~outbox_level ~message_index message trace = outbox_message_execution_failed (outbox_level, message_index, message, trace)) +let outbox_message_missing ~outbox_level ~message_index = + Simple.(emit outbox_message_missing (outbox_level, message_index)) + module Publisher = struct let request_failed view worker_status errors = Simple.(emit Publisher.request_failed (view, worker_status, errors)) diff --git a/src/lib_smart_rollup_node/commitment_event.mli b/src/lib_smart_rollup_node/commitment_event.mli index 0457abc21b8b76e6bcc58e0e38b1a43fb6722ec2..5ba1bdefa07aafba1f2195a66299e20cfd88d46c 100644 --- a/src/lib_smart_rollup_node/commitment_event.mli +++ b/src/lib_smart_rollup_node/commitment_event.mli @@ -78,6 +78,9 @@ val outbox_message_execution_failed : tztrace -> unit Lwt.t +val outbox_message_missing : + outbox_level:int32 -> message_index:int -> unit Lwt.t + (** Events emmitted by the Publisher worker *) module Publisher : sig (** [request_failed view status errors] emits the event that a worker diff --git a/src/lib_smart_rollup_node/node_context.ml b/src/lib_smart_rollup_node/node_context.ml index 2931af12b77136c0f4caea3dd9858007bd7fa982..45236bba08823c2c7897870aab3a2ab91c1e8a3f 100644 --- a/src/lib_smart_rollup_node/node_context.ml +++ b/src/lib_smart_rollup_node/node_context.ml @@ -665,8 +665,14 @@ let set_outbox_message_executed {store; _} ~outbox_level ~index = let register_outbox_messages {store; _} ~outbox_level ~indexes = let open Lwt_result_syntax in - let*? indexes = Bitset.from_list indexes in - Store.Outbox_messages.register_outbox_messages store ~outbox_level ~indexes + if indexes = [] then + (* Remove any previously added messages for this level (e.g. in case of + reorg) because there aren't any in this level. *) + Store.Outbox_messages.delete_outbox_messages store ~outbox_level + else + (* Overwrite (or add) messages for this level. *) + let*? indexes = Bitset.from_list indexes in + Store.Outbox_messages.register_outbox_messages store ~outbox_level ~indexes let get_executable_pending_outbox_messages {store; lcc; current_protocol; _} = let max_level = (Reference.get lcc).level in diff --git a/src/lib_smart_rollup_node/outbox_execution.ml b/src/lib_smart_rollup_node/outbox_execution.ml index 9f0c13d52d9ba91b746a73ecc8d007e869719b1a..79151b3e3f8e9bb5edc4291b2f0ebf2f1479b4b1 100644 --- a/src/lib_smart_rollup_node/outbox_execution.ml +++ b/src/lib_smart_rollup_node/outbox_execution.ml @@ -216,7 +216,11 @@ let outbox_message_match_filter (message : Outbox_message.summary) let publish_executable_messages (node_ctxt : _ Node_context.t) = let open Lwt_result_syntax in let operator = Node_context.get_operator node_ctxt Executing_outbox in - unless (operator = None) @@ fun () -> + let executes = + Option.is_some operator + && not (List.is_empty node_ctxt.config.execute_outbox_messages_filter) + in + when_ executes @@ fun () -> (* Configured to execute outbox messages *) let lcc = Reference.get node_ctxt.lcc in let* lcc_block_hash = Node_context.hash_of_level node_ctxt lcc.level in @@ -245,7 +249,13 @@ let publish_executable_messages (node_ctxt : _ Node_context.t) = List.assoc ~equal:Int.equal message_index outbox in match message with - | None -> assert false + | None -> + let*! () = + Commitment_event.outbox_message_missing + ~outbox_level + ~message_index + in + return_unit | Some (Whitelist_update _) -> (* Don't execute whitelist updates as this is done by {!publish_execute_whitelist_update_message}. *) diff --git a/src/lib_smart_rollup_node/rollup_node_daemon.ml b/src/lib_smart_rollup_node/rollup_node_daemon.ml index ed42e4d15de5d3ef9d91f4613dad97af0c9eb24c..cc7b1d11086405e1e101fe2679d529f365fefe77 100644 --- a/src/lib_smart_rollup_node/rollup_node_daemon.ml +++ b/src/lib_smart_rollup_node/rollup_node_daemon.ml @@ -121,14 +121,8 @@ let register_outbox_messages (module Plugin : Protocol_plugin_sig.S) node_ctxt let*! outbox_messages = Plugin.Pvm.get_outbox_messages node_ctxt pvm_state ~outbox_level:level in - match outbox_messages with - | [] -> return_unit - | _ -> - let indexes = List.map fst outbox_messages in - Node_context.register_outbox_messages - node_ctxt - ~outbox_level:level - ~indexes + let indexes = List.map fst outbox_messages in + Node_context.register_outbox_messages node_ctxt ~outbox_level:level ~indexes (* Process a L1 that we have never seen and for which we have processed the predecessor. *) diff --git a/src/lib_smart_rollup_node/sql_store.ml b/src/lib_smart_rollup_node/sql_store.ml index 0d9f0614833cf0216ac329e940b21e189e773cf3..27e8e86c791af7495e94a158ca62d47f7b9e5392 100644 --- a/src/lib_smart_rollup_node/sql_store.ml +++ b/src/lib_smart_rollup_node/sql_store.ml @@ -610,6 +610,13 @@ module Outbox_messages = struct ON CONFLICT (outbox_level) DO UPDATE SET messages = $2 |sql} + let delete = + (level ->. unit) + @@ {sql| + DELETE FROM outbox_messages + where outbox_level = ? + |sql} + let range = (t2 level level ->* t2 level messages_per_level) @@ {sql| @@ -652,6 +659,10 @@ module Outbox_messages = struct with_connection store conn @@ fun conn -> Sqlite.Db.exec conn Q.register (outbox_level, indexes) + let delete_outbox_messages ?conn store ~outbox_level = + with_connection store conn @@ fun conn -> + Sqlite.Db.exec conn Q.delete outbox_level + let set_outbox_message_executed ?conn store ~outbox_level ~index = let open Lwt_result_syntax in with_connection store conn @@ fun conn -> diff --git a/src/lib_smart_rollup_node/sql_store.mli b/src/lib_smart_rollup_node/sql_store.mli index fda85f60af987c23a980be874eb708e19cf74e6e..c46eeed363fcf2eb8ad6182a9f3e26f84bae67d6 100644 --- a/src/lib_smart_rollup_node/sql_store.mli +++ b/src/lib_smart_rollup_node/sql_store.mli @@ -171,6 +171,10 @@ module Outbox_messages : sig indexes:Bitset.t -> unit tzresult Lwt.t + (** Delete outbox messages for a given outbox level. *) + val delete_outbox_messages : + ?conn:Sqlite.conn -> rw -> outbox_level:int32 -> unit tzresult Lwt.t + (** Register an outbox message as executed by its outbox level and its index in the outbox. *) val set_outbox_message_executed : diff --git a/src/lib_smart_rollup_node/store.ml b/src/lib_smart_rollup_node/store.ml index 5af23ea6601d69ead10fca0bff6d36eb02024869..655971ecc2ca1d28a9524290b374ee28899bf9af 100644 --- a/src/lib_smart_rollup_node/store.ml +++ b/src/lib_smart_rollup_node/store.ml @@ -275,6 +275,9 @@ module Outbox_messages = struct let register_outbox_messages s ~outbox_level ~indexes = Outbox_messages.register_outbox_messages (write_to s) ~outbox_level ~indexes + let delete_outbox_messages s ~outbox_level = + Outbox_messages.delete_outbox_messages (write_to s) ~outbox_level + let set_outbox_message_executed s ~outbox_level ~index = let open Lwt_result_syntax in match read_from s with diff --git a/tezt/tests/sc_rollup.ml b/tezt/tests/sc_rollup.ml index 7bd93a1bf3b2dc7391285a2d630c89ec2fb76237..e5ebf2c25f9eb99d7f0179238c4a3d7d0d93d3ba 100644 --- a/tezt/tests/sc_rollup.ml +++ b/tezt/tests/sc_rollup.ml @@ -635,6 +635,42 @@ let sc_rollup_node_disconnects_scenario sc_rollup_node sc_rollup node client = Test.fail "Refutation loop did not process after reconnection"); ] +let setup_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 () in + let* () = Client.Admin.trust_address client ~peer:node2 + and* () = Client.Admin.trust_address client2 ~peer:node in + let* () = Client.Admin.connect_address client ~peer:node2 in + let divergence ~branch1 ~branch2 = + let* identity2 = Node.wait_for_identity node2 in + let* () = Client.Admin.kick_peer client ~peer:identity2 in + let* () = branch1 client in + let* () = branch2 client2 in + let* head1 = Client.RPC.call client @@ RPC.get_chain_block_hash () + and* head2 = Client.RPC.call client2 @@ RPC.get_chain_block_hash () in + Check.((head1 <> head2) string) + ~error_msg: + "The two nodes should have diverged but didn't. Check definitions of \ + ~branch1 and ~branch2 in your call to divergence from setup_reorg." ; + (* One extra block in branch2 *) + let* () = Client.bake_for_and_wait client2 in + unit + in + let trigger_reorg () = + let* level2 = Node.get_level node2 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* head1 = Client.RPC.call client @@ RPC.get_chain_block_hash () + and* head2 = Client.RPC.call client2 @@ RPC.get_chain_block_hash () in + Check.((head1 = head2) string) + ~error_msg:"Node1 should have reorged to %R but is still on %L." ; + unit + in + return (divergence, trigger_reorg) + let setup_double_reorg node client = let nodes_args = Node.[Synchronisation_threshold 0; History_mode Archive; No_bootstrap_peers] @@ -4252,6 +4288,53 @@ let test_refutation_reward_and_punishment ~kind = unit +let originate_outbox_target_contract ~storage_ty ~init_storage ~sc_rollup + ~executor ~originator client = + Log.info "Originate target contract" ; + let prg = + Printf.sprintf + {| + { + parameter (or (%s %%default) (%s %%aux)); + storage (%s :s); + code + { + # Check that SENDER is the rollup address + SENDER; + PUSH address %S; + ASSERT_CMPEQ; + # Check that SOURCE is the implicit account used for executing + # the outbox message. + SOURCE; + PUSH address %S; + ASSERT_CMPEQ; + UNPAIR; + IF_LEFT + { SWAP ; DROP; NIL operation } + { SWAP ; DROP; NIL operation }; + PAIR; + } + } + |} + storage_ty + storage_ty + storage_ty + sc_rollup + executor + in + let* address = + Client.originate_contract + ~alias:"target" + ~amount:(Tez.of_int 100) + ~burn_cap:(Tez.of_int 100) + ~src:originator + ~prg + ~init:init_storage + client + in + let* () = Client.bake_for_and_wait client in + return address + (* Testing the execution of outbox messages ---------------------------------------- @@ -4336,51 +4419,12 @@ let test_outbox_message_generic ?supports ?regression ?expected_error in let* () = Sc_rollup_node.run ~event_level:`Debug rollup_node sc_rollup [] in let* _level = Sc_rollup_node.wait_sync ~timeout:30. rollup_node in - let originate_target_contract () = - Log.info "Originate target contract" ; - let prg = - Printf.sprintf - {| - { - parameter (or (%s %%default) (%s %%aux)); - storage (%s :s); - code - { - # Check that SENDER is the rollup address - SENDER; - PUSH address %S; - ASSERT_CMPEQ; - # Check that SOURCE is the implicit account used for executing - # the outbox message. - SOURCE; - PUSH address %S; - ASSERT_CMPEQ; - UNPAIR; - IF_LEFT - { SWAP ; DROP; NIL operation } - { SWAP ; DROP; NIL operation }; - PAIR; - } - } - |} - storage_ty - storage_ty - storage_ty - sc_rollup - src2 - in - let* address = - Client.originate_contract - ~alias:"target" - ~amount:(Tez.of_int 100) - ~burn_cap:(Tez.of_int 100) - ~src - ~prg - ~init:init_storage - client - in - let* () = Client.bake_for_and_wait client in - return address + let consumed_outputs outbox_level = + Node.RPC.call node + @@ RPC.get_chain_block_context_smart_rollups_smart_rollup_consumed_outputs + ~sc_rollup + ~outbox_level + () in let check_contract_execution address expected_storage = Log.info "Check contract execution" ; @@ -4546,51 +4590,62 @@ let test_outbox_message_generic ?supports ?regression ?expected_error return None in if Option.is_some expected_error then unit - else - let* () = check_outbox_execution outbox_level [0] `Executable in - if auto_execute_outbox then + else if auto_execute_outbox then + let* consumed = consumed_outputs outbox_level in + if consumed = [0] then (* Already executed automatically *) + unit + else + let* () = check_outbox_execution outbox_level [0] `Executable in bake_until_execute_outbox_message ~at_least:1 ~timeout:30. client rollup_node - else - let* answer = check_expected_outbox () in - match answer with - | None -> failwith "Unexpected error during proof generation" - | Some {commitment_hash; proof} -> ( - match expected_l1_error with - | None -> - let* () = check_outbox_execution outbox_level [0] `Executable in - let*! () = - Client.Sc_rollup.execute_outbox_message - ~hooks - ~burn_cap:(Tez.of_int 1) - ~rollup:sc_rollup - ~src:src2 - ~commitment_hash - ~proof - client - in - let* () = Client.bake_for_and_wait client in - let* _ = Sc_rollup_node.wait_sync rollup_node ~timeout:10. in - let* _ = Client.RPC.call client @@ RPC.get_chain_block () in - unit - (* Client.bake_for_and_wait client *) - | Some msg -> - let*? process = - Client.Sc_rollup.execute_outbox_message - ~hooks - ~burn_cap:(Tez.of_int 10) - ~rollup:sc_rollup - ~src:src2 - ~commitment_hash - ~proof - client - in - Process.check_error ~msg process) - in - let* target_contract_address = originate_target_contract () in + else + let* answer = check_expected_outbox () in + match answer with + | None -> failwith "Unexpected error during proof generation" + | Some {commitment_hash; proof} -> ( + match expected_l1_error with + | None -> + let* () = check_outbox_execution outbox_level [0] `Executable in + let*! () = + Client.Sc_rollup.execute_outbox_message + ~hooks + ~burn_cap:(Tez.of_int 1) + ~rollup:sc_rollup + ~src:src2 + ~commitment_hash + ~proof + client + in + let* () = Client.bake_for_and_wait client in + let* _ = Sc_rollup_node.wait_sync rollup_node ~timeout:10. in + let* _ = Client.RPC.call client @@ RPC.get_chain_block () in + unit + (* Client.bake_for_and_wait client *) + | Some msg -> + let*? process = + Client.Sc_rollup.execute_outbox_message + ~hooks + ~burn_cap:(Tez.of_int 10) + ~rollup:sc_rollup + ~src:src2 + ~commitment_hash + ~proof + client + in + Process.check_error ~msg process) + in + let* target_contract_address = + originate_outbox_target_contract + ~storage_ty + ~init_storage + ~sc_rollup + ~executor:src2 + ~originator:src + client + in let* _level = Sc_rollup_node.wait_sync ~timeout:30. rollup_node in let* source_contract_address = originate_forward_smart_contract client protocol @@ -4601,16 +4656,15 @@ let test_outbox_message_generic ?supports ?regression ?expected_error source_contract_address target_contract_address in - let consumed_outputs () = - Node.RPC.call node - @@ RPC.get_chain_block_context_smart_rollups_smart_rollup_consumed_outputs - ~sc_rollup - ~outbox_level - () + let* () = + if auto_execute_outbox then (* may be already executed automatically *) + unit + else + let* prior_consumed_outputs = consumed_outputs outbox_level in + Check.((prior_consumed_outputs = []) (list int)) + ~error_msg:"Expected empty list found %L for consumed outputs" ; + unit in - let* prior_consumed_outputs = consumed_outputs () in - Check.((prior_consumed_outputs = []) (list int)) - ~error_msg:"Expected empty list found %L for consumed outputs" ; let* () = trigger_outbox_message_execution ?expected_l1_error @@ -4621,7 +4675,7 @@ let test_outbox_message_generic ?supports ?regression ?expected_error | None -> let* () = if Option.is_none expected_l1_error then ( - let* after_consumed_outputs = consumed_outputs () in + let* after_consumed_outputs = consumed_outputs outbox_level in Check.((after_consumed_outputs = [message_index]) (list int)) ~error_msg:"Expected %R found %L for consumed outputs" ; unit) @@ -4718,6 +4772,80 @@ let test_outbox_message_reorg protocols ~kind = ~reorg:true protocols +let test_outbox_message_reorg_disappear ~kind = + let commitment_period = 2 and challenge_window = 2 in + test_full_scenario + ~parameters_ty:"bytes" + ~kind + ~commitment_period + ~challenge_window + { + tags = ["outbox"; "reorg"]; + variant = None; + description = + "Outbox execution when reorg without message should not crash"; + } + ~uses:(fun _protocol -> [Constant.octez_codec]) + @@ fun protocol rollup_node sc_rollup node client -> + let src = Constant.bootstrap1.public_key_hash in + let executor = Constant.bootstrap2.public_key_hash in + Sc_rollup_node.change_operators rollup_node [(Executing_outbox, executor)] ; + let* () = + Process.check @@ Sc_rollup_node.spawn_config_init rollup_node sc_rollup + in + Sc_rollup_node.Config_file.update + rollup_node + Sc_rollup_node.patch_config_execute_outbox ; + Log.info "config updated" ; + let* () = + Sc_rollup_node.run ~event_level:`Debug rollup_node sc_rollup [No_degraded] + in + let* _level = Sc_rollup_node.wait_sync ~timeout:30. rollup_node in + let* target_contract_address = + originate_outbox_target_contract + ~storage_ty:"int" + ~init_storage:"0" + ~sc_rollup + ~executor + ~originator:src + client + in + Log.info "Perform rollup execution and cement" ; + let trigger_output_message client = + let parameters_json = `O [("int", `String "37")] in + let transaction = + Sc_rollup_helpers. + { + destination = target_contract_address; + entrypoint = None; + parameters = parameters_json; + parameters_ty = None; + } + in + let* answer = + Codec.encode + ~name: + (Protocol.encoding_prefix protocol ^ ".smart_rollup.outbox.message") + (Sc_rollup_helpers.json_of_output_tx_batch [transaction]) + in + let payload = String.trim answer in + send_text_messages ~format:`Hex client [payload] + in + let* divergence, trigger_reorg = setup_reorg node client in + let* () = + (* First branch triggers an output message, while second branch has no + message on the rollup. *) + divergence ~branch1:trigger_output_message ~branch2:Client.bake_for_and_wait + in + let* outbox_level = Sc_rollup_node.wait_sync rollup_node ~timeout:10. in + let* () = trigger_reorg () in + let* _ = + bake_until_lcc_updated ~timeout:100. client rollup_node ~level:outbox_level + in + let* () = Client.bake_for_and_wait client in + let* _level = Sc_rollup_node.wait_sync ~timeout:10. rollup_node in + unit + let test_outbox_message protocols ~kind = let test (expected_error, earliness, entrypoint, message_kind, auto_execute_outbox) @@ -7249,6 +7377,7 @@ let register_protocol_independent () = protocols ; test_rollup_node_inbox ~kind ~variant:"basic" basic_scenario protocols ; test_outbox_message_reorg ~kind protocols ; + test_outbox_message_reorg_disappear ~kind protocols ; test_gc "many_gc" ~kind