Validator manager commands for the Keymanager APIs (#6261)

* Validator manager commands for standard key-manager APIs * Merge latest unstable * Fix Some in lib.rs * Replace Arg::with_name with Arg::new * Update takes_value * Remove clap::App * Change App to Command * Add command in use * Remove generic in ArgMatches * Fix matches.get_flag * Fixes * fix error handling * SetTrue in import * Fix * Fix builder-proposal flag (will delete the flag later) * Minor fix * Fix prefer_builder_proposals * Remove unwrap * Error handling from Michael * Add cli help text * Use None in import to simplify * Delete unwrap * Revert flags option * Simplify help command code * Remove flag header in move * Merge remote-tracking branch 'origin/unstable' into pahor/validator-manager-standard-keystore * Add log in VC when keystore is deleted * Delete duplicated log when validator does not exist * Simplify log code * Rename remove to delete * cargo-fmt * Try to remove a function * make-cli * Error handling * Merge branch 'vm' of https://github.com/chong-he/lighthouse into vm * Update CLI hel text * make-cli * Fix checks * Merge branch 'vm' of https://github.com/chong-he/lighthouse into vm * Try to fix check errors * Fix test * Remove changes * Update flag name * CLI display order * Move builde_proposals flag * Add doc * mdlint * Update validator_manager/src/list_validators.rs Co-authored-by: Mac L <mjladson@pm.me> * Delete empty line * Fix list * Simplify delete * Add support to delete more validators * Fix test * Rename response * Add (s) * Add test to delete multiple validators * Book and cli * Make cli * Only log when keystore is deleted * Revise deletion log * Add validator pubkey to error message * Merge import * Thank you Mac * Test * Add flags * Error handling for password * make cli * Merge remote-tracking branch 'origin/unstable' into vm * make cli * Fix test * Merge branch 'vm' of https://github.com/chong-he/lighthouse into vm * Fix test * vm test * Debug trait thank you Michael * Fix test * Merge branch 'unstable' into vm * test * testing * Combine import validator(s) * make cli * Add requires * Update book * mdlint * Only show import log when import is successful * delete testing * Test for standard format * Test standard format * Test * fix builder_proposals flag * Fix test for standard format * Add requires * Fix vm test * make cli * Remove flag header * Merge branch 'vm' of https://github.com/chong-he/lighthouse into vm * make cli * Delete space * Merge branch 'vm' of https://github.com/chong-he/lighthouse into vm * Merge branch 'unstable' into vm * Rename delete_validator to delete_validators * Rearrange * Remove pub in run function * Fix grammar * Apply suggestions from code review Co-authored-by: Michael Sproul <micsproul@gmail.com> * Remove description * Merge branch 'vm' of https://github.com/chong-he/lighthouse into vm * Close bracket * make cli * Revise list code and test * Revise import flag * make cli * Comment out test * Update vm test * Simplify * Merge remote-tracking branch 'origin/unstable' into vm * make cli * Add test * Add password as a requirement for keystore file * Correct flags in docs * typo
sigp · Oct 29, 2024 · fdf456f · fdf456f
1 parent fe889c6
commit fdf456f
Show file tree

Hide file tree

Showing 16 changed files with 1,031 additions and 73 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md
@@ -15,6 +15,7 @@
   * [The `validator-manager` Command](./validator-manager.md)
     * [Creating validators](./validator-manager-create.md)
     * [Moving validators](./validator-manager-move.md)
+    * [Managing validators](./validator-manager-api.md)
   * [Slashing Protection](./slashing-protection.md)
   * [Voluntary Exits](./voluntary-exit.md)
   * [Partial Withdrawals](./partial-withdrawal.md)

diff --git a/book/src/help_vm.md b/book/src/help_vm.md
@@ -23,6 +23,11 @@ Commands:
           "create-validators" command. This command only supports validators
           signing via a keystore on the local file system (i.e., not Web3Signer
           validators).
+  list
+          Lists all validators in a validator client using the HTTP API.
+  delete
+          Deletes one or more validators from a validator client using the HTTP
+          API.
   help
           Print this message or the help of the given subcommand(s)
 

diff --git a/book/src/help_vm_import.md b/book/src/help_vm_import.md
@@ -5,9 +5,17 @@ Uploads validators to a validator client using the HTTP API. The validators are
 defined in a JSON file which can be generated using the "create-validators"
 command.
 
-Usage: lighthouse validator_manager import [OPTIONS] --validators-file <PATH_TO_JSON_FILE>
+Usage: lighthouse validator_manager import [OPTIONS]
 
 Options:
+      --builder-boost-factor <UINT64>
+          When provided, the imported validator will use this percentage
+          multiplier to apply to the builder's payload value when choosing
+          between a builder payload header and payload from the local execution
+          node.
+      --builder-proposals <builder-proposals>
+          When provided, the imported validator will attempt to create blocks
+          via builder rather than the local EL. [possible values: true, false]
   -d, --datadir <DIR>
           Used to specify a custom root data directory for lighthouse keys and
           databases. Defaults to $HOME/.lighthouse/{network} where network is
@@ -17,6 +25,10 @@ Options:
           Specifies the verbosity level used when emitting logs to the terminal.
           [default: info] [possible values: info, debug, trace, warn, error,
           crit]
+      --gas-limit <UINT64>
+          When provided, the imported validator will use this gas limit. It is
+          recommended to leave this as the default value by not specifying this
+          flag.
       --genesis-state-url <URL>
           A URL of a beacon-API compatible server from which to download the
           genesis state. Checkpoint sync server URLs can generally be used with
@@ -26,6 +38,10 @@ Options:
       --genesis-state-url-timeout <SECONDS>
           The timeout in seconds for the request to --genesis-state-url.
           [default: 180]
+      --keystore-file <PATH_TO_KEYSTORE_FILE>
+          The path to a keystore JSON file to be imported to the validator
+          client. This file is usually created using staking-deposit-cli or
+          ethstaker-deposit-cli
       --log-format <FORMAT>
           Specifies the log format used when emitting logs to the terminal.
           [possible values: JSON]
@@ -50,6 +66,15 @@ Options:
       --network <network>
           Name of the Eth2 chain Lighthouse will sync and follow. [possible
           values: mainnet, gnosis, chiado, sepolia, holesky]
+      --password <STRING>
+          Password of the keystore file.
+      --prefer-builder-proposals <prefer-builder-proposals>
+          When provided, the imported validator will always prefer blocks
+          constructed by builders, regardless of payload value. [possible
+          values: true, false]
+      --suggested-fee-recipient <ETH1_ADDRESS>
+          When provided, the imported validator will use the suggested fee
+          recipient. Omit this flag to use the default value from the VC.
   -t, --testnet-dir <DIR>
           Path to directory containing eth2_testnet specs. Defaults to a
           hard-coded Lighthouse testnet. Only effective if there is no existing
@@ -60,10 +85,8 @@ Options:
       --vc-token <PATH>
           The file containing a token required by the validator client.
       --vc-url <HTTP_ADDRESS>
-          A HTTP(S) address of a validator client using the keymanager-API. If
-          this value is not supplied then a 'dry run' will be conducted where no
-          changes are made to the validator client. [default:
-          http://localhost:5062]
+          A HTTP(S) address of a validator client using the keymanager-API.
+          [default: http://localhost:5062]
 
 Flags:
       --disable-log-timestamp

diff --git a/book/src/validator-manager-api.md b/book/src/validator-manager-api.md
@@ -0,0 +1,39 @@
+# Managing Validators
+
+The `lighthouse validator-manager` uses the [Keymanager API](https://ethereum.github.io/keymanager-APIs/#/) to list, import and delete keystores via the HTTP API. This requires the validator client running with the flag `--http`.
+
+## Delete
+
+The `delete` command deletes one or more validators from the validator client. It will also modify the `validator_definitions.yml` file automatically so there is no manual action required from the user after the delete. To `delete`:
+
+```bash
+lighthouse vm delete --vc-token <API-TOKEN-PATH> --validators pubkey1,pubkey2
+```
+
+Example:
+
+```bash
+lighthouse vm delete --vc-token ~/.lighthouse/mainnet/validators/api-token.txt --validators 0x8885c29b8f88ee9b9a37b480fd4384fed74bda33d85bc8171a904847e65688b6c9bb4362d6597fd30109fb2def6c3ae4,0xa262dae3dcd2b2e280af534effa16bedb27c06f2959e114d53bd2a248ca324a018dc73179899a066149471a94a1bc92f
+```
+
+## Import
+
+The `import` command imports validator keystores generated by the staking-deposit-cli/ethstaker-deposit-cli. To import a validator keystore:
+
+```bash
+lighthouse vm import --vc-token <API-TOKEN-PATH> --keystore-file /path/to/json --password keystore_password
+```
+
+Example:
+
+```
+lighthouse vm import --vc-token ~/.lighthouse/mainnet/validators/api-token.txt --keystore-file keystore.json --password keystore_password
+```
+
+## List
+
+To list the validators running on the validator client:
+
+```bash
+lighthouse vm list --vc-token ~/.lighthouse/mainnet/validators/api-token.txt
+```
diff --git a/book/src/validator-manager-create.md b/book/src/validator-manager-create.md
@@ -69,6 +69,8 @@ lighthouse \
 > Be sure to remove `./validators.json` after the import is successful since it
 > contains unencrypted validator keystores.
 
+> Note: To import validators with validator-manager using keystore files created using the staking deposit CLI, refer to [Managing Validators](./validator-manager-api.md#import).
+
 ## Detailed Guide
 
 This guide will create two validators and import them to a VC. For simplicity,

diff --git a/common/account_utils/src/lib.rs b/common/account_utils/src/lib.rs
@@ -9,13 +9,16 @@ use eth2_wallet::{
 use filesystem::{create_with_600_perms, Error as FsError};
 use rand::{distributions::Alphanumeric, Rng};
 use serde::{Deserialize, Serialize};
-use std::fs::{self, File};
 use std::io;
 use std::io::prelude::*;
 use std::path::{Path, PathBuf};
 use std::str::from_utf8;
 use std::thread::sleep;
 use std::time::Duration;
+use std::{
+    fs::{self, File},
+    str::FromStr,
+};
 use zeroize::Zeroize;
 
 pub mod validator_definitions;
@@ -215,6 +218,14 @@ pub fn mnemonic_from_phrase(phrase: &str) -> Result<Mnemonic, String> {
 #[serde(transparent)]
 pub struct ZeroizeString(String);
 
+impl FromStr for ZeroizeString {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(Self(s.to_owned()))
+    }
+}
+
 impl From<String> for ZeroizeString {
     fn from(s: String) -> Self {
         Self(s)

diff --git a/lighthouse/tests/validator_manager.rs b/lighthouse/tests/validator_manager.rs
@@ -9,7 +9,9 @@ use tempfile::{tempdir, TempDir};
 use types::*;
 use validator_manager::{
     create_validators::CreateConfig,
+    delete_validators::DeleteConfig,
     import_validators::ImportConfig,
+    list_validators::ListConfig,
     move_validators::{MoveConfig, PasswordSource, Validators},
 };
 
@@ -105,6 +107,18 @@ impl CommandLineTest<MoveConfig> {
     }
 }
 
+impl CommandLineTest<ListConfig> {
+    fn validators_list() -> Self {
+        Self::default().flag("list", None)
+    }
+}
+
+impl CommandLineTest<DeleteConfig> {
+    fn validators_delete() -> Self {
+        Self::default().flag("delete", None)
+    }
+}
+
 #[test]
 pub fn validator_create_without_output_path() {
     CommandLineTest::validators_create().assert_failed();
@@ -199,10 +213,18 @@ pub fn validator_import_defaults() {
         .flag("--vc-token", Some("./token.json"))
         .assert_success(|config| {
             let expected = ImportConfig {
-                validators_file_path: PathBuf::from("./vals.json"),
+                validators_file_path: Some(PathBuf::from("./vals.json")),
+                keystore_file_path: None,
                 vc_url: SensitiveUrl::parse("http://localhost:5062").unwrap(),
                 vc_token_path: PathBuf::from("./token.json"),
                 ignore_duplicates: false,
+                password: None,
+                fee_recipient: None,
+                builder_boost_factor: None,
+                gas_limit: None,
+                builder_proposals: None,
+                enabled: None,
+                prefer_builder_proposals: None,
             };
             assert_eq!(expected, config);
         });
@@ -216,10 +238,18 @@ pub fn validator_import_misc_flags() {
         .flag("--ignore-duplicates", None)
         .assert_success(|config| {
             let expected = ImportConfig {
-                validators_file_path: PathBuf::from("./vals.json"),
+                validators_file_path: Some(PathBuf::from("./vals.json")),
+                keystore_file_path: None,
                 vc_url: SensitiveUrl::parse("http://localhost:5062").unwrap(),
                 vc_token_path: PathBuf::from("./token.json"),
                 ignore_duplicates: true,
+                password: None,
+                fee_recipient: None,
+                builder_boost_factor: None,
+                gas_limit: None,
+                builder_proposals: None,
+                enabled: None,
+                prefer_builder_proposals: None,
             };
             assert_eq!(expected, config);
         });
@@ -233,7 +263,17 @@ pub fn validator_import_missing_token() {
 }
 
 #[test]
-pub fn validator_import_missing_validators_file() {
+pub fn validator_import_using_both_file_flags() {
+    CommandLineTest::validators_import()
+        .flag("--vc-token", Some("./token.json"))
+        .flag("--validators-file", Some("./vals.json"))
+        .flag("--keystore-file", Some("./keystore.json"))
+        .flag("--password", Some("abcd"))
+        .assert_failed();
+}
+
+#[test]
+pub fn validator_import_missing_both_file_flags() {
     CommandLineTest::validators_import()
         .flag("--vc-token", Some("./token.json"))
         .assert_failed();
@@ -394,3 +434,37 @@ pub fn validator_move_count() {
             assert_eq!(expected, config);
         });
 }
+
+#[test]
+pub fn validator_list_defaults() {
+    CommandLineTest::validators_list()
+        .flag("--vc-token", Some("./token.json"))
+        .assert_success(|config| {
+            let expected = ListConfig {
+                vc_url: SensitiveUrl::parse("http://localhost:5062").unwrap(),
+                vc_token_path: PathBuf::from("./token.json"),
+            };
+            assert_eq!(expected, config);
+        });
+}
+
+#[test]
+pub fn validator_delete_defaults() {
+    CommandLineTest::validators_delete()
+        .flag(
+            "--validators",
+            Some(&format!("{},{}", EXAMPLE_PUBKEY_0, EXAMPLE_PUBKEY_1)),
+        )
+        .flag("--vc-token", Some("./token.json"))
+        .assert_success(|config| {
+            let expected = DeleteConfig {
+                vc_url: SensitiveUrl::parse("http://localhost:5062").unwrap(),
+                vc_token_path: PathBuf::from("./token.json"),
+                validators_to_delete: vec![
+                    PublicKeyBytes::from_str(EXAMPLE_PUBKEY_0).unwrap(),
+                    PublicKeyBytes::from_str(EXAMPLE_PUBKEY_1).unwrap(),
+                ],
+            };
+            assert_eq!(expected, config);
+        });
+}
diff --git a/validator_client/src/http_api/keystores.rs b/validator_client/src/http_api/keystores.rs
@@ -75,12 +75,6 @@ pub fn import<T: SlotClock + 'static, E: EthSpec>(
         )));
     }
 
-    info!(
-        log,
-        "Importing keystores via standard HTTP API";
-        "count" => request.keystores.len(),
-    );
-
     // Import slashing protection data before keystores, so that new keystores don't start signing
     // without it. Do not return early on failure, propagate the failure to each key.
     let slashing_protection_status =
@@ -156,6 +150,19 @@ pub fn import<T: SlotClock + 'static, E: EthSpec>(
         statuses.push(status);
     }
 
+    let successful_import = statuses
+        .iter()
+        .filter(|status| matches!(status.status, ImportKeystoreStatus::Imported))
+        .count();
+
+    if successful_import > 0 {
+        info!(
+            log,
+            "Imported keystores via standard HTTP API";
+            "count" => successful_import,
+        );
+    }
+
     Ok(ImportKeystoresResponse { data: statuses })
 }
 
@@ -238,7 +245,23 @@ pub fn delete<T: SlotClock + 'static, E: EthSpec>(
     task_executor: TaskExecutor,
     log: Logger,
 ) -> Result<DeleteKeystoresResponse, Rejection> {
-    let export_response = export(request, validator_store, task_executor, log)?;
+    let export_response = export(request, validator_store, task_executor, log.clone())?;
+
+    // Check the status is Deleted to confirm deletion is successful, then only display the log
+    let successful_deletion = export_response
+        .data
+        .iter()
+        .filter(|response| matches!(response.status.status, DeleteKeystoreStatus::Deleted))
+        .count();
+
+    if successful_deletion > 0 {
+        info!(
+            log,
+            "Deleted keystore via standard HTTP API";
+            "count" => successful_deletion,
+        );
+    }
+
     Ok(DeleteKeystoresResponse {
         data: export_response
             .data

diff --git a/validator_manager/Cargo.toml b/validator_manager/Cargo.toml
@@ -20,6 +20,7 @@ tree_hash = { workspace = true }
 eth2 = { workspace = true }
 hex = { workspace = true }
 tokio = { workspace = true }
+derivative = { workspace = true }
 
 [dev-dependencies]
 tempfile = { workspace = true }

diff --git a/validator_manager/src/create_validators.rs b/validator_manager/src/create_validators.rs
@@ -45,15 +45,6 @@ pub fn cli_app() -> Command {
                 Another, optional JSON file is created which contains a list of validator \
                 deposits in the same format as the \"ethereum/staking-deposit-cli\" tool.",
         )
-        .arg(
-            Arg::new("help")
-                .long("help")
-                .short('h')
-                .help("Prints help information")
-                .action(ArgAction::HelpLong)
-                .display_order(0)
-                .help_heading(FLAG_HEADER),
-        )
         .arg(
             Arg::new(OUTPUT_PATH_FLAG)
                 .long(OUTPUT_PATH_FLAG)