From d7621f2faa56cb9caf05fa620e1fe612e016ec46 Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Thu, 30 Jan 2025 01:13:32 +0200 Subject: [PATCH 01/12] Agnostic_baker: Fix and improve tezt testing --- src/bin_agnostic_baker/parameters.ml | 2 +- tezt/tests/agnostic_baker_test.ml | 78 ++++++++++++++++------------ tezt/tests/main.ml | 4 +- 3 files changed, 49 insertions(+), 35 deletions(-) diff --git a/src/bin_agnostic_baker/parameters.ml b/src/bin_agnostic_baker/parameters.ml index c985a297d8c6..0d57461c7686 100644 --- a/src/bin_agnostic_baker/parameters.ml +++ b/src/bin_agnostic_baker/parameters.ml @@ -94,7 +94,7 @@ let protocol_info = function | "PsQuebecnLByd3JwTiGadoG4nGWi3HYiLXUjkibeFV8dCFeVMUg" ) as full_hash -> (String.sub full_hash 0 8, Active) | "ProtoALphaALphaALphaALphaALphaALphaALphaALphaDdp3zK" -> ("alpha", Active) - | _ -> (*We assume that unmatched protocols are beta ones*) ("beta", Active) + | _ -> (*We assume that unmatched protocols are next ones*) ("next", Active) let protocol_short_hash h = fst (protocol_info (Protocol_hash.to_b58check h)) diff --git a/tezt/tests/agnostic_baker_test.ml b/tezt/tests/agnostic_baker_test.ml index a9d2bcb9fb8f..96e322e4f9f5 100644 --- a/tezt/tests/agnostic_baker_test.ml +++ b/tezt/tests/agnostic_baker_test.ml @@ -10,6 +10,15 @@ Component: Agnostic baker (octez-experimental-agnostic-baker) Invocation: dune exec tezt/tests/main.exe -- --file agnostic_baker_test.ml Subject: Ensure that the agnostic baker behaves as expected + + Tests + ----- + - each migration test starts from an [Active] protocol (which has a baker binary) + and migrates to the next protocol + - each migration test can either use remote signing or not + - each migration test can either be a resilience or normal test; the former testing + that killing the baker during migration results in a graceful restart + - there is also a start and stop test *) let team = Tag.layer1 @@ -169,24 +178,26 @@ let perform_protocol_migration ?(resilience_test = false) ?node_name let* () = Agnostic_baker.terminate baker in unit -let raw_migrate ~resilience_test ?(use_remote_signer = false) = +let raw_migrate ~migrate_from ~migrate_to ~resilience_test ~use_remote_signer = let remote_signer_text, remote_signer = if use_remote_signer then (" using HTTP remote signer", [Constant.octez_signer]) else ("", []) in - let parameters = JSON.parse_file (Protocol.parameter_file Protocol.Alpha) in - Protocol.register_test + let parameters = JSON.parse_file (Protocol.parameter_file migrate_to) in + Test.register ~__FILE__ ~title: (Format.asprintf - "Protocol migration from protocol to alpha with agnostic baker%s %s" + "Protocol migration from protocol %s to protocol %s with agnostic \ + baker%s %s" + (Protocol.tag migrate_from) + (Protocol.tag migrate_to) remote_signer_text (if resilience_test then "and resilience test" else "")) - ~tags:["protocol"; "migration"; "agnostic"; "baker"; Tag.ci_disabled] - ~uses:(fun _protocol -> - [Constant.octez_experimental_agnostic_baker] @ remote_signer) - @@ fun protocol -> + ~tags:["protocol"; "migration"; "agnostic"; "baker"] + ~uses:([Constant.octez_experimental_agnostic_baker] @ remote_signer) + @@ fun () -> let blocks_per_cycle = JSON.(get "blocks_per_cycle" parameters |> as_int) in let consensus_rights_delay = JSON.(get "consensus_rights_delay" parameters |> as_int) @@ -198,41 +209,42 @@ let raw_migrate ~resilience_test ?(use_remote_signer = false) = ~use_remote_signer ~blocks_per_cycle ~migration_level - ~migrate_from:protocol - ~migrate_to:Protocol.Alpha + ~migrate_from + ~migrate_to ~baked_blocks_after_migration: (2 * consensus_rights_delay * blocks_per_cycle) () in unit -let start_stop = - Protocol.register_test +let migrate = raw_migrate ~resilience_test:false + +let migrate_with_resilience_test = raw_migrate ~resilience_test:true + +let register ~migrate_from ~migrate_to = + (* We want to migrate only from Active protocols *) + if Agnostic_baker.protocol_status migrate_from = Active then ( + migrate ~migrate_from ~migrate_to ~use_remote_signer:false ; + migrate ~migrate_from ~migrate_to ~use_remote_signer:true ; + migrate_with_resilience_test + ~migrate_from + ~migrate_to + ~use_remote_signer:false ; + migrate_with_resilience_test + ~migrate_from + ~migrate_to + ~use_remote_signer:true) + else () + +let register_protocol_independent () = + Test.register ~__FILE__ ~title:"Agnostic baker starts and stops" - ~tags:[team; "sandbox"; "agnostic"; "baker"; "init"; Tag.ci_disabled] - ~uses:(fun _protocol -> [Constant.octez_experimental_agnostic_baker]) - @@ fun _protocol -> + ~tags:[team; "sandbox"; "agnostic"; "baker"; "init"] + ~uses:[Constant.octez_experimental_agnostic_baker] + @@ fun () -> let* node, client = Client.init_with_node `Client () in let* baker = Agnostic_baker.init node client in let* () = Agnostic_baker.wait_for_ready baker in let* () = Agnostic_baker.terminate baker in unit - -let migrate = raw_migrate ~resilience_test:false - -let migrate_with_resilience_test = raw_migrate ~resilience_test:true - -let register ~protocols = - start_stop protocols ; - let filter_migratable_protocols protocol = - (* We want to migrate only from Active protocols *) - Agnostic_baker.protocol_status protocol = Active - in - let migratable_protocols = - List.filter filter_migratable_protocols protocols - in - migrate ~use_remote_signer:false migratable_protocols ; - migrate ~use_remote_signer:true migratable_protocols ; - migrate_with_resilience_test ~use_remote_signer:false migratable_protocols ; - migrate_with_resilience_test ~use_remote_signer:true migratable_protocols diff --git a/tezt/tests/main.ml b/tezt/tests/main.ml index f7d324e2ec3e..5635194bfd95 100644 --- a/tezt/tests/main.ml +++ b/tezt/tests/main.ml @@ -44,6 +44,7 @@ let alpha_can_stitch_from_its_predecessor = false (* Tests that are protocol-independent. They do not take a protocol as a parameter and thus need to be registered only once. *) let register_protocol_independent_tests () = + Agnostic_baker_test.register_protocol_independent () ; Binaries.register_protocol_independent () ; Bootstrap.register_protocol_independent () ; Cli_tezos.register_protocol_independent () ; @@ -68,6 +69,7 @@ let register_protocol_independent_tests () = (* Tests related to protocol migration. *) let register_protocol_migration_tests () = let migrate_from = Option.get @@ Protocol.previous_protocol migrate_to in + Agnostic_baker_test.register ~migrate_from ~migrate_to ; Mockup.register_constant_migration ~migrate_from ~migrate_to ; Protocol_migration.register ~migrate_from ~migrate_to ; Weeklynet.register () ; @@ -100,6 +102,7 @@ let register_old_protocol_migration_tests () = | _, Alpha -> () (* Already in register_protocol_migration_tests *) | None, _ -> () | Some migrate_from, migrate_to -> + Agnostic_baker_test.register ~migrate_from ~migrate_to ; Sc_rollup_migration.register ~migrate_from ~migrate_to) Protocol.all @@ -114,7 +117,6 @@ let register_old_protocol_migration_tests () = let register_protocol_tests_that_use_supports_correctly () = let protocols = Protocol.all in Adaptive_issuance.register ~protocols ; - Agnostic_baker_test.register ~protocols ; Bad_annot.register ~protocols ; Bad_indentation.register ~protocols ; Baker_test.register ~protocols ; -- GitLab From f41a0e0e804986d9ca8530da991db2d900a0e09f Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Wed, 29 Jan 2025 19:09:22 +0000 Subject: [PATCH 02/12] Agnostic_baker: Improve existing documentation --- src/bin_agnostic_baker/daemon.ml | 24 +++++++++++++++++++----- src/bin_agnostic_baker/daemon.mli | 20 ++++++++++++++++---- src/bin_agnostic_baker/parameters.mli | 4 ++-- src/bin_agnostic_baker/rpc_services.ml | 3 +++ src/bin_agnostic_baker/rpc_services.mli | 12 ++++++------ src/bin_agnostic_baker/run_args.mli | 4 ++-- 6 files changed, 48 insertions(+), 19 deletions(-) diff --git a/src/bin_agnostic_baker/daemon.ml b/src/bin_agnostic_baker/daemon.ml index abfb1484ca47..e9f013d5bef7 100644 --- a/src/bin_agnostic_baker/daemon.ml +++ b/src/bin_agnostic_baker/daemon.ml @@ -54,6 +54,9 @@ module MakeBaker (Name : Lwt_process_watchdog.NAME) : BAKER = struct let* () = Agnostic_baker_events.(emit stopping_baker) baker.protocol_hash in Watchdog.stop baker.process + (** [spawn_baker protocol_hash ~binaries_directory ~baker_args] spawns a baker + for the given [protocol_hash] using the [~binaries_directory] as path for + the baker binary and with [~baker_args] as command line arguments. *) let spawn_baker protocol_hash ~binaries_directory ~baker_args = let open Lwt_result_syntax in let args_as_string = @@ -103,6 +106,9 @@ type 'a state = { type 'a t = 'a state +(** [monitor_heads ~node_addr] creates a stream which returns the data + of the heads of the current network; this information is received + from the RPC calls at the endpoint given by [~node_addr]. *) let monitor_heads ~node_addr = let open Lwt_result_syntax in let uri = Format.sprintf "%s/monitor/heads/main" node_addr in @@ -127,6 +133,10 @@ let monitor_heads ~node_addr = ignore (loop () : unit Lwt.t) ; return stream +(** [hot_swap_baker ~state ~next_protocol_hash] performs a swap in the current + [~state] of the agnostic baker, exchanging the current baker with the one + corresponding to [~next_protocol_hash]. This is done by shutting down the + current baking binary and generating the new binary instead. *) let hot_swap_baker ~state ~next_protocol_hash = let open Lwt_result_syntax in let* (module CurrentBaker : BAKER), current_baker = @@ -168,6 +178,10 @@ let hot_swap_baker ~state ~next_protocol_hash = state.current_baker <- Some new_baker ; return_unit +(** [monitor_voting_periods ~state head_stream] creates a process which listens + to the [head_stream] stream (which returns the data of the heads of the network + chain) in order to know when to "hot swap" (fork) the current protocol baking + binary with the one associated with the next protocol. *) let monitor_voting_periods ~state head_stream = let open Lwt_result_syntax in let node_addr = state.node_endpoint in @@ -200,11 +214,11 @@ let monitor_voting_periods ~state head_stream = let* () = loop () in return_unit -(* Aims to start the baker associated to the current protocol. If - the protocol is considered as frozen (not active anymore), and - there is thus no actual baker binary anymore, the initial phase - consist in waiting until an active protocol is observed on - monitored heads. *) +(** [may_start_initial_baker state] aims to start the baker associated + to the current protocol. If the protocol is considered as [frozen] (not + [active] anymore), and there is thus no actual baker binary anymore, the + initial phase consists in waiting until an [active] protocol is observed on + monitored heads function. *) let may_start_initial_baker state = let open Lwt_result_syntax in let*! () = Agnostic_baker_events.(emit experimental_binary) () in diff --git a/src/bin_agnostic_baker/daemon.mli b/src/bin_agnostic_baker/daemon.mli index 3cdc92dd9ab7..513819b76700 100644 --- a/src/bin_agnostic_baker/daemon.mli +++ b/src/bin_agnostic_baker/daemon.mli @@ -5,12 +5,24 @@ (* *) (*****************************************************************************) -(** Daemon handling the bakers life cycle. *) +(** Daemon handling the baker's life cycle. + +It is used to [create] and [run] a protocol-agnostic process which uses the existing +baking binaries in an adaptive way, depending on the current protocol obtained +from the chain. + +It relies on a [state] which contains the [endpoint] to contact the running node, +together with the current baker which is being run. + +To do so, it also spawns a "monitoring" process which follows the heads of the +chain, as reported by the node from the [state], more precisely which monitors +the voting period. By doing that, it decides when to swap to a different baking +binary. *) type 'a t -(** [create binaries_directory node_endpoint baker_args] returns a non - initialized daemon.*) +(** [create ~binaries_directory ~node_endpoint ~baker_args] returns a non + initialized daemon. *) val create : binaries_directory:string option -> node_endpoint:string -> @@ -18,5 +30,5 @@ val create : 'a t (** [run t] Runs the daemon responsible for the spawn/stop of the - baker daemons. *) + baker daemons. *) val run : 'a t -> unit tzresult Lwt.t diff --git a/src/bin_agnostic_baker/parameters.mli b/src/bin_agnostic_baker/parameters.mli index 1b57b0d250ec..fe22e5bf98cd 100644 --- a/src/bin_agnostic_baker/parameters.mli +++ b/src/bin_agnostic_baker/parameters.mli @@ -12,9 +12,9 @@ val default_node_endpoint : string val log_config : base_dir:string option -> Tezos_base.Internal_event_config.t (** Status of a protocol, based on Manifest/Product_octez/Protocol. A - protocol is considered as Active while it is running on a network, + protocol is considered as [Active] while it is running on a network, and thus, have dedicated binaries. Otherwise, the protocol is - Frozen as not running anymore and no associated binaries. + [Frozen] as not running anymore and no associated binaries. Warning, it is needed to update status for each new protocol added. *) diff --git a/src/bin_agnostic_baker/rpc_services.ml b/src/bin_agnostic_baker/rpc_services.ml index 67866da85720..725f47615932 100644 --- a/src/bin_agnostic_baker/rpc_services.ml +++ b/src/bin_agnostic_baker/rpc_services.ml @@ -19,6 +19,9 @@ let request_uri ~node_addr ~uri = tzfail (Cannot_connect_to_node node_addr) | e -> raise e) +(** [call_and_wrap_rpc ~node_addr ~uri ~f] makes the RPC call given + by the [~uri] against [~node_addr], and in case of a well-formed + response, it applies [~f] to it. *) let call_and_wrap_rpc ~node_addr ~uri ~f = let open Lwt_result_syntax in let* resp, body = request_uri ~node_addr ~uri in diff --git a/src/bin_agnostic_baker/rpc_services.mli b/src/bin_agnostic_baker/rpc_services.mli index 5b962315d3c9..bdf44fb647fb 100644 --- a/src/bin_agnostic_baker/rpc_services.mli +++ b/src/bin_agnostic_baker/rpc_services.mli @@ -5,9 +5,9 @@ (* *) (*****************************************************************************) -(** request_uri ~node_addr ~uri] is a raw call that will return the - Cohttp response of a RPC call, given a [uri], against the - [node_addr]. *) +(** [request_uri ~node_addr ~uri] is a raw call that will return the + Cohttp response of an RPC call, given a [~uri], against the + [~node_addr]. *) val request_uri : node_addr:string -> uri:string -> @@ -18,12 +18,12 @@ val request_uri : block. *) val get_next_protocol_hash : node_addr:string -> Protocol_hash.t tzresult Lwt.t -(** [get_next_protocol_hash ~node_addr] returns the protocol hash of +(** [get_current_proposal ~node_addr] returns the protocol hash of the current voting period, if any. *) val get_current_proposal : node_addr:string -> Protocol_hash.t option tzresult Lwt.t -(** [get_next_protocol_hash ~node_addr] returns the current voting - period in addition to the number of remaining block until the end +(** [get_current_period ~node_addr] returns the current voting + period in addition to the number of remaining blocks until the end of the period. *) val get_current_period : node_addr:string -> (string * int) tzresult Lwt.t diff --git a/src/bin_agnostic_baker/run_args.mli b/src/bin_agnostic_baker/run_args.mli index ff420d04bb7d..cfe66e06de6d 100644 --- a/src/bin_agnostic_baker/run_args.mli +++ b/src/bin_agnostic_baker/run_args.mli @@ -13,7 +13,7 @@ type args = { baker_args : string list; } -(** [parse_args args] is a raw utility that aims to parse the give +(** [parse_args args] is a raw utility that aims to parse the given arguments from the command line and to return, respectively, the - endpoint, base_dir, binaries_directory and baker_args. *) + [endpoint], [base_dir], [binaries_directory] and [baker_args]. *) val parse_args : string array -> args -- GitLab From 145b8d335829050c7654a9881851855e143c175f Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Thu, 30 Jan 2025 13:10:24 +0200 Subject: [PATCH 03/12] Agnostic_baker: Add README --- src/bin_agnostic_baker/README.md | 59 ++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 src/bin_agnostic_baker/README.md diff --git a/src/bin_agnostic_baker/README.md b/src/bin_agnostic_baker/README.md new file mode 100644 index 000000000000..bec810985ea7 --- /dev/null +++ b/src/bin_agnostic_baker/README.md @@ -0,0 +1,59 @@ +# Agnostic Baker (experimental) + +## Overview + +Agnostic Baker is a protocol-independent binary that dynamically selects the +appropriate baking binary based on the active protocol. It monitors the state of +the blockchain and automatically switches to the correct binary when a new +protocol is encountered (for example, during migrations, or at startup). + +It is designed to simplify the baking process for users, such that they will no +longer need to run two baker binaries at migration time. + +## Experimental purpose + +For now, the binary is continuously being developed and tested. This is the reason +why users are warned that the binary is experimental and that it should not be +used for real-life scenarios, for instance, baking on `mainnet`. + +## Usage + +To run the agnostic baker, the command line syntax is quite similar to the one +for the protocol-dependent baking binaries: + +```bash +./octez-experimental-agnostic-baker [OCTEZ-EXPERIMENTAL-AGNOSTIC-BAKER-COMMANDS] \ +-- [OCTEZ-BAKER-COMMANDS] +``` + +The `[OCTEZ-EXPERIMENTAL-AGNOSTIC-BAKER-COMMANDS]` list consists of arguments specific +to the agnostic baker binary and they include: + +- `--binaries-directory` : where to find the baker binaries to use +- `--endpoint` : endpoint to communicate with the connected node +- `--base-dir` : directory dedicated to the agnostic baker (that can contain logs +and configuration files) +-- `--help` : displays help information + +The `[OCTEZ-BAKER-COMMANDS]` list consists of all the arguments that can be used +for the specific protocol baking binary. To be more clear, if a user wants to use +the agnostic baker to replace a baking command which would be + +```bash +./octez-baker- [OCTEZ-BAKER-COMMANDS] +``` + +they can do this by using the same `[OCTEZ-BAKER-COMMANDS]` and let the agnostic +baker run the binary for ``, information obtained from the node. + +Notice that the two types of arguments are separated by a clear `--`. + +## How it works + +1. **Initialization**: The daemon starts and connects to the specified Tezos node. +2. **Protocol Detection**: It fetches the currently active protocol. This is done via an RPC request on the `head` metadata against the Tezos node. +3. **Baker Selection**: Based on the active protocol, it selects the corresponding `octez-baker-` binary. +4. **Baker Execution**: The chosen binary is executed with the specified arguments +(`[OCTEZ-BAKER-COMMANDS]`). +5. **Chain Monitoring**: The daemon continuously monitors the chain for new blocks and protocol changes (based on the voting period). +6. **Protocol Updates**: If a new protocol is encountered, the daemon stops the current baker and starts a new one matching the new protocol. -- GitLab From 8191a26baa5b9edba61f1abe8f4fbed2765f9ea8 Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Thu, 30 Jan 2025 13:26:42 +0200 Subject: [PATCH 04/12] Agnostic_baker: Add changelog --- src/bin_agnostic_baker/CHANGES.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 src/bin_agnostic_baker/CHANGES.md diff --git a/src/bin_agnostic_baker/CHANGES.md b/src/bin_agnostic_baker/CHANGES.md new file mode 100644 index 000000000000..3f0a2247b72f --- /dev/null +++ b/src/bin_agnostic_baker/CHANGES.md @@ -0,0 +1,16 @@ +# Agnostic Baker Changelog + +- Improve existing documentation [!16491](https://gitlab.com/tezos/tezos/-/merge_requests/16491) +- Fix and improve tezt tests [!16492](https://gitlab.com/tezos/tezos/-/merge_requests/16492) +- Release agnostic baker as experimental [!16318](https://gitlab.com/tezos/tezos/-/merge_requests/16318) +- Agnostic baker uses generic watchdog [!15508](https://gitlab.com/tezos/tezos/-/merge_requests/15508) +- Change name into `octez-experimental-agnostic-baker` [!16434](https://gitlab.com/tezos/tezos/-/merge_requests/16434) +- Fix `--without-dal` issue in tezt tests [!16426](https://gitlab.com/tezos/tezos/-/merge_requests/16426) +- Add remote signer tezt tests [!16201](https://gitlab.com/tezos/tezos/-/merge_requests/16201) +- Refactoring and cleanup [!15325](https://gitlab.com/tezos/tezos/-/merge_requests/15324) +- Agnostic baker switches on new protocol [!15305](https://gitlab.com/tezos/tezos/-/merge_requests/15305) +- Logs improvement [!15072](https://gitlab.com/tezos/tezos/-/merge_requests/15072) +- Agnostic baker starts on known active protocol [!15071](https://gitlab.com/tezos/tezos/-/merge_requests/15071) +- Add monitoring of voting periods - [!15046](https://gitlab.com/tezos/tezos/-/merge_requests/15046) +- Prevent agnostic baker to be run without arguments - [!15202](https://gitlab.com/tezos/tezos/-/merge_requests/15202) +- Introduce dummy agnostic baker - [!15029](https://gitlab.com/tezos/tezos/-/merge_requests/15029) -- GitLab From 3222bd6efc76c3cd1025a29fed4f2b60a673c590 Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Wed, 15 Jan 2025 16:23:49 +0000 Subject: [PATCH 05/12] Agnostic_baker: Small refactoring introducing protocol_info type --- src/bin_agnostic_baker/parameters.ml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/bin_agnostic_baker/parameters.ml b/src/bin_agnostic_baker/parameters.ml index 0d57461c7686..802a5328b332 100644 --- a/src/bin_agnostic_baker/parameters.ml +++ b/src/bin_agnostic_baker/parameters.ml @@ -2,6 +2,7 @@ (* *) (* SPDX-License-Identifier: MIT *) (* Copyright (c) 2024 Nomadic Labs, *) +(* Copyright (c) 2025 TriliTech *) (* *) (*****************************************************************************) @@ -64,6 +65,8 @@ let status_encoding = (fun () -> Frozen); ]) +type protocol_info = {short_hash : string; status : status} + (* From Manifest/Product_octez/Protocol*) let protocol_info = function | ( "ProtoGenesisGenesisGenesisGenesisGenesisGenesk612im" @@ -89,13 +92,17 @@ let protocol_info = function | "PtNairobiyssHuh87hEhfVBGCVrK3WnS8Z2FT4ymB5tAa4r1nQf" | "ProxfordYmVfjWnRcgjWH36fW6PArwqykTFzotUxRs6gmTcZDuH" | "PtParisBxoLz5gzMmn3d9WBQNoPSZakgnkMC2VNuQ3KXfUtUQeZ" ) as full_hash -> - (String.sub full_hash 0 8, Frozen) + {short_hash = String.sub full_hash 0 8; status = Frozen} | ( "PsParisCZo7KAh1Z1smVd9ZMZ1HHn5gkzbM94V3PLCpknFWhUAi" | "PsQuebecnLByd3JwTiGadoG4nGWi3HYiLXUjkibeFV8dCFeVMUg" ) as full_hash -> - (String.sub full_hash 0 8, Active) - | "ProtoALphaALphaALphaALphaALphaALphaALphaALphaDdp3zK" -> ("alpha", Active) - | _ -> (*We assume that unmatched protocols are next ones*) ("next", Active) + {short_hash = String.sub full_hash 0 8; status = Active} + | "ProtoALphaALphaALphaALphaALphaALphaALphaALphaDdp3zK" -> + {short_hash = "alpha"; status = Active} + | _ -> + (*We assume that unmatched protocols are next ones*) + {short_hash = "next"; status = Active} -let protocol_short_hash h = fst (protocol_info (Protocol_hash.to_b58check h)) +let protocol_short_hash h = + (protocol_info (Protocol_hash.to_b58check h)).short_hash -let protocol_status h = snd (protocol_info (Protocol_hash.to_b58check h)) +let protocol_status h = (protocol_info (Protocol_hash.to_b58check h)).status -- GitLab From 4b473d4bede03ac641b61b47353c1043893ae5bf Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Wed, 15 Jan 2025 16:27:36 +0000 Subject: [PATCH 06/12] Agnostic_baker: Add rules for checking validity of baking command arguments --- src/bin_agnostic_baker/baker_args_rules.ml | 62 +++++++++++++++++++++ src/bin_agnostic_baker/baker_args_rules.mli | 34 +++++++++++ src/bin_agnostic_baker/parameters.ml | 47 ++++++++++++---- src/bin_agnostic_baker/parameters.mli | 3 + 4 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 src/bin_agnostic_baker/baker_args_rules.ml create mode 100644 src/bin_agnostic_baker/baker_args_rules.mli diff --git a/src/bin_agnostic_baker/baker_args_rules.ml b/src/bin_agnostic_baker/baker_args_rules.ml new file mode 100644 index 000000000000..b4cee7530f17 --- /dev/null +++ b/src/bin_agnostic_baker/baker_args_rules.ml @@ -0,0 +1,62 @@ +(*****************************************************************************) +(* *) +(* SPDX-License-Identifier: MIT *) +(* Copyright (c) 2025 TriliTech *) +(* *) +(*****************************************************************************) + +type t = + (* A mandatory rule which states that the baking argument [arg] must appear, + and if it doesn't, it will default to [default_value]. *) + | Mandatory of {arg : string; default_value : string} + (* A mandatory rule which states that one of the baking arguments from [args] + must appear, and if none does, the default is [default_arg]. *) + | One_of of {args : string list; default_arg : string} + +(* This list needs to be updated every time a new protocol is adopted + and every time a new baking argument rule is created in alpha/beta. *) +module Rules = struct + let mandatory_liquidity_rule = + Mandatory {arg = "--liquidity-baking-toggle-vote"; default_value = "pass"} + + let optional_dal_node_rule = + One_of + {args = ["--dal-node"; "--without-dal"]; default_arg = "--without-dal"} +end + +let warning_msg = function + | Mandatory {arg; _} -> Printf.sprintf "Missing mandatory argument: %s" arg + | One_of {args; _} -> + Printf.sprintf "One of [%s] must be provided" (String.concat "; " args) + +let solution_msg = function + | Mandatory {arg; default_value} -> + Printf.sprintf + "Adding mandatory argument `%s %s` to baking command" + arg + default_value + | One_of {args; default_arg} -> + Printf.sprintf + "Adding default argument `%s` to baking command. The other options are \ + `%s`." + default_arg + (String.concat "; " args) + +(** [is_broken_rule ~baker_args rule] checks if [rule] is violated within the + list of baking command arguments [~baker_args]. *) +let is_broken_rule ~baker_args rule = + match rule with + | Mandatory {arg; _} -> not (List.mem ~equal:String.equal arg baker_args) + | One_of {args; _} -> + not + (List.exists + (fun arg -> List.mem ~equal:String.equal arg baker_args) + args) + +let get_broken_rules ~baker_args rules = + List.filter (is_broken_rule ~baker_args) rules + +let solve_broken_rule ~baker_args rule = + match rule with + | Mandatory {arg; default_value} -> baker_args @ [arg; default_value] + | One_of {default_arg; _} -> baker_args @ [default_arg] diff --git a/src/bin_agnostic_baker/baker_args_rules.mli b/src/bin_agnostic_baker/baker_args_rules.mli new file mode 100644 index 000000000000..31ec7ea2e4ea --- /dev/null +++ b/src/bin_agnostic_baker/baker_args_rules.mli @@ -0,0 +1,34 @@ +(*****************************************************************************) +(* *) +(* SPDX-License-Identifier: MIT *) +(* Copyright (c) 2025 TriliTech *) +(* *) +(*****************************************************************************) + +type t + +module Rules : sig + (** The [--liquidity-baking-toggle-vote] option must appear in the baking + command arguments, if it is not provided, its defaulted to ["pass"] *) + val mandatory_liquidity_rule : t + + (** Either one of the [--dal-node] or [--without-dal] options must appear + in the baking command arguments, otherwise the default is [--without-dal]. *) + val optional_dal_node_rule : t +end + +(** [warning_msg rule] returns a warning message regarding a broken [rule] *) +val warning_msg : t -> string + +(** [solution_msg rule] returns the message after solving the issue generated + by a broken [rule] *) +val solution_msg : t -> string + +(** [get_broken_rules ~baker_ags rules] returns all broken rules from + [rules] in the list of baker command arguments from [~baker_args]. *) +val get_broken_rules : baker_args:string list -> t list -> t list + +(** [solve_broken_rule ~baker_args broken_rule] takes a list of baker + command arguments [~baker_args] and returns a new list which solves + the issue encountered in the [broken_rule]. *) +val solve_broken_rule : baker_args:string list -> t -> string list diff --git a/src/bin_agnostic_baker/parameters.ml b/src/bin_agnostic_baker/parameters.ml index 802a5328b332..795953dbe378 100644 --- a/src/bin_agnostic_baker/parameters.ml +++ b/src/bin_agnostic_baker/parameters.ml @@ -65,9 +65,13 @@ let status_encoding = (fun () -> Frozen); ]) -type protocol_info = {short_hash : string; status : status} +type protocol_info = { + short_hash : string; + status : status; + baker_args_rules : Baker_args_rules.t list; +} -(* From Manifest/Product_octez/Protocol*) +(* From Manifest/Product_octez/Protocol *) let protocol_info = function | ( "ProtoGenesisGenesisGenesisGenesisGenesisGenesk612im" | "Ps9mPmXaRzmzk35gbAYNCAw6UXdE2qoABTHbN2oEEc1qM7CwT9P" @@ -91,18 +95,41 @@ let protocol_info = function | "PtMumbai2TmsJHNGRkD8v8YDbtao7BLUC3wjASn1inAKLFCjaH1" | "PtNairobiyssHuh87hEhfVBGCVrK3WnS8Z2FT4ymB5tAa4r1nQf" | "ProxfordYmVfjWnRcgjWH36fW6PArwqykTFzotUxRs6gmTcZDuH" - | "PtParisBxoLz5gzMmn3d9WBQNoPSZakgnkMC2VNuQ3KXfUtUQeZ" ) as full_hash -> - {short_hash = String.sub full_hash 0 8; status = Frozen} - | ( "PsParisCZo7KAh1Z1smVd9ZMZ1HHn5gkzbM94V3PLCpknFWhUAi" - | "PsQuebecnLByd3JwTiGadoG4nGWi3HYiLXUjkibeFV8dCFeVMUg" ) as full_hash -> - {short_hash = String.sub full_hash 0 8; status = Active} + | "PtParisBxoLz5gzMmn3d9WBQNoPSZakgnkMC2VNuQ3KXfUtUQeZ" + | "PsParisCZo7KAh1Z1smVd9ZMZ1HHn5gkzbM94V3PLCpknFWhUAi" ) as full_hash -> + { + short_hash = String.sub full_hash 0 8; + status = Frozen; + baker_args_rules = []; + } + | "PsQuebecnLByd3JwTiGadoG4nGWi3HYiLXUjkibeFV8dCFeVMUg" as full_hash -> + { + short_hash = String.sub full_hash 0 8; + status = Active; + baker_args_rules = [Baker_args_rules.Rules.mandatory_liquidity_rule]; + } | "ProtoALphaALphaALphaALphaALphaALphaALphaALphaDdp3zK" -> - {short_hash = "alpha"; status = Active} + { + short_hash = "alpha"; + status = Active; + baker_args_rules = + Baker_args_rules.Rules. + [mandatory_liquidity_rule; optional_dal_node_rule]; + } | _ -> - (*We assume that unmatched protocols are next ones*) - {short_hash = "next"; status = Active} + (* We assume that unmatched protocols are next ones *) + { + short_hash = "next"; + status = Active; + baker_args_rules = + Baker_args_rules.Rules. + [mandatory_liquidity_rule; optional_dal_node_rule]; + } let protocol_short_hash h = (protocol_info (Protocol_hash.to_b58check h)).short_hash let protocol_status h = (protocol_info (Protocol_hash.to_b58check h)).status + +let protocol_baker_args_rules h = + (protocol_info (Protocol_hash.to_b58check h)).baker_args_rules diff --git a/src/bin_agnostic_baker/parameters.mli b/src/bin_agnostic_baker/parameters.mli index fe22e5bf98cd..20d1342e17e8 100644 --- a/src/bin_agnostic_baker/parameters.mli +++ b/src/bin_agnostic_baker/parameters.mli @@ -2,6 +2,7 @@ (* *) (* SPDX-License-Identifier: MIT *) (* Copyright (c) 2024 Nomadic Labs, *) +(* Copyright (c) 2025 TriliTech *) (* *) (*****************************************************************************) @@ -27,3 +28,5 @@ val status_encoding : status t val protocol_short_hash : Protocol_hash.t -> string val protocol_status : Protocol_hash.t -> status + +val protocol_baker_args_rules : Protocol_hash.t -> Baker_args_rules.t trace -- GitLab From deae1badc5cfa15c9265c925e0d9a87cea22a413 Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Wed, 15 Jan 2025 16:29:17 +0000 Subject: [PATCH 07/12] Agnostic_baker: Plug rule checking in the baker spawn command --- .../agnostic_baker_events.ml | 21 +++++++++++++++++ src/bin_agnostic_baker/daemon.ml | 23 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/bin_agnostic_baker/agnostic_baker_events.ml b/src/bin_agnostic_baker/agnostic_baker_events.ml index cdb6585bf353..fa1aa1ce5760 100644 --- a/src/bin_agnostic_baker/agnostic_baker_events.ml +++ b/src/bin_agnostic_baker/agnostic_baker_events.ml @@ -2,6 +2,7 @@ (* *) (* SPDX-License-Identifier: MIT *) (* Copyright (c) 2024 Nomadic Labs, *) +(* Copyright (c) 2025 TriliTech *) (* *) (*****************************************************************************) @@ -89,6 +90,16 @@ let period_status = ("period", string) ("remaining", int31) +let solved_broken_rule = + declare_1 + ~section + ~alternative_color + ~level:Notice + ~name:"solved_broken_rule" + ~msg:"{solution_msg}" + ("solution_msg", string) + ~pp1:Format.pp_print_string + (* Warning *) let experimental_binary = declare_0 @@ -101,3 +112,13 @@ let experimental_binary = it is intended for testing purposes only. Please do not use it on \ `mainnet`." () + +let detected_broken_rule = + declare_1 + ~section + ~alternative_color + ~level:Warning + ~name:"detected_broken_rule" + ~msg:"{error_msg}" + ("error_msg", string) + ~pp1:Format.pp_print_string diff --git a/src/bin_agnostic_baker/daemon.ml b/src/bin_agnostic_baker/daemon.ml index e9f013d5bef7..5d063968129c 100644 --- a/src/bin_agnostic_baker/daemon.ml +++ b/src/bin_agnostic_baker/daemon.ml @@ -2,6 +2,7 @@ (* *) (* SPDX-License-Identifier: MIT *) (* Copyright (c) 2024 Nomadic Labs, *) +(* Copyright (c) 2025 TriliTech *) (* *) (*****************************************************************************) @@ -54,11 +55,33 @@ module MakeBaker (Name : Lwt_process_watchdog.NAME) : BAKER = struct let* () = Agnostic_baker_events.(emit stopping_baker) baker.protocol_hash in Watchdog.stop baker.process + (** [check_rules ~baker_args ~rules] obtains the broken rules from [~rules] + in [~baker_args], warns the user of the broken rules, tells them how they + are going to get fixed, and then fixes them. *) + let check_rules ~baker_args ~rules = + let open Lwt_syntax in + let broken_rules = Baker_args_rules.get_broken_rules ~baker_args rules in + List.fold_left_s + (fun baker_args broken_rule -> + let* () = + Agnostic_baker_events.(emit detected_broken_rule) + (Baker_args_rules.warning_msg broken_rule) + in + let+ () = + Agnostic_baker_events.(emit solved_broken_rule) + (Baker_args_rules.solution_msg broken_rule) + in + Baker_args_rules.solve_broken_rule ~baker_args broken_rule) + baker_args + broken_rules + (** [spawn_baker protocol_hash ~binaries_directory ~baker_args] spawns a baker for the given [protocol_hash] using the [~binaries_directory] as path for the baker binary and with [~baker_args] as command line arguments. *) let spawn_baker protocol_hash ~binaries_directory ~baker_args = let open Lwt_result_syntax in + let rules = Parameters.protocol_baker_args_rules protocol_hash in + let*! baker_args = check_rules ~baker_args ~rules in let args_as_string = Format.asprintf "%a" -- GitLab From 4265c1af462243a5f2d4de3404f1162cbd1c9d50 Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Tue, 21 Jan 2025 12:02:14 +0000 Subject: [PATCH 08/12] Agnostic_baker: Add --auto-patch-cli argument option --- .../agnostic_baker_events.ml | 14 +++++++ src/bin_agnostic_baker/daemon.ml | 38 +++++++++++++++---- src/bin_agnostic_baker/daemon.mli | 5 ++- src/bin_agnostic_baker/main_agnostic_baker.ml | 15 ++++++-- src/bin_agnostic_baker/run_args.ml | 15 ++++++-- src/bin_agnostic_baker/run_args.mli | 1 + 6 files changed, 72 insertions(+), 16 deletions(-) diff --git a/src/bin_agnostic_baker/agnostic_baker_events.ml b/src/bin_agnostic_baker/agnostic_baker_events.ml index fa1aa1ce5760..408afc2b2873 100644 --- a/src/bin_agnostic_baker/agnostic_baker_events.ml +++ b/src/bin_agnostic_baker/agnostic_baker_events.ml @@ -90,6 +90,20 @@ let period_status = ("period", string) ("remaining", int31) +let auto_patch_cli_activated = + declare_1 + ~section + ~alternative_color + ~level:Notice + ~name:"auto_patch_cli_activated" + ~msg: + "The automatic CLI patching for the agnostic baker has been activated. \ + This implies that the command line arguments will be checked against \ + the rules of the binary corresponding to the following protocol: \ + {proto_hash}" + ("proto_hash", Protocol_hash.encoding) + ~pp1:Protocol_hash.pp_short + let solved_broken_rule = declare_1 ~section diff --git a/src/bin_agnostic_baker/daemon.ml b/src/bin_agnostic_baker/daemon.ml index 5d063968129c..c878af659f15 100644 --- a/src/bin_agnostic_baker/daemon.ml +++ b/src/bin_agnostic_baker/daemon.ml @@ -39,6 +39,7 @@ module type BAKER = sig Protocol_hash.t -> binaries_directory:string option -> baker_args:string trace -> + auto_patch_cli:bool -> (baker, tztrace) result Lwt.t end @@ -75,13 +76,25 @@ module MakeBaker (Name : Lwt_process_watchdog.NAME) : BAKER = struct baker_args broken_rules - (** [spawn_baker protocol_hash ~binaries_directory ~baker_args] spawns a baker - for the given [protocol_hash] using the [~binaries_directory] as path for - the baker binary and with [~baker_args] as command line arguments. *) - let spawn_baker protocol_hash ~binaries_directory ~baker_args = + (** [spawn_baker protocol_hash ~binaries_directory ~baker_args ~auto_patch_cli] + spawns a baker for the given [protocol_hash] using the [~binaries_directory] + as path for the baker binary and with [~baker_args] as command line arguments. + + If [~auto_patch_cli] argument is [true], then in case the baker command does + not "work" (i.e. does not obey the rules specified in `baker_args_rules`) regarding + its CLI arguments, then the command will pe patched automatically. *) + let spawn_baker protocol_hash ~binaries_directory ~baker_args ~auto_patch_cli + = let open Lwt_result_syntax in - let rules = Parameters.protocol_baker_args_rules protocol_hash in - let*! baker_args = check_rules ~baker_args ~rules in + let*! baker_args = + if auto_patch_cli then + let*! () = + Agnostic_baker_events.(emit auto_patch_cli_activated) protocol_hash + in + let rules = Parameters.protocol_baker_args_rules protocol_hash in + check_rules ~baker_args ~rules + else Lwt.return baker_args + in let args_as_string = Format.asprintf "%a" @@ -124,6 +137,7 @@ type 'a state = { binaries_directory : string option; node_endpoint : string; baker_args : string list; + auto_patch_cli : bool; mutable current_baker : (baker_instance * baker) option; } @@ -195,6 +209,7 @@ let hot_swap_baker ~state ~next_protocol_hash = next_protocol_hash ~binaries_directory:state.binaries_directory ~baker_args:state.baker_args + ~auto_patch_cli:state.auto_patch_cli in return (Baker (module NewBaker), new_baker) in @@ -277,6 +292,7 @@ let may_start_initial_baker state = protocol_hash ~binaries_directory:state.binaries_directory ~baker_args:state.baker_args + ~auto_patch_cli:state.auto_patch_cli in return (Baker (module NewBaker), new_baker) in @@ -307,8 +323,14 @@ let may_start_initial_baker state = in may_start ~head_stream:None () -let create ~binaries_directory ~node_endpoint ~baker_args = - {binaries_directory; node_endpoint; baker_args; current_baker = None} +let create ~binaries_directory ~node_endpoint ~baker_args ~auto_patch_cli = + { + binaries_directory; + node_endpoint; + baker_args; + auto_patch_cli; + current_baker = None; + } let run state = let open Lwt_result_syntax in diff --git a/src/bin_agnostic_baker/daemon.mli b/src/bin_agnostic_baker/daemon.mli index 513819b76700..d5ce5b5da4f2 100644 --- a/src/bin_agnostic_baker/daemon.mli +++ b/src/bin_agnostic_baker/daemon.mli @@ -21,12 +21,13 @@ binary. *) type 'a t -(** [create ~binaries_directory ~node_endpoint ~baker_args] returns a non - initialized daemon. *) +(** [create ~binaries_directory ~node_endpoint ~baker_args ~auto_patch_cli] + returns a non initialized daemon.*) val create : binaries_directory:string option -> node_endpoint:string -> baker_args:string trace -> + auto_patch_cli:bool -> 'a t (** [run t] Runs the daemon responsible for the spawn/stop of the diff --git a/src/bin_agnostic_baker/main_agnostic_baker.ml b/src/bin_agnostic_baker/main_agnostic_baker.ml index ac627582e02c..2c7835539351 100644 --- a/src/bin_agnostic_baker/main_agnostic_baker.ml +++ b/src/bin_agnostic_baker/main_agnostic_baker.ml @@ -7,7 +7,14 @@ let run () = let open Lwt_result_syntax in - let Run_args.{node_endpoint; base_dir; binaries_directory; baker_args} = + let Run_args. + { + node_endpoint; + base_dir; + binaries_directory; + baker_args; + auto_patch_cli; + } = Run_args.parse_args Sys.argv in let*! () = @@ -15,8 +22,10 @@ let run () = ~config:(Parameters.log_config ~base_dir) () in - let daemon = Daemon.create ~binaries_directory ~node_endpoint ~baker_args in - let* (_ : unit) = Daemon.run daemon in + let daemon = + Daemon.create ~binaries_directory ~node_endpoint ~baker_args ~auto_patch_cli + in + let* () = Daemon.run daemon in let*! () = Lwt_utils.never_ending () in return_unit diff --git a/src/bin_agnostic_baker/run_args.ml b/src/bin_agnostic_baker/run_args.ml index 7b7654d93d29..f66b9373f335 100644 --- a/src/bin_agnostic_baker/run_args.ml +++ b/src/bin_agnostic_baker/run_args.ml @@ -15,6 +15,8 @@ let base_dir_arg = "--base-dir" let base_dir_short_arg = "--b" +let auto_patch_cli_arg = "--auto-patch-cli" + let help_arg = "--help" let print_help () = @@ -26,9 +28,11 @@ let print_help () = Format.printf "OCTEZ-EXPERIMENTAL-AGNOSTIC-BAKER-COMMANDS:\n\ \ %s: display help\n\ - \ %s: path to the octez-baker binaries@.@." + \ %s: path to the octez-baker binaries\n\ + \ %s: try to automatically patch the CLI errors@.@." help_arg - binaries_directory_arg ; + binaries_directory_arg + auto_patch_cli_arg ; Format.printf "OCTEZ-BAKER-COMMANDS:\n Run ./octez-baker- --help@." @@ -44,6 +48,9 @@ let version_cmd args = exit 0) else () +let auto_patch_cli_cmd args = + List.mem ~equal:String.equal auto_patch_cli_arg args + let split_args ?(on = "--") = let rec loop acc = function | [] -> (List.rev acc, []) @@ -79,6 +86,7 @@ type args = { base_dir : string option; binaries_directory : string option; baker_args : string list; + auto_patch_cli : bool; } let parse_args all_args = @@ -97,5 +105,6 @@ let parse_args all_args = (get_endpoint baker_args) in let binaries_directory = get_binaries_directory agnostic_baker_args in + let auto_patch_cli = auto_patch_cli_cmd agnostic_baker_args in let base_dir = get_base_dir baker_args in - {node_endpoint; base_dir; binaries_directory; baker_args} + {node_endpoint; base_dir; binaries_directory; baker_args; auto_patch_cli} diff --git a/src/bin_agnostic_baker/run_args.mli b/src/bin_agnostic_baker/run_args.mli index cfe66e06de6d..d137a880bf02 100644 --- a/src/bin_agnostic_baker/run_args.mli +++ b/src/bin_agnostic_baker/run_args.mli @@ -11,6 +11,7 @@ type args = { base_dir : string option; binaries_directory : string option; baker_args : string list; + auto_patch_cli : bool; } (** [parse_args args] is a raw utility that aims to parse the given -- GitLab From c26bb9aa02c135a9c7b387879e6a7dd16cc63df8 Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Wed, 29 Jan 2025 12:49:34 +0000 Subject: [PATCH 09/12] Tezt: Fix agnostic_baker test using the new --auto-patch-cli flag --- tezt/lib_tezos/agnostic_baker.ml | 17 +++++++++++++---- tezt/lib_tezos/agnostic_baker.mli | 9 ++++++++- tezt/tests/agnostic_baker_test.ml | 21 +++++++++++++++------ 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/tezt/lib_tezos/agnostic_baker.ml b/tezt/lib_tezos/agnostic_baker.ml index 6543fe5102f4..8c975e1d8ee2 100644 --- a/tezt/lib_tezos/agnostic_baker.ml +++ b/tezt/lib_tezos/agnostic_baker.ml @@ -23,6 +23,7 @@ module Parameters = struct base_dir : string; node_data_dir : string; node_rpc_endpoint : Endpoint.t; + auto_patch_cli : bool; mutable pending_ready : unit option Lwt.u list; remote_mode : bool; liquidity_baking_toggle_vote : liquidity_baking_vote option; @@ -54,7 +55,7 @@ let create_from_uris ?runner ?(path = Uses.path Constant.octez_experimental_agnostic_baker) ?name ?color ?event_pipe ?(delegates = []) ?(remote_mode = false) ?(liquidity_baking_toggle_vote = Some Pass) ?use_dal_node ~base_dir - ~node_data_dir ~node_rpc_endpoint () = + ~node_data_dir ~node_rpc_endpoint ~auto_patch_cli () = let agnostic_baker = create ~path @@ -68,6 +69,7 @@ let create_from_uris ?runner base_dir; node_data_dir; node_rpc_endpoint; + auto_patch_cli; pending_ready = []; remote_mode; liquidity_baking_toggle_vote; @@ -81,7 +83,7 @@ let handle_event node ({name; _} : event) = let create ?runner ?path ?name ?color ?event_pipe ?(delegates = []) ?(remote_mode = false) ?(liquidity_baking_toggle_vote = Some Pass) - ?use_dal_node node client = + ?use_dal_node ?(auto_patch_cli = false) node client = let agnostic_baker = create_from_uris ?runner @@ -96,6 +98,7 @@ let create ?runner ?path ?name ?color ?event_pipe ?(delegates = []) ~base_dir:(Client.base_dir client) ~node_data_dir:(Node.data_dir node) ~node_rpc_endpoint:(Node.as_rpc_endpoint node) + ~auto_patch_cli () in on_event agnostic_baker (handle_event agnostic_baker) ; @@ -113,6 +116,10 @@ let run ?event_level ?event_sections_levels (agnostic_baker : t) = let node_addr = Endpoint.as_string agnostic_baker.persistent_state.node_rpc_endpoint in + let auto_patch_cli_arg = + if agnostic_baker.persistent_state.auto_patch_cli then ["--auto-patch-cli"] + else [] + in let run_args = if agnostic_baker.persistent_state.remote_mode then ["remotely"] else ["with"; "local"; "node"; node_data_dir] @@ -129,7 +136,8 @@ let run ?event_level ?event_sections_levels (agnostic_baker : t) = | Some endpoint -> ["--dal-node"; endpoint] in let arguments = - ["--"; "--endpoint"; node_addr; "--base-dir"; base_dir; "run"] + auto_patch_cli_arg + @ ["--"; "--endpoint"; node_addr; "--base-dir"; base_dir; "run"] @ run_args @ delegates @ liquidity_baking_toggle_vote @ use_dal_node in @@ -168,7 +176,7 @@ let wait_for_ready agnostic_baker = let init ?runner ?(path = Uses.path Constant.octez_experimental_agnostic_baker) ?name ?color ?event_level ?event_pipe ?event_sections_levels ?(delegates = []) ?remote_mode ?liquidity_baking_toggle_vote ?use_dal_node - node client = + ?auto_patch_cli node client = let* () = Node.wait_for_ready node in let agnostic_baker = create @@ -180,6 +188,7 @@ let init ?runner ?(path = Uses.path Constant.octez_experimental_agnostic_baker) ?remote_mode ?liquidity_baking_toggle_vote ?use_dal_node + ?auto_patch_cli ~delegates node client diff --git a/tezt/lib_tezos/agnostic_baker.mli b/tezt/lib_tezos/agnostic_baker.mli index 183c2b134cc6..01827f43299e 100644 --- a/tezt/lib_tezos/agnostic_baker.mli +++ b/tezt/lib_tezos/agnostic_baker.mli @@ -100,6 +100,10 @@ val protocol_status : Protocol.t -> protocol_status former option is passed, otherwise if the flag is [Some ], then the latter option is passed. The default value is [None]. + [auto_patch_cli] is a flag used for an automatic mechanism which tries to correct the + command line call of the baker, to obey the rules specific to the baker binary (this + can vary depending on the corresponding protocol). + If [remote_mode] is specified, the agnostic baker will run in RPC-only mode. *) val create : @@ -112,6 +116,7 @@ val create : ?remote_mode:bool -> ?liquidity_baking_toggle_vote:liquidity_baking_vote option -> ?use_dal_node:string -> + ?auto_patch_cli:bool -> Node.t -> Client.t -> t @@ -143,10 +148,11 @@ val create_from_uris : base_dir:string -> node_data_dir:string -> node_rpc_endpoint:Endpoint.t -> + auto_patch_cli:bool -> unit -> t -(** Initialize an agnositc baker. +(** Initialize an agnostic baker. This creates agnostic baker, waits for it to be ready, and then returns it. @@ -194,6 +200,7 @@ val init : ?remote_mode:bool -> ?liquidity_baking_toggle_vote:liquidity_baking_vote option -> ?use_dal_node:string -> + ?auto_patch_cli:bool -> Node.t -> Client.t -> t Lwt.t diff --git a/tezt/tests/agnostic_baker_test.ml b/tezt/tests/agnostic_baker_test.ml index 96e322e4f9f5..942177e015ae 100644 --- a/tezt/tests/agnostic_baker_test.ml +++ b/tezt/tests/agnostic_baker_test.ml @@ -29,6 +29,12 @@ let wait_for_active_protocol_waiting agnostic_baker = "waiting_for_active_protocol.v0" (fun _json -> Some ()) +let wait_for_auto_patch agnostic_baker = + Agnostic_baker.wait_for + agnostic_baker + "auto_patch_cli_activated.v0" + (fun _json -> Some ()) + let wait_for_baker_process agnostic_baker protocol_short_hash = Agnostic_baker.wait_for agnostic_baker @@ -42,8 +48,8 @@ let wait_for_baker_process agnostic_baker protocol_short_hash = test, making sure it is restarted as expected. *) let perform_protocol_migration ?(resilience_test = false) ?node_name ?client_name ?parameter_file ?(use_remote_signer = false) ~blocks_per_cycle - ~migration_level ~migrate_from ~migrate_to ~baked_blocks_after_migration () - = + ~migration_level ~migrate_from ~migrate_to ~baked_blocks_after_migration + ~auto_patch_cli () = assert (migration_level >= blocks_per_cycle) ; Log.info "Node starting" ; let* client, node = @@ -71,7 +77,7 @@ let perform_protocol_migration ?(resilience_test = false) ?node_name else unit in Log.info "Node %s initialized" (Node.name node) ; - let baker = Agnostic_baker.create node client in + let baker = Agnostic_baker.create node client ~auto_patch_cli in let wait_for_active_protocol_waiting = wait_for_active_protocol_waiting baker in @@ -178,7 +184,8 @@ let perform_protocol_migration ?(resilience_test = false) ?node_name let* () = Agnostic_baker.terminate baker in unit -let raw_migrate ~migrate_from ~migrate_to ~resilience_test ~use_remote_signer = +let raw_migrate ~migrate_from ~migrate_to ~resilience_test ~use_remote_signer + ~auto_patch_cli = let remote_signer_text, remote_signer = if use_remote_signer then (" using HTTP remote signer", [Constant.octez_signer]) @@ -213,13 +220,15 @@ let raw_migrate ~migrate_from ~migrate_to ~resilience_test ~use_remote_signer = ~migrate_to ~baked_blocks_after_migration: (2 * consensus_rights_delay * blocks_per_cycle) + ~auto_patch_cli () in unit -let migrate = raw_migrate ~resilience_test:false +let migrate = raw_migrate ~resilience_test:false ~auto_patch_cli:true -let migrate_with_resilience_test = raw_migrate ~resilience_test:true +let migrate_with_resilience_test = + raw_migrate ~resilience_test:true ~auto_patch_cli:true let register ~migrate_from ~migrate_to = (* We want to migrate only from Active protocols *) -- GitLab From 083ad12ab96ec713619ff3a7a78d0d8068009ef7 Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Thu, 30 Jan 2025 13:52:06 +0200 Subject: [PATCH 10/12] Tezt: Remove default --without-dal behaviour from the manual test --- tezt/lib_tezos/agnostic_baker.ml | 21 ++++++--------------- tezt/lib_tezos/agnostic_baker.mli | 8 -------- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/tezt/lib_tezos/agnostic_baker.ml b/tezt/lib_tezos/agnostic_baker.ml index 8c975e1d8ee2..f8241989c9fa 100644 --- a/tezt/lib_tezos/agnostic_baker.ml +++ b/tezt/lib_tezos/agnostic_baker.ml @@ -27,7 +27,6 @@ module Parameters = struct mutable pending_ready : unit option Lwt.u list; remote_mode : bool; liquidity_baking_toggle_vote : liquidity_baking_vote option; - use_dal_node : string option; } type session_state = {mutable ready : bool} @@ -54,8 +53,8 @@ let set_ready agnostic_baker = let create_from_uris ?runner ?(path = Uses.path Constant.octez_experimental_agnostic_baker) ?name ?color ?event_pipe ?(delegates = []) ?(remote_mode = false) - ?(liquidity_baking_toggle_vote = Some Pass) ?use_dal_node ~base_dir - ~node_data_dir ~node_rpc_endpoint ~auto_patch_cli () = + ?(liquidity_baking_toggle_vote = Some Pass) ~base_dir ~node_data_dir + ~node_rpc_endpoint ~auto_patch_cli () = let agnostic_baker = create ~path @@ -73,7 +72,6 @@ let create_from_uris ?runner pending_ready = []; remote_mode; liquidity_baking_toggle_vote; - use_dal_node; } in agnostic_baker @@ -83,7 +81,7 @@ let handle_event node ({name; _} : event) = let create ?runner ?path ?name ?color ?event_pipe ?(delegates = []) ?(remote_mode = false) ?(liquidity_baking_toggle_vote = Some Pass) - ?use_dal_node ?(auto_patch_cli = false) node client = + ?(auto_patch_cli = false) node client = let agnostic_baker = create_from_uris ?runner @@ -91,7 +89,6 @@ let create ?runner ?path ?name ?color ?event_pipe ?(delegates = []) ?name ?color ?event_pipe - ?use_dal_node ~delegates ~remote_mode ~liquidity_baking_toggle_vote @@ -130,15 +127,10 @@ let run ?event_level ?event_sections_levels (agnostic_baker : t) = liquidity_baking_vote_to_string agnostic_baker.persistent_state.liquidity_baking_toggle_vote in - let use_dal_node = - match agnostic_baker.persistent_state.use_dal_node with - | None -> ["--without-dal"] - | Some endpoint -> ["--dal-node"; endpoint] - in let arguments = auto_patch_cli_arg @ ["--"; "--endpoint"; node_addr; "--base-dir"; base_dir; "run"] - @ run_args @ delegates @ liquidity_baking_toggle_vote @ use_dal_node + @ run_args @ delegates @ liquidity_baking_toggle_vote in let on_terminate _ = @@ -175,8 +167,8 @@ let wait_for_ready agnostic_baker = let init ?runner ?(path = Uses.path Constant.octez_experimental_agnostic_baker) ?name ?color ?event_level ?event_pipe ?event_sections_levels - ?(delegates = []) ?remote_mode ?liquidity_baking_toggle_vote ?use_dal_node - ?auto_patch_cli node client = + ?(delegates = []) ?remote_mode ?liquidity_baking_toggle_vote ?auto_patch_cli + node client = let* () = Node.wait_for_ready node in let agnostic_baker = create @@ -187,7 +179,6 @@ let init ?runner ?(path = Uses.path Constant.octez_experimental_agnostic_baker) ?event_pipe ?remote_mode ?liquidity_baking_toggle_vote - ?use_dal_node ?auto_patch_cli ~delegates node diff --git a/tezt/lib_tezos/agnostic_baker.mli b/tezt/lib_tezos/agnostic_baker.mli index 01827f43299e..a9bb00a4810f 100644 --- a/tezt/lib_tezos/agnostic_baker.mli +++ b/tezt/lib_tezos/agnostic_baker.mli @@ -95,11 +95,6 @@ val protocol_status : Protocol.t -> protocol_status then [--liquidity-baking-toggle-vote x] is passed. The default value is [Some Pass]. - [use_dal_node] is passed to the agnostic baker daemon through the - [--without-dal] or [--dal-node ] flags. If the flag is [None], the - former option is passed, otherwise if the flag is [Some ], then the - latter option is passed. The default value is [None]. - [auto_patch_cli] is a flag used for an automatic mechanism which tries to correct the command line call of the baker, to obey the rules specific to the baker binary (this can vary depending on the corresponding protocol). @@ -115,7 +110,6 @@ val create : ?delegates:string list -> ?remote_mode:bool -> ?liquidity_baking_toggle_vote:liquidity_baking_vote option -> - ?use_dal_node:string -> ?auto_patch_cli:bool -> Node.t -> Client.t -> @@ -144,7 +138,6 @@ val create_from_uris : ?delegates:string list -> ?remote_mode:bool -> ?liquidity_baking_toggle_vote:liquidity_baking_vote option -> - ?use_dal_node:string -> base_dir:string -> node_data_dir:string -> node_rpc_endpoint:Endpoint.t -> @@ -199,7 +192,6 @@ val init : ?delegates:string list -> ?remote_mode:bool -> ?liquidity_baking_toggle_vote:liquidity_baking_vote option -> - ?use_dal_node:string -> ?auto_patch_cli:bool -> Node.t -> Client.t -> -- GitLab From 6aa9b96202a977d9292ed77db9fb55c1f15205dd Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Thu, 30 Jan 2025 13:58:20 +0200 Subject: [PATCH 11/12] Agnostic_baker: Add entry in README for automatic patching --- src/bin_agnostic_baker/README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/bin_agnostic_baker/README.md b/src/bin_agnostic_baker/README.md index bec810985ea7..54665d46c123 100644 --- a/src/bin_agnostic_baker/README.md +++ b/src/bin_agnostic_baker/README.md @@ -34,6 +34,8 @@ to the agnostic baker binary and they include: - `--base-dir` : directory dedicated to the agnostic baker (that can contain logs and configuration files) -- `--help` : displays help information +-- `auto-patch-cli` : attempts to fix the CLI in case the baker binary arguments do not +obey the rules (specified in `baker_args_rules`) The `[OCTEZ-BAKER-COMMANDS]` list consists of all the arguments that can be used for the specific protocol baking binary. To be more clear, if a user wants to use @@ -48,6 +50,15 @@ baker run the binary for ``, information obtained from the node. Notice that the two types of arguments are separated by a clear `--`. +The automatic patching mechanism (specified by `--auto-patch-cli` flag) was added to solve +an existing issue: *potential incompatibility of command line arguments when swapping baking binaries*. +This happens because the `[OCTEZ-BAKER-COMMANDS]` is preserved when the baking binary is swapped, +therefore if there is an incompatibility between that list and the new protocol binary, the protocol +would be stopped. The patching approach relies on a list of hard-coded rules which, if not obeyed, +can be fixed (for instance if an argument is mandatory, it can be added with a default case, e.g. +`--liquidity-baking-toggle-vote` with default `pass`). The goal here is to increase the robustness +of the agnostic baker binary. + ## How it works 1. **Initialization**: The daemon starts and connects to the specified Tezos node. -- GitLab From 284710e55376c9b32ad396a124bdf5edcb43499f Mon Sep 17 00:00:00 2001 From: Gabriel Moise Date: Thu, 30 Jan 2025 13:58:33 +0200 Subject: [PATCH 12/12] Agnostic_baker: Add entry in CHANGES for automatic patching --- src/bin_agnostic_baker/CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/bin_agnostic_baker/CHANGES.md b/src/bin_agnostic_baker/CHANGES.md index 3f0a2247b72f..0c1d2d8153af 100644 --- a/src/bin_agnostic_baker/CHANGES.md +++ b/src/bin_agnostic_baker/CHANGES.md @@ -1,5 +1,6 @@ # Agnostic Baker Changelog +- Add automatic patching mechanism [!16240](https://gitlab.com/tezos/tezos/-/merge_requests/16240) - Improve existing documentation [!16491](https://gitlab.com/tezos/tezos/-/merge_requests/16491) - Fix and improve tezt tests [!16492](https://gitlab.com/tezos/tezos/-/merge_requests/16492) - Release agnostic baker as experimental [!16318](https://gitlab.com/tezos/tezos/-/merge_requests/16318) -- GitLab