From a9b7d6950f8e97857e4d28d4d9b5ba1f6d60d0ab Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Mon, 14 Nov 2022 14:41:20 +0000 Subject: [PATCH 1/4] Proto: change type of PVM_with_context_and_state.reveal --- src/proto_alpha/bin_sc_rollup_node/refutation_game.ml | 3 ++- src/proto_alpha/lib_protocol/alpha_context.mli | 2 +- src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml | 5 +++-- src/proto_alpha/lib_protocol/sc_rollup_proof_repr.mli | 2 +- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml b/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml index b6d1a0b495ee..706cb879c253 100644 --- a/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml +++ b/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml @@ -216,7 +216,8 @@ module Make (Interpreter : Interpreter.S) : let state = start_state let reveal hash = - Reveals.get ~data_dir:node_ctxt.data_dir ~pvm_name:PVM.name ~hash + Lwt.return + @@ Reveals.get ~data_dir:node_ctxt.data_dir ~pvm_name:PVM.name ~hash module Inbox_with_history = struct include Context.Inbox diff --git a/src/proto_alpha/lib_protocol/alpha_context.mli b/src/proto_alpha/lib_protocol/alpha_context.mli index 13a69ba6733a..7c3f9ebfb99b 100644 --- a/src/proto_alpha/lib_protocol/alpha_context.mli +++ b/src/proto_alpha/lib_protocol/alpha_context.mli @@ -3746,7 +3746,7 @@ module Sc_rollup : sig val proof_encoding : proof Data_encoding.t - val reveal : Reveal_hash.t -> string option + val reveal : Reveal_hash.t -> string option Lwt.t module Inbox_with_history : sig include Inbox.Merkelized_operations with type inbox_context = context diff --git a/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml b/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml index 89837840437e..4ea5aeb0a600 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml +++ b/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.ml @@ -353,7 +353,7 @@ module type PVM_with_context_and_state = sig val proof_encoding : proof Data_encoding.t - val reveal : Sc_rollup_PVM_sig.Reveal_hash.t -> string option + val reveal : Sc_rollup_PVM_sig.Reveal_hash.t -> string option Lwt.t module Inbox_with_history : sig include @@ -423,7 +423,8 @@ let produce ~metadata pvm_and_state commit_level = in return (Some inbox_proof, input) | Needs_reveal (Reveal_raw_data h) -> ( - match reveal h with + let*! res = reveal h in + match res with | None -> proof_error "No reveal" | Some data -> return diff --git a/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.mli b/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.mli index ba90592e0725..f8ec62e6b800 100644 --- a/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.mli +++ b/src/proto_alpha/lib_protocol/sc_rollup_proof_repr.mli @@ -155,7 +155,7 @@ module type PVM_with_context_and_state = sig val proof_encoding : proof Data_encoding.t - val reveal : Sc_rollup_PVM_sig.Reveal_hash.t -> string option + val reveal : Sc_rollup_PVM_sig.Reveal_hash.t -> string option Lwt.t module Inbox_with_history : sig include -- GitLab From e8981d6865f5248db3f70cd830dcd52194461518 Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Mon, 14 Nov 2022 14:41:39 +0000 Subject: [PATCH 2/4] Dac/Rollup node: Better UX for reveal data --- .../bin_sc_rollup_node/fueled_pvm.ml | 16 ++--- .../bin_sc_rollup_node/refutation_game.ml | 7 +- src/proto_alpha/bin_sc_rollup_node/reveals.ml | 70 ++++++++++++++++--- .../bin_sc_rollup_node/reveals.mli | 15 ++-- 4 files changed, 82 insertions(+), 26 deletions(-) diff --git a/src/proto_alpha/bin_sc_rollup_node/fueled_pvm.ml b/src/proto_alpha/bin_sc_rollup_node/fueled_pvm.ml index f2f261e05f20..ef11d4374e99 100644 --- a/src/proto_alpha/bin_sc_rollup_node/fueled_pvm.ml +++ b/src/proto_alpha/bin_sc_rollup_node/fueled_pvm.ml @@ -106,16 +106,12 @@ module Make failing_ticks next_state | Needs_reveal (Reveal_raw_data hash) -> ( - match Reveals.get ~data_dir ~pvm_name:PVM.name ~hash with - | None -> tzfail (Sc_rollup_node_errors.Cannot_retrieve_reveal hash) - | Some data -> ( - let*! next_state = - PVM.set_input (Reveal (Raw_data data)) state - in - match F.consume F.one_tick_consumption fuel with - | None -> return (state, fuel, current_tick, failing_ticks) - | Some fuel -> - go fuel (Int64.succ current_tick) failing_ticks next_state)) + let* data = Reveals.get ~data_dir ~pvm_name:PVM.name ~hash in + let*! next_state = PVM.set_input (Reveal (Raw_data data)) state in + match F.consume F.one_tick_consumption fuel with + | None -> return (state, fuel, current_tick, failing_ticks) + | Some fuel -> + go fuel (Int64.succ current_tick) failing_ticks next_state) | Needs_reveal Reveal_metadata -> ( let*! next_state = PVM.set_input (Reveal (Metadata metadata)) state diff --git a/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml b/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml index 706cb879c253..0cbce2f8efad 100644 --- a/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml +++ b/src/proto_alpha/bin_sc_rollup_node/refutation_game.ml @@ -216,8 +216,11 @@ module Make (Interpreter : Interpreter.S) : let state = start_state let reveal hash = - Lwt.return - @@ Reveals.get ~data_dir:node_ctxt.data_dir ~pvm_name:PVM.name ~hash + let open Lwt_syntax in + let* res = + Reveals.get ~data_dir:node_ctxt.data_dir ~pvm_name:PVM.name ~hash + in + match res with Ok data -> return @@ Some data | Error _ -> return None module Inbox_with_history = struct include Context.Inbox diff --git a/src/proto_alpha/bin_sc_rollup_node/reveals.ml b/src/proto_alpha/bin_sc_rollup_node/reveals.ml index 16c487c76b5f..4f0523afefa3 100644 --- a/src/proto_alpha/bin_sc_rollup_node/reveals.ml +++ b/src/proto_alpha/bin_sc_rollup_node/reveals.ml @@ -23,11 +23,55 @@ (* *) (*****************************************************************************) -let string_of_file filename = - let cin = open_in filename in - let s = really_input_string cin (in_channel_length cin) in - close_in cin ; - s +module Reveal_hash = Protocol.Alpha_context.Sc_rollup.Reveal_hash + +type error += + | Wrong_hash of {found : Reveal_hash.t; expected : Reveal_hash.t} + | Could_not_open_preimage_file of String.t + +let () = + register_error_kind + ~id:"sc_rollup.node.wrong_hash_of_reveal_preimage" + ~title:"Hash of reveal preimage is not correct" + ~description:"Hash of reveal preimage is not correct." + ~pp:(fun ppf (found, expected) -> + Format.fprintf + ppf + "The hash of reveal preimage is %a while a value of %a is expected" + Reveal_hash.pp + found + Reveal_hash.pp + expected) + `Permanent + Data_encoding.( + obj2 + (req "found" Reveal_hash.encoding) + (req "expected" Reveal_hash.encoding)) + (function + | Wrong_hash {found; expected} -> Some (found, expected) | _ -> None) + (fun (found, expected) -> Wrong_hash {found; expected}) ; + register_error_kind + ~id:"sc_rollup.node.could_not_open_reveal_preimage_file" + ~title:"Could not open reveal preimage file" + ~description:"Could not open reveal preimage file." + ~pp:(fun ppf hash -> + Format.fprintf + ppf + "Could not open file containing preimage of reveal hash %s" + hash) + `Permanent + Data_encoding.(obj1 (req "hash" string)) + (function + | Could_not_open_preimage_file filename -> Some filename | _ -> None) + (fun filename -> Could_not_open_preimage_file filename) + +let file_contents filename = + let open Lwt_result_syntax in + Lwt.catch + (fun () -> + let*! contents = Lwt_utils_unix.read_file filename in + return contents) + (fun _ -> tzfail @@ Could_not_open_preimage_file filename) let save_string filename s = let cout = open_out filename in @@ -35,9 +79,7 @@ let save_string filename s = close_out cout let path data_dir pvm_name hash = - let hash = - Format.asprintf "%a" Protocol.Alpha_context.Sc_rollup.Reveal_hash.pp hash - in + let hash = Format.asprintf "%a" Reveal_hash.pp hash in Filename.(concat (concat data_dir pvm_name) hash) let ensure_dir_exists data_dir pvm_name = @@ -48,7 +90,16 @@ let ensure_dir_exists data_dir pvm_name = else Sys.mkdir path 0o700 let get ~data_dir ~pvm_name ~hash = - try Some (string_of_file (path data_dir pvm_name hash)) with _ -> None + let open Lwt_result_syntax in + let filename = path data_dir pvm_name hash in + let* contents = file_contents filename in + let*? () = + let contents_hash = Reveal_hash.hash_string [contents] in + error_unless + (Reveal_hash.equal contents_hash hash) + (Wrong_hash {found = contents_hash; expected = hash}) + in + return contents module Arith = struct let pvm_name = @@ -115,7 +166,6 @@ module Arith = struct let rec aux successor_hash linked_chunks = function | [] -> linked_chunks | chunk :: rev_chunks -> - let open Protocol.Alpha_context.Sc_rollup in let cell = match successor_hash with | None -> chunk diff --git a/src/proto_alpha/bin_sc_rollup_node/reveals.mli b/src/proto_alpha/bin_sc_rollup_node/reveals.mli index d2ad0657cd61..10abf2d3d7f0 100644 --- a/src/proto_alpha/bin_sc_rollup_node/reveals.mli +++ b/src/proto_alpha/bin_sc_rollup_node/reveals.mli @@ -50,14 +50,21 @@ open Protocol.Alpha_context -(** [get ~data_dir ~pvm_name ~hash] returns [Some data] such that - [Input_hash.hash_string [data] = hash]. If such [data] is known - to the rollup node. Otherwise, returns [None]. *) +(** [get ~data_dir ~pvm_name ~hash] retrieves the data associated with + the reveal hash [hash] from disk. May fail with: + {ul + {li [Wrong_hash {found; expected}] where [expected = hash], and + [found <> hash], if the data is retrieved and hashes to the wrong + hash [found],} + {li [Could_not_open_preimage_file filename] if the function tries to + retrieve the data from [filename], but it cannot read the contents + of the file.} + } *) val get : data_dir:string -> pvm_name:string -> hash:Sc_rollup.Reveal_hash.t -> - string option + string tzresult Lwt.t (** [import ~data_dir ~pvm_name ~filename] turns the content of ~filename into a chunk of pages of (at most) 4KB, returning the hash of the first -- GitLab From 8cbe98c14f21dfb120161f31e2718471daa8d3db Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Wed, 16 Nov 2022 10:45:38 +0000 Subject: [PATCH 3/4] Proto: Remove Sc_rollup_errors.Cannot_retrieve_reveal --- .../sc_rollup_node_errors.ml | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/src/proto_alpha/bin_sc_rollup_node/sc_rollup_node_errors.ml b/src/proto_alpha/bin_sc_rollup_node/sc_rollup_node_errors.ml index f42609b47575..281ba1165498 100644 --- a/src/proto_alpha/bin_sc_rollup_node/sc_rollup_node_errors.ml +++ b/src/proto_alpha/bin_sc_rollup_node/sc_rollup_node_errors.ml @@ -40,7 +40,6 @@ type error += } | Missing_PVM_state of Tezos_crypto.Block_hash.t * Int32.t | Cannot_checkout_context of Tezos_crypto.Block_hash.t * string option - | Cannot_retrieve_reveal of Sc_rollup.Reveal_hash.t type error += | Lost_game of @@ -235,19 +234,4 @@ let () = (function | Lost_game (loser, reason, slashed) -> Some (loser, reason, slashed) | _ -> None) - (fun (loser, reason, slashed) -> Lost_game (loser, reason, slashed)) ; - - register_error_kind - `Permanent - ~id:"internal.cannot_retrieve_reveal" - ~title:"Internal error: Cannot retrieve reveal of hash" - ~description:"The rollup node cannot retrieve a reveal asked by the rollup." - ~pp:(fun ppf hash -> - Format.fprintf - ppf - "The node cannot retrieve a reveal for hash %a" - Sc_rollup.Reveal_hash.pp - hash) - Data_encoding.(obj1 (req "hash" Sc_rollup.Reveal_hash.encoding)) - (function Cannot_retrieve_reveal hash -> Some hash | _ -> None) - (fun hash -> Cannot_retrieve_reveal hash) + (fun (loser, reason, slashed) -> Lost_game (loser, reason, slashed)) -- GitLab From 28dd40b97b660efb515c3f6c658fd78f9ca8fcb9 Mon Sep 17 00:00:00 2001 From: Andrea Cerone Date: Mon, 14 Nov 2022 14:42:13 +0000 Subject: [PATCH 4/4] Dac/Tezt: rollup node stops if reveal data hash is wrong --- tezt/lib_tezos/sc_rollup_node.ml | 9 +++++++ tezt/lib_tezos/sc_rollup_node.mli | 10 +++++++ tezt/tests/sc_rollup.ml | 45 ++++++++++++++++++++++++++++--- 3 files changed, 61 insertions(+), 3 deletions(-) diff --git a/tezt/lib_tezos/sc_rollup_node.ml b/tezt/lib_tezos/sc_rollup_node.ml index ebc59d52a88d..ccda3a431b01 100644 --- a/tezt/lib_tezos/sc_rollup_node.ml +++ b/tezt/lib_tezos/sc_rollup_node.ml @@ -76,6 +76,15 @@ let wait sc_node = (name sc_node) | Running {process; _} -> Process.wait process +let wait_for_failure_handler sc_node = + match sc_node.status with + | Not_running -> + Test.fail + "Smart-contract rollup node %s is not running, cannot wait for it to \ + terminate" + (name sc_node) + | Running {process; _} -> fun () -> Process.check ~expect_failure:true process + let name sc_node = sc_node.name let rpc_host sc_node = sc_node.persistent_state.rpc_host diff --git a/tezt/lib_tezos/sc_rollup_node.mli b/tezt/lib_tezos/sc_rollup_node.mli index cad11ff23360..18b862783191 100644 --- a/tezt/lib_tezos/sc_rollup_node.mli +++ b/tezt/lib_tezos/sc_rollup_node.mli @@ -111,6 +111,16 @@ val run : t -> string list -> unit Lwt.t running, make the test fail. *) val wait : t -> Unix.process_status Lwt.t +(** [wait_for_failure_handler sc_node] returns a function of type + [unit -> unit Lwt.t] which, when invoked, waits for sc_node to terminate + and then checks that its exit status code is different from 0. If + the rollup node terminated before the function handler is invoked, then + only the exit code is checked. If the rollup node is not running when the + function is invoked, the test where the function is invoked fails. If the + handler is invoked and the rollup node terminates with status code 0, + then the test where the function is invoked fails. *) +val wait_for_failure_handler : t -> unit -> unit Lwt.t + (** Send SIGTERM and wait for the process to terminate. Default [timeout] is 30 seconds, after which SIGKILL is sent. *) diff --git a/tezt/tests/sc_rollup.ml b/tezt/tests/sc_rollup.ml index 23754180dd7e..db5ce1fa3ebe 100644 --- a/tezt/tests/sc_rollup.ml +++ b/tezt/tests/sc_rollup.ml @@ -2074,9 +2074,7 @@ let test_rollup_arith_uses_reveals ~kind = close_out cout ; filename in - let* hash = - Sc_rollup_node.import sc_rollup_node ~pvm_name:"arith" ~filename - in + let* hash = Sc_rollup_node.import sc_rollup_node ~pvm_name:kind ~filename in let* genesis_info = RPC.Client.call ~hooks client @@ RPC.get_chain_block_context_sc_rollups_sc_rollup_genesis_info sc_rollup @@ -2111,6 +2109,46 @@ let test_rollup_arith_uses_reveals ~kind = (value = nadd) int ~error_msg:"Invalid value in rollup state (%L <> %R)") ; unit +let test_reveals_fails_on_wrong_hash ~kind = + test_full_scenario + ~timeout:120 + ~kind + { + tags = ["reveals"]; + variant = None; + description = "reveal data fails with wrong hash"; + } + @@ fun sc_rollup_node _sc_rollup_client sc_rollup _node client -> + (* This value has been obtained from the logs of the test + scrrh1kXE3tnCVTJ21aDNVeaV86e8rS6jtiMEDpjZJtDnLXRThQdmy. *) + let hash = "scrrh1kXE3tnCVTJ21aDNVeaV86e8rS6jtiMEDpjZJtDnLXRThQdmy" in + let pvm_dir = + Filename.concat (Sc_rollup_node.data_dir sc_rollup_node) "arith" + in + let filename = Filename.concat pvm_dir hash in + let () = Sys.mkdir pvm_dir 0o700 in + let cout = open_out filename in + let () = output_string cout "Some data that is not related to the hash" in + let () = close_out cout in + let* genesis_info = + RPC.Client.call ~hooks client + @@ RPC.get_chain_block_context_sc_rollups_sc_rollup_genesis_info sc_rollup + in + let init_level = JSON.(genesis_info |-> "level" |> as_int) in + + let* () = Sc_rollup_node.run sc_rollup_node [] in + (* Prepare the handler to wait for the rollup node to fail before + sending the L1 message that will trigger the failure. This + ensures that the failure handler can access the status code + of the rollup node even after it has terminated. *) + let expect_failure = Sc_rollup_node.wait_for_failure_handler sc_rollup_node in + let* _level = + Sc_rollup_node.wait_for_level ~timeout:120. sc_rollup_node init_level + in + + let* () = send_text_messages client ["hash:" ^ hash] in + expect_failure () + (* TODO: https://gitlab.com/tezos/tezos/-/issues/4147 remove the need to have a scoru to run wallet command. In tezt the @@ -3283,6 +3321,7 @@ let register ~protocols = ~internal:false ; (* DAC tests, not supported yet by the Wasm PVM *) test_rollup_arith_uses_reveals protocols ~kind:"arith" ; + test_reveals_fails_on_wrong_hash protocols ~kind:"arith" ; (* Shared tezts - will be executed for both PVMs. *) register ~kind:"wasm_2_0_0" ~protocols ; register ~kind:"arith" ~protocols -- GitLab