From d000739eacc37307b9eead51871fb9746941d2c0 Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Tue, 20 Sep 2022 08:58:55 +0200 Subject: [PATCH 1/4] Tezt: add protocol table branch switch test --- tezt/tests/protocol_table_update.ml | 43 +++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/tezt/tests/protocol_table_update.ml b/tezt/tests/protocol_table_update.ml index 61a2dbcd0fc8..3412eb433aed 100644 --- a/tezt/tests/protocol_table_update.ml +++ b/tezt/tests/protocol_table_update.ml @@ -26,7 +26,7 @@ (* Testing ------- Component: Store/protocol activation - Invocation: dune exec tezt/tests/main.exe -- protocol table update + Invocation: dune exec tezt/tests/main.exe -- --file protocol_table_update.ml Subject: Checks if a protocol activation is well handled in the store *) @@ -174,5 +174,44 @@ let test_protocol_table_update ~migrate_from ~migrate_to = Test.fail "Activation block must be equal." ; Lwt.return_unit +(** Checks that a protocol table is correctly updated after a branch + switch where the new branch has a protocol activation through an + user activated upgrade. *) +let test_branch_switch ~migrate_from ~migrate_to = + Test.register + ~__FILE__ + ~title:"protocol table" + ~tags:["protocol"; "table"; "branch"] + @@ fun () -> + let migration_level = 2 in + let uau = + Node.Config_file.set_sandbox_network_with_user_activated_upgrades + [(migration_level, migrate_to)] + in + let* node1 = Node.init ~patch_config:uau [Synchronisation_threshold 0] in + let* client1 = Client.(init ~endpoint:(Node node1) ()) in + let* () = Client.activate_protocol ~protocol:migrate_from client1 in + let* node2 = Node.init [Synchronisation_threshold 0] in + let* client2 = Client.(init ~endpoint:(Node node2) ()) in + let* () = Client.Admin.connect_address ~peer:node1 client2 in + let* () = repeat 2 (fun () -> Client.bake_for_and_wait client1) in + let* _ = Node.wait_for_level node1 3 in + let* () = Client.bake_for_and_wait client2 in + let* _ = Node.wait_for_level node2 2 in + let* () = Node.terminate node2 in + let () = Node.Config_file.update node2 uau in + let* () = Node.run node2 [Synchronisation_threshold 0] in + let* () = Node.wait_for_ready node2 in + (* The block is invalid but is still accepted. *) + let* () = Client.Admin.trust_address ~peer:node1 client2 in + let* () = Client.Admin.connect_address ~peer:node1 client2 in + let* _ = Node.wait_for_level node2 3 in + let path = ["chains"; "main"; "blocks"; "head"; "protocols"] in + let* _ = Client.rpc GET path client1 in + let path = ["chains"; "main"; "blocks"; "head"; "protocols"] in + let* _ = Client.rpc GET path client2 in + unit + let register ~migrate_from ~migrate_to = - test_protocol_table_update ~migrate_from ~migrate_to + test_protocol_table_update ~migrate_from ~migrate_to ; + test_branch_switch ~migrate_from ~migrate_to -- GitLab From de1ab0d48762bafa8291b8201b6351d87005d46d Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Tue, 20 Sep 2022 11:50:53 +0200 Subject: [PATCH 2/4] Shell: fix protocol activation table while switching branch --- src/lib_shell/chain_validator.ml | 13 ++++++------- src/lib_store/store.mli | 8 ++++---- src/lib_store/unix/store.ml | 29 ++++++++++++++++++++++++++++- src/lib_store/unix/store.mli | 8 ++++---- 4 files changed, 42 insertions(+), 16 deletions(-) diff --git a/src/lib_shell/chain_validator.ml b/src/lib_shell/chain_validator.ml index a385ac3aa89b..e4844fb9b147 100644 --- a/src/lib_shell/chain_validator.ml +++ b/src/lib_shell/chain_validator.ml @@ -485,18 +485,17 @@ let on_validation_request w peer start_testchain active_chains spawn_child block else Lwt.return_unit in Lwt_watcher.notify nv.new_head_input block ; - let is_branch_switch = + let is_head_increment = Block_hash.equal head_hash block_header.shell.predecessor in let event = - if is_branch_switch then Head_increment else Branch_switch + if is_head_increment then Head_increment else Branch_switch in let* () = - if is_branch_switch then - Store.Chain.may_update_ancestor_protocol_level - chain_store - ~head:block - else return_unit + when_ (not is_head_increment) (fun () -> + Store.Chain.may_update_ancestor_protocol_level + chain_store + ~head:block) in let*! () = may_synchronise_context nv.synchronisation_state chain_store diff --git a/src/lib_store/store.mli b/src/lib_store/store.mli index 6f3930eee255..489ebabe4f60 100644 --- a/src/lib_store/store.mli +++ b/src/lib_store/store.mli @@ -894,10 +894,10 @@ module Chain : sig unit tzresult Lwt.t (** [may_update_ancestor_protocol_level chain_store ~head] tries to - find the activation block of the [head]'s protocol, checks that - its an ancestor and tries to update it if that's not the case. If - the registered activation block is not reachable (already - pruned), this function does nothing. *) + find the activation block of the [head]'s protocol, checks that + its an ancestor and tries to update it if that's not the case. If + the registered activation block is not reachable (already + pruned), this function does nothing. *) val may_update_ancestor_protocol_level : chain_store -> head:Block.block -> unit tzresult Lwt.t diff --git a/src/lib_store/unix/store.ml b/src/lib_store/unix/store.ml index 629575894029..162b7136b1f4 100644 --- a/src/lib_store/unix/store.ml +++ b/src/lib_store/unix/store.ml @@ -2394,7 +2394,34 @@ module Chain = struct find_activation_block chain_store ~protocol_level:head_proto_level in match o with - | None -> return_unit + | None -> + (* If no activation block is registered for a given protocol + level, we search for the activation block. This activation + block is expected to be found above the savepoint. If not, + the savepoint will be considered as the activation + block. *) + let*! _, savepoint_level = savepoint chain_store in + let rec find_activation_block lower_bound block = + let*! pred = Block.read_predecessor_opt chain_store block in + match pred with + | None -> return block + | Some pred -> + let pred_proto_level = Block.proto_level pred in + if Compare.Int.(pred_proto_level <= Int.pred head_proto_level) + then return block + else if Compare.Int32.(Block.level pred <= lower_bound) then + return pred + else find_activation_block lower_bound pred + in + let* activation_block = find_activation_block savepoint_level head in + let protocol_level = Block.proto_level head in + (* The head must have an associated context stored. *) + let* context = Block.context chain_store head in + let*! activated_protocol = Context_ops.get_protocol context in + set_protocol_level + chain_store + ~protocol_level + (activation_block, activated_protocol) | Some {block; protocol; _} -> ( let*! _, savepoint_level = savepoint chain_store in if Compare.Int32.(savepoint_level > snd block) then diff --git a/src/lib_store/unix/store.mli b/src/lib_store/unix/store.mli index f572e556153d..449ac6fb2fd1 100644 --- a/src/lib_store/unix/store.mli +++ b/src/lib_store/unix/store.mli @@ -898,10 +898,10 @@ module Chain : sig unit tzresult Lwt.t (** [may_update_ancestor_protocol_level chain_store ~head] tries to - find the activation block of the [head]'s protocol, checks that - its an ancestor and tries to update it if that's not the case. If - the registered activation block is not reachable (already - pruned), this function does nothing. *) + find the activation block of the [head]'s protocol, checks that + its an ancestor and tries to update it if that's not the case. If + the registered activation block is not reachable (already + pruned), this function does nothing. *) val may_update_ancestor_protocol_level : chain_store -> head:Block.block -> unit tzresult Lwt.t -- GitLab From ab9b731fe3c821e6993a991aa319af247fff9e90 Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Fri, 23 Sep 2022 12:04:47 +0200 Subject: [PATCH 3/4] Store: fix protocols table while exporting snapshots --- src/lib_store/unix/snapshots.ml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lib_store/unix/snapshots.ml b/src/lib_store/unix/snapshots.ml index e7b0e1db2565..151d28f26d61 100644 --- a/src/lib_store/unix/snapshots.ml +++ b/src/lib_store/unix/snapshots.ml @@ -1979,13 +1979,12 @@ module Make_snapshot_exporter (Exporter : EXPORTER) : Snapshot_exporter = struct let export_protocols snapshot_exporter export_block all_protocol_levels protocol_store_dir progress_display_mode = let open Lwt_syntax in - let export_level = Store.Block.level export_block in - (* Filter protocols to only export the protocols with an activation - block below the block target. *) + let export_proto_level = Store.Block.proto_level export_block in + (* Export only the protocols with a protocol level below the + protocol's level of the targeted export block. *) let protocol_levels = Protocol_levels.filter - (fun _ {Protocol_levels.block = _, activation_level; _} -> - activation_level < export_level) + (fun proto_level _ -> proto_level <= export_proto_level) all_protocol_levels in let* () = -- GitLab From 8d41beb9c62e38a38f0890216c019ec40e9270e0 Mon Sep 17 00:00:00 2001 From: Victor Allombert Date: Wed, 28 Sep 2022 16:00:08 +0200 Subject: [PATCH 4/4] Changelog: fix protocol activation table while switching branch --- CHANGES.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index df50eeb1976e..857e3f67998f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -72,6 +72,10 @@ Node - The ``config`` and ``identity`` node commands no longer try to update the data directory version (``version.json``). +- Fixed a bug in the store that was generating an incorrect protocol + table during a branch switch containing a user activated protocol + upgrade. + Client ------ -- GitLab