diff --git a/CHANGES.rst b/CHANGES.rst index aee47474d01a10f5eceb4cb69d7a703b8071c003..a818182cf4de027e002f655702564b07ce16c54c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -149,6 +149,10 @@ Smart Rollup node and cementation status for a given block (or an estimated timestamp otherwise). (MR :gl:`!15409`) +- Fix an issue in the background store migration which could make the rollup + node send old heads in its stream at the end of the migration. (MR + :gl:`!15739`) + Smart Rollup WASM Debugger -------------------------- diff --git a/manifest/product_octez.ml b/manifest/product_octez.ml index 8333ff930391d4f2dc6b270e7aa85a7ce5eafa2f..9df6148bc2ec72249f354065f9a2101aaf129610 100644 --- a/manifest/product_octez.ml +++ b/manifest/product_octez.ml @@ -8208,7 +8208,7 @@ let _octez_smart_rollup_node_lib_tests = @ protocol_deps) in tezt - ["canary"; "test_context_gc"; "test_store_gc"] + ["canary"; "test_context_gc"; "test_store"] ~path:"src/lib_smart_rollup_node/test/" ~opam:"tezos-smart-rollup-node-lib-test" ~synopsis:"Tests for the smart rollup node library" diff --git a/src/lib_smart_rollup_node/sql_store.ml b/src/lib_smart_rollup_node/sql_store.ml index cccbc9afb0404a0b0c1d6583631acbac464d3dc3..3a68dcd10b83b325060432e8cdd4a9e4e5f7ec35 100644 --- a/src/lib_smart_rollup_node/sql_store.ml +++ b/src/lib_smart_rollup_node/sql_store.ml @@ -1168,7 +1168,9 @@ module State = struct module Make_level (N : sig val name : string - end) : S with type value := int32 = struct + end) : S with type value = int32 = struct + type value = int32 + module Q = Q (N) let set ?conn store level = @@ -1186,7 +1188,9 @@ module State = struct val name : string val type_ : value Caqti_type.t - end) : S with type value := N.value = struct + end) : S with type value = N.value = struct + type value = N.value + module Q = Q (N) let set = Q.set_value N.type_ @@ -1208,6 +1212,8 @@ module State = struct val type_ : value Caqti_type.t end) = struct + type value = N.value * int32 + module Q = Q (N) let set = Q.set_both N.type_ diff --git a/src/lib_smart_rollup_node/sql_store.mli b/src/lib_smart_rollup_node/sql_store.mli index 9c7a2daa1d3637f8f7cc8b63ed70d7e92cf90ecc..fda85f60af987c23a980be874eb708e19cf74e6e 100644 --- a/src/lib_smart_rollup_node/sql_store.mli +++ b/src/lib_smart_rollup_node/sql_store.mli @@ -370,23 +370,23 @@ module State : sig (** Only the history necessary to play refutation games is kept (i.e. after the LCC only) *) - module Finalized_level : S with type value := Block_hash.t * int32 + module Finalized_level : S with type value = Block_hash.t * int32 - module LCC : S with type value := Commitment.Hash.t * int32 + module LCC : S with type value = Commitment.Hash.t * int32 - module LPC : S with type value := Commitment.Hash.t * int32 + module LPC : S with type value = Commitment.Hash.t * int32 - module Last_gc_target : S with type value := int32 + module Last_gc_target : S with type value = int32 - module Last_gc_triggered_at : S with type value := int32 + module Last_gc_triggered_at : S with type value = int32 - module Last_successful_gc_target : S with type value := int32 + module Last_successful_gc_target : S with type value = int32 - module Last_successful_gc_triggered_at : S with type value := int32 + module Last_successful_gc_triggered_at : S with type value = int32 - module Last_context_split : S with type value := int32 + module Last_context_split : S with type value = int32 - module History_mode : S with type value := history_mode + module History_mode : S with type value = history_mode - module L2_head : S with type value := Block_hash.t * int32 + module L2_head : S with type value = Block_hash.t * int32 end diff --git a/src/lib_smart_rollup_node/store_migration.ml b/src/lib_smart_rollup_node/store_migration.ml index 7342ef700a04d1b30da60091f726cf6f0fe2be8e..0b376a66ed7299785926e0a5eaaf1501ea7e7a6d 100644 --- a/src/lib_smart_rollup_node/store_migration.ml +++ b/src/lib_smart_rollup_node/store_migration.ml @@ -709,13 +709,25 @@ module V5_sqlite_migrations = struct let migrate_rollup_node_state (v4_store : _ Store_v4.t) (v5_store : _ Store_v5.t) = let open Lwt_result_syntax in + let write_only_if_fresher (type a) + (module V5 : Sql_store.State.S with type value = a) proj value = + let* v5_value = V5.get v5_store in + match v5_value with + | Some v5_value when proj v5_value >= proj value -> + (* Data in V5 store is more up-to-date. *) + return_unit + | _ -> V5.set v5_store value + in (* migrate lcc *) let* lcc = Store_v4.Lcc.read v4_store.lcc in let* () = match lcc with | None -> return_unit | Some {commitment; level} -> - Store_v5.State.LCC.set v5_store (commitment, level) + write_only_if_fresher + (module Store_v5.State.LCC) + snd + (commitment, level) in (* migrate lpc *) let* lpc = Store_v4.Lpc.read v4_store.lpc in @@ -724,7 +736,10 @@ module V5_sqlite_migrations = struct | None -> return_unit | Some commitment -> let hash = Commitment.hash commitment in - Store_v5.State.LPC.set v5_store (hash, commitment.inbox_level) + write_only_if_fresher + (module Store_v5.State.LPC) + snd + (hash, commitment.inbox_level) in (* migrate head *) let* head = Store_v4.L2_head.read v4_store.l2_head in @@ -732,8 +747,9 @@ module V5_sqlite_migrations = struct match head with | None -> return_unit | Some head -> - Store_v5.State.L2_head.set - v5_store + write_only_if_fresher + (module Store_v5.State.L2_head) + snd (head.header.block_hash, head.header.level) in (* migrate finalized *) @@ -750,8 +766,9 @@ module V5_sqlite_migrations = struct let finalized_hash = WithExceptions.Option.get ~loc:__LOC__ finalized_hash in - Store_v5.State.Finalized_level.set - v5_store + write_only_if_fresher + (module Store_v5.State.Finalized_level) + snd (finalized_hash, finalized_level) in (* migrate context split *) @@ -761,7 +778,11 @@ module V5_sqlite_migrations = struct let* () = match split with | None -> return_unit - | Some l -> Store_v5.State.Last_context_split.set v5_store l + | Some l -> + write_only_if_fresher + (module Store_v5.State.Last_context_split) + Fun.id + l in (* migrate last GC *) let* gc = Store_v4.Gc_levels.read v4_store.gc_levels in @@ -770,9 +791,15 @@ module V5_sqlite_migrations = struct | None -> return_unit | Some {last_gc_level; first_available_level} -> let* () = - Store_v5.State.Last_gc_target.set v5_store first_available_level + write_only_if_fresher + (module Store_v5.State.Last_gc_target) + Fun.id + first_available_level in - Store_v5.State.Last_gc_triggered_at.set v5_store last_gc_level + write_only_if_fresher + (module Store_v5.State.Last_gc_triggered_at) + Fun.id + last_gc_level in (* migrate last successful GC *) let* gc = Store_v4.Gc_levels.read v4_store.successful_gc_levels in @@ -781,12 +808,14 @@ module V5_sqlite_migrations = struct | None -> return_unit | Some {last_gc_level; first_available_level} -> let* () = - Store_v5.State.Last_successful_gc_target.set - v5_store + write_only_if_fresher + (module Store_v5.State.Last_successful_gc_target) + Fun.id first_available_level in - Store_v5.State.Last_successful_gc_triggered_at.set - v5_store + write_only_if_fresher + (module Store_v5.State.Last_successful_gc_triggered_at) + Fun.id last_gc_level in (* migrate history mode *) diff --git a/src/lib_smart_rollup_node/store_v2.ml b/src/lib_smart_rollup_node/store_v2.ml index 154637228bb4b20a2908ed776df0430b2d32faba..510b00d12d16d9c3332ee2cbbfed879d4aa8b888 100644 --- a/src/lib_smart_rollup_node/store_v2.ml +++ b/src/lib_smart_rollup_node/store_v2.ml @@ -602,7 +602,8 @@ let iter_l2_blocks ?progress metadata ({l2_blocks; l2_head; _} as store) f = let+ first_level = first_available_level metadata store in let progress_bar = let total = - Int32.sub head.header.level first_level |> Int32.to_int + Int32.sub head.header.level first_level + |> Int32.succ |> Int32.to_int in Progress_bar.progress_bar ~counter:`Int ~message total in diff --git a/src/lib_smart_rollup_node/test/dune b/src/lib_smart_rollup_node/test/dune index c01c77da6e9b03b348f4a61bfaf2c607e510648a..84a2faa2d9c0435ef9f3678fd86db43f0ccdea3a 100644 --- a/src/lib_smart_rollup_node/test/dune +++ b/src/lib_smart_rollup_node/test/dune @@ -30,7 +30,7 @@ -open Octez_smart_rollup_node -open Octez_smart_rollup_node_test_helpers -open Octez_alcotezt) - (modules canary test_context_gc test_store_gc)) + (modules canary test_context_gc test_store)) (executable (name main) diff --git a/src/lib_smart_rollup_node/test/helpers/helpers.ml b/src/lib_smart_rollup_node/test/helpers/helpers.ml index fa784efe46dea237cf6b18b146702f7b327d4eb5..d48eba01faec437a91d2a484880963d95dd18760 100644 --- a/src/lib_smart_rollup_node/test/helpers/helpers.ml +++ b/src/lib_smart_rollup_node/test/helpers/helpers.ml @@ -82,7 +82,16 @@ let add_l2_genesis_block (node_ctxt : _ Node_context.t) ~boot_sector = Layer1.{hash = Block_hash.zero; level = node_ctxt.genesis_info.level} in let* () = Node_context.save_level node_ctxt head in - let predecessor = head in + let predecessor = + Layer1. + { + hash = + Block_hash.of_hex_exn + (`Hex + "0000000000000000000000000000000000000000000000000000000000000001"); + level = Int32.pred head.level; + } + in let predecessor_timestamp = Time.Protocol.epoch in let*? (module Plugin) = Protocol_plugins.proto_plugin_for_protocol @@ -97,6 +106,9 @@ let add_l2_genesis_block (node_ctxt : _ Node_context.t) ~boot_sector = in let* inbox_hash = Node_context.save_inbox node_ctxt inbox in let inbox_witness = Inbox.current_witness inbox in + let* () = + Node_context.save_messages node_ctxt inbox_witness ~level:head.level [] + in let ctxt = Context.empty node_ctxt.context in let num_ticks = 0L in let initial_tick = Z.zero in @@ -134,18 +146,21 @@ let add_l2_genesis_block (node_ctxt : _ Node_context.t) ~boot_sector = let* () = Node_context.set_l2_head node_ctxt l2_block in return l2_block -let initialize_node_context protocol ?(constants = default_constants) kind - ~boot_sector = +let initialize_node_context ?data_dir protocol ?(constants = default_constants) + kind ~boot_sector = let open Lwt_result_syntax in incr uid ; (* To avoid any conflict with previous runs of this test. *) let data_dir = - Tezt.Temp.dir - (Format.asprintf - "sc-rollup-node-test-%a-%d" - Protocol_hash.pp_short - protocol - !uid) + match data_dir with + | Some d -> d + | None -> + Tezt.Temp.dir + (Format.asprintf + "sc-rollup-node-test-%a-%d" + Protocol_hash.pp_short + protocol + !uid) in let base_dir = Tezt.Temp.dir @@ -176,10 +191,10 @@ let initialize_node_context protocol ?(constants = default_constants) kind in return (ctxt, genesis) -let with_node_context ?constants kind protocol ~boot_sector f = +let with_node_context ?data_dir ?constants kind protocol ~boot_sector f = let open Lwt_result_syntax in let* node_ctxt, genesis = - initialize_node_context protocol ?constants kind ~boot_sector + initialize_node_context ?data_dir protocol ?constants kind ~boot_sector in Lwt.finalize (fun () -> f node_ctxt ~genesis) @@ fun () -> let open Lwt_syntax in @@ -234,7 +249,15 @@ let add_l2_block (node_ctxt : _ Node_context.t) ?(is_first_block = false) level = predecessor_l2_block.header.level; } in - let* () = Node_context.set_l2_head node_ctxt predecessor_l2_block in + let* old_head = Node_context.last_processed_head_opt node_ctxt in + let* () = + match old_head with + | Some o + when Block_hash.( + o.header.block_hash = predecessor_l2_block.header.block_hash) -> + return_unit + | _ -> Node_context.set_l2_head node_ctxt predecessor_l2_block + in let pred_level = predecessor_l2_block.header.level in let head = make_header @@ -258,6 +281,7 @@ let add_l2_block (node_ctxt : _ Node_context.t) ?(is_first_block = false) let append_l2_block (node_ctxt : _ Node_context.t) ?(is_first_block = false) messages = let open Lwt_result_syntax in + let*! () = Lwt.pause () in let* predecessor_l2_block = Node_context.last_processed_head_opt node_ctxt in let* predecessor_l2_block = match predecessor_l2_block with @@ -282,6 +306,117 @@ let append_dummy_l2_chain node_ctxt ~length = in append_l2_blocks node_ctxt batches +module V4 = struct + open Octez_smart_rollup_node_store + + let migrate_back_block (v4_store : Store_v4.rw) (node_ctxt : _ Node_context.t) + (block : Sc_rollup_block.t) = + let open Lwt_result_syntax in + let* () = + Store_v4.Levels_to_hashes.add + v4_store.levels_to_hashes + block.header.level + block.header.block_hash + in + let* () = + Option.iter_es + (fun commitment_hash -> + let* commitment = + Node_context.get_commitment node_ctxt commitment_hash + in + Store_v4.Commitments.append + v4_store.commitments + ~key:commitment_hash + ~value:commitment) + block.header.commitment_hash + in + let* inbox = Node_context.get_inbox node_ctxt block.header.inbox_hash in + let* () = + Store_v4.Inboxes.append + v4_store.inboxes + ~key:block.header.inbox_hash + ~value:inbox + in + let* messages = + Node_context.find_messages node_ctxt block.header.inbox_witness + in + let* () = + Store_v4.Messages.append + v4_store.messages + ~key:block.header.inbox_witness + ~header:block.header.predecessor + ~value:(Option.value messages ~default:[]) + in + let* () = + Store_v4.L2_blocks.append + v4_store.l2_blocks + ~key:block.header.block_hash + ~header:block.header + ~value:{block with header = ()} + in + let* () = Store_v4.L2_head.write v4_store.l2_head block in + return_unit + + let migrate_back v4_store node_ctxt chain = + List.iter_es (migrate_back_block v4_store node_ctxt) chain + + let with_store_migration_node_context ?constants kind protocol ~boot_sector + ~chain_size f = + let open Lwt_result_syntax in + let* data_dir, cctxt, current_protocol, genesis, chain = + with_node_context ?constants kind protocol ~boot_sector + @@ fun node_ctxt ~genesis -> + let* chain = append_dummy_l2_chain node_ctxt ~length:chain_size in + let* v4_store = + Store_v4.load + Read_write + ~index_buffer_size:1000 + ~l2_blocks_cache_size:1 + (Configuration.default_storage_dir node_ctxt.data_dir) + in + let chain = genesis :: chain in + let* () = migrate_back v4_store node_ctxt chain in + let* () = Store_v4.close v4_store in + return + ( node_ctxt.data_dir, + node_ctxt.cctxt, + Reference.get node_ctxt.current_protocol, + genesis, + chain ) + in + (* Clean up V5 store *) + let*! () = + List.iter_s + (fun f -> + Lwt.catch + (fun () -> Lwt_unix.unlink (Filename.concat data_dir f)) + (fun _ -> Lwt.return_unit)) + Sql_store.(sqlite_file_name :: extra_sqlite_files) + in + let* () = + Store_version.write_version_file + ~dir:(Configuration.default_storage_dir data_dir) + Store_v4.version + in + let* ctxt = + Node_context_loader.Internal_for_tests.create_node_context + cctxt + current_protocol + ~data_dir + kind + in + let commitment_hash = + WithExceptions.Option.get ~loc:__LOC__ genesis.header.commitment_hash + in + let ctxt = + {ctxt with genesis_info = {ctxt.genesis_info with commitment_hash}} + in + Lwt.finalize (fun () -> f ctxt chain) @@ fun () -> + let open Lwt_syntax in + let* _ = Node_context_loader.close ctxt in + return_unit +end + module Assert = struct module Make_with_encoding (X : sig type t @@ -325,3 +460,31 @@ let alcotest ?name speed ?constants kind protocol ~boot_sector f = | Error err -> Format.printf "@\n%a@." pp_print_trace err ; Lwt.fail Alcotest.Test_error + +let store_migration_alcotest ?name speed ?constants kind protocol ~boot_sector + ~chain_size f = + let name = + Format.asprintf + "%s%a %a" + (match name with None -> "" | Some n -> n ^ " - ") + Protocol_hash.pp_short + protocol + Kind.pp + kind + in + Alcotest_lwt.test_case name speed @@ fun _lwt_switch () -> + let open Lwt_result_syntax in + let*! r = + V4.with_store_migration_node_context + ?constants + kind + protocol + ~boot_sector + ~chain_size + f + in + match r with + | Ok () -> Lwt.return_unit + | Error err -> + Format.printf "@\n%a@." pp_print_trace err ; + Lwt.fail Alcotest.Test_error diff --git a/src/lib_smart_rollup_node/test/helpers/helpers.mli b/src/lib_smart_rollup_node/test/helpers/helpers.mli index 006a163183ad8b3b30b16611ffe0578da9e557e3..9ee29740e253ecb281ceae46ad4a39225812c7fb 100644 --- a/src/lib_smart_rollup_node/test/helpers/helpers.mli +++ b/src/lib_smart_rollup_node/test/helpers/helpers.mli @@ -38,6 +38,7 @@ open Octez_smart_rollup_node node context is properly closed. Test that need a node context need to use this function in order to avoid file descriptor leaks. *) val with_node_context : + ?data_dir:string -> ?constants:Rollup_constants.protocol_constants -> Kind.t -> Protocol_hash.t -> @@ -127,3 +128,17 @@ val alcotest : genesis:Sc_rollup_block.t -> unit tzresult Lwt.t) -> unit Alcotest_lwt.test_case + +(** Build an alcotest test case that executes with node context initialized with + a chain of size [chain_size] in the previous (V4) store in order to run + store migration tests. *) +val store_migration_alcotest : + ?name:string -> + Alcotest_lwt.speed_level -> + ?constants:Rollup_constants.protocol_constants -> + Kind.t -> + Protocol_hash.t -> + boot_sector:string -> + chain_size:int -> + (Node_context.rw -> Sc_rollup_block.t list -> unit tzresult Lwt.t) -> + unit Alcotest_lwt.test_case diff --git a/src/lib_smart_rollup_node/test/test_store_gc.ml b/src/lib_smart_rollup_node/test/test_store.ml similarity index 64% rename from src/lib_smart_rollup_node/test/test_store_gc.ml rename to src/lib_smart_rollup_node/test/test_store.ml index 73b969104fc8156549a5ad70bff699b81075dbc1..b64f009b85d003475f717f948eeb54d4bd2bf9e5 100644 --- a/src/lib_smart_rollup_node/test/test_store_gc.ml +++ b/src/lib_smart_rollup_node/test/test_store.ml @@ -9,8 +9,8 @@ ------- Component: Smart rollup node library Invocation: dune exec src/lib_smart_rollup_node/test/main.exe \ - -- -f src/lib_smart_rollup_node/test/test_store_gc.ml - Subject: Unit tests GC for rollup node store + -- -f src/lib_smart_rollup_node/test/test_store.ml + Subject: Unit tests for rollup node store *) let build_chain node_ctxt ~genesis ~length = @@ -188,14 +188,126 @@ let gc_test_reorg node_ctxt ~genesis = in return_unit +let rec wait_migration (store : _ Store.t) = + let open Lwt_syntax in + match store with + | Normal _ | Hybrid {migration_done = {contents = true}; _} -> Lwt.return_unit + | Hybrid {migration_done = {contents = false}; _} -> + let* () = Lwt_unix.sleep 0.1 in + wait_migration store + +let test_store_migration_head node_ctxt chain = + let open Lwt_result_syntax in + let additional_chain_size = 10 in + let* extra_chain = + Helpers.append_dummy_l2_chain node_ctxt ~length:additional_chain_size + in + let store = Node_context.Internal_for_tests.unsafe_get_store node_ctxt in + let*! () = wait_migration store in + let* head = Node_context.last_processed_head_opt node_ctxt in + let* () = + match head with + | None -> Assert.fail_msg "No head after migration" + | Some head -> + let last = + List.last_opt extra_chain |> WithExceptions.Option.get ~loc:__LOC__ + in + Assert.equal + ~loc:__LOC__ + ~pp:(fun fmt -> Format.fprintf fmt "%ld") + ~msg:"L2 head is correct after migration" + head.header.level + last.header.level ; + return_unit + in + check_chain_ok ~gc_level:0l node_ctxt store (chain @ extra_chain) + +let test_store_migration_stream (node_ctxt : _ Node_context.t) chain = + let open Lwt_result_syntax in + let additional_chain_size1 = 500 in + let additional_chain_size2 = 100 in + let block_stream, stopper = + Lwt_watcher.create_stream node_ctxt.global_block_watcher + in + let extra_chain1 = + Helpers.append_dummy_l2_chain node_ctxt ~length:additional_chain_size1 + in + let prev = ref 0l in + let stream_check = + Lwt_stream.iter + (fun (block : Sc_rollup_block.t) -> + Assert.lt + ~loc:__LOC__ + ~pp:(fun fmt -> Format.fprintf fmt "%ld") + ~msg:"L2 blocks streamed in order" + !prev + block.header.level ; + prev := block.header.level) + block_stream + in + let* extra_chain1 in + let store = Node_context.Internal_for_tests.unsafe_get_store node_ctxt in + let*! () = wait_migration store in + let* extra_chain2 = + Helpers.append_dummy_l2_chain node_ctxt ~length:additional_chain_size2 + in + Lwt_watcher.shutdown stopper ; + let*! () = stream_check in + let* head = Node_context.last_processed_head_opt node_ctxt in + let* () = + match head with + | None -> Assert.fail_msg "No head after migration" + | Some head -> + let last = + List.last_opt extra_chain2 |> WithExceptions.Option.get ~loc:__LOC__ + in + Assert.equal + ~loc:__LOC__ + ~pp:(fun fmt -> Format.fprintf fmt "%ld") + ~msg:"L2 head is correct after migration" + head.header.level + last.header.level ; + Assert.equal + ~loc:__LOC__ + ~pp:(fun fmt -> Format.fprintf fmt "%ld") + ~msg:"Last streamed block is correct" + !prev + last.header.level ; + return_unit + in + check_chain_ok + ~gc_level:0l + node_ctxt + store + (chain @ extra_chain1 @ extra_chain2) + let mk_tests t = List.map (fun proto -> Helpers.alcotest `Quick Wasm_2_0_0 proto ~boot_sector:"" t) (Protocol_plugins.registered_protocols ()) +let mk_store_migration_tests t = + List.map + (fun proto -> + Helpers.store_migration_alcotest + `Quick + Wasm_2_0_0 + proto + ~boot_sector:"" + ~chain_size:100 + t) + (Protocol_plugins.registered_protocols ()) + let () = Alcotest_lwt.run ~__FILE__ "lib_smart_rollup_node" - [("store_gc", mk_tests gc_test); ("store_gc_reorg", mk_tests gc_test_reorg)] + [ + ("store_gc", mk_tests gc_test); + ("store_gc_reorg", mk_tests gc_test_reorg); + ( "store_migration_head", + mk_store_migration_tests test_store_migration_head ); + ( "store_migration_stream", + mk_store_migration_tests test_store_migration_stream ); + ] |> Lwt_main.run