[operator-tool] Add addtional checks to VerifyValidatorState command

Closes: #10106
666lcz · Dec 30, 2021 · a290b08 · a290b08
1 parent 65d3259
commit a290b08
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 34 deletions.
diff --git a/config/management/operational/src/command.rs b/config/management/operational/src/command.rs
@@ -73,9 +73,7 @@ pub enum Command {
     ValidatorConfig(crate::validator_config::ValidatorConfig),
     #[structopt(about = "Displays the current validator set infos registered on the blockchain")]
     ValidatorSet(crate::validator_set::ValidatorSet),
-    #[structopt(
-        about = "Compare a local validator state against the validator state held on-chain"
-    )]
+    #[structopt(about = "Compare the local validator state to the state held on-chain")]
     VerifyValidatorState(crate::validator_state::VerifyValidatorState),
 }
 

diff --git a/config/management/operational/src/test_helper.rs b/config/management/operational/src/test_helper.rs
@@ -718,12 +718,10 @@ impl OperationalTool {
             "
                 {command}
                 --json-server {host}
-                --chain-id {chain_id}
                 --validator-backend {backend_args}
             ",
             command = command(TOOL_NAME, CommandName::VerifyValidatorState),
             host = self.host,
-            chain_id = self.chain_id.id(),
             backend_args = backend_args(backend)?,
         );
         let command = Command::from_iter(args.split_whitespace());

diff --git a/config/management/operational/src/validator_state.rs b/config/management/operational/src/validator_state.rs
@@ -2,67 +2,119 @@
 // SPDX-License-Identifier: Apache-2.0
 
 use crate::{json_rpc::JsonRpcClientWrapper, validator_config::DecryptedValidatorConfig};
-use diem_global_constants::{CONSENSUS_KEY, OWNER_ACCOUNT};
-use diem_management::error::Error;
+use diem_global_constants::{
+    CONSENSUS_KEY, FULLNODE_NETWORK_KEY, OWNER_ACCOUNT, VALIDATOR_NETWORK_KEY,
+};
+use diem_management::{
+    config::ConfigPath, error::Error, secure_backend::ValidatorBackend, storage::to_x25519,
+};
 use serde::Serialize;
 use structopt::StructOpt;
 
-#[derive(Debug, Default, Serialize)]
+#[derive(Debug, Serialize)]
 pub struct VerifyValidatorStateResult {
-    /// Check if a validator is in the latest validator set on-chain.
-    pub in_validator_set: Option<bool>,
     /// Check if the consensus key held in secure storage matches
     /// that registered on-chain for the validator.
     pub consensus_key_match: Option<bool>,
+
+    /// Check if the consensus key is unique
+    pub consensus_key_unique: Option<bool>,
+
+    /// Check if the fullnode network key held in secure storage matches
+    /// that registered on-chain.
+    pub fullnode_network_key_match: Option<bool>,
+
+    /// Check if a validator is in the latest validator set on-chain.
+    pub in_validator_set: Option<bool>,
+
+    /// Check if the validator network key held in secure storage matches
+    /// that registered on-chain.
+    pub validator_network_key_match: Option<bool>,
 }
 
 impl VerifyValidatorStateResult {
-    pub fn is_state_consistent(&self) -> bool {
-        self.in_validator_set == Some(true) && self.consensus_key_match == Some(true)
+    pub fn is_valid_state(&self) -> bool {
+        self.in_validator_set == Some(true)
+            && self.consensus_key_match == Some(true)
+            && self.consensus_key_unique == Some(true)
+            && self.validator_network_key_match == Some(true)
+            && self.fullnode_network_key_match == Some(true)
     }
 }
 
 #[derive(Debug, StructOpt)]
 pub struct VerifyValidatorState {
+    #[structopt(flatten)]
+    config: ConfigPath,
     #[structopt(long, required_unless = "config")]
     json_server: Option<String>,
     #[structopt(flatten)]
-    validator_config: diem_management::validator_config::ValidatorConfig,
+    validator_backend: ValidatorBackend,
 }
 
 impl VerifyValidatorState {
     pub fn execute(self) -> Result<VerifyValidatorStateResult, Error> {
         // Load the config, storage backend and create a json rpc client.
         let config = self
-            .validator_config
-            .config()?
-            .override_json_server(&self.json_server);
+            .config
+            .load()?
+            .override_json_server(&self.json_server)
+            .override_validator_backend(&self.validator_backend.validator_backend)?;
         let storage = config.validator_backend();
         let client = JsonRpcClientWrapper::new(config.json_server);
         let owner_account = storage.account_address(OWNER_ACCOUNT)?;
 
-        // Verify if the validator is in the set
-        let in_validator_set = client
-            .validator_set(None)?
-            .iter()
-            .any(|vi| vi.account_address() == &owner_account);
+        let validator_infos = client.validator_set(None)?;
 
-        // TODO(khiemngo): consider return early if the validator is not in the set
+        let mut result = VerifyValidatorStateResult {
+            consensus_key_match: None,
+            consensus_key_unique: None,
+            fullnode_network_key_match: None,
+            in_validator_set: None,
+            validator_network_key_match: None,
+        };
 
-        // Fetch the current on-chain config for this operator's owner
+        // Verify if the validator is in the set.
+        result.in_validator_set = Some(
+            validator_infos
+                .iter()
+                .any(|vi| vi.account_address() == &owner_account),
+        );
+
+        // Fetch the current on-chain config for this operator's owner.
+        // Check if the consensus key held in secure storage matches
+        // that registered on-chain.
         let validator_config = client.validator_config(owner_account).and_then(|vc| {
             DecryptedValidatorConfig::from_validator_config_resource(&vc, owner_account)
         })?;
-
         let storage_key = storage.ed25519_public_from_private(CONSENSUS_KEY)?;
-        let consensus_key_match = storage_key == validator_config.consensus_public_key;
+        result.consensus_key_match = Some(storage_key == validator_config.consensus_public_key);
+
+        // Check if the consensus key is unique
+        result.consensus_key_unique = Some(!validator_infos.iter().any(|vi| {
+            vi.account_address() != &owner_account && vi.consensus_public_key() == &storage_key
+        }));
+
+        // Check if the validator network key held in secure storage
+        // matches that registered on-chain.
+        let storage_key = storage.ed25519_public_from_private(VALIDATOR_NETWORK_KEY)?;
+        result.validator_network_key_match = Some(
+            Some(to_x25519(storage_key)?)
+                == validator_config
+                    .validator_network_address
+                    .find_noise_proto(),
+        );
+
+        // Check if the fullnode network key held in secure storage
+        // matches that registered on-chain.
+        let storage_key = storage.ed25519_public_from_private(FULLNODE_NETWORK_KEY)?;
+        result.fullnode_network_key_match = Some(
+            Some(to_x25519(storage_key)?)
+                == validator_config.fullnode_network_address.find_noise_proto(),
+        );
 
-        // TODO(khiemngo): add checks for validator/fullnode network addresses
-        // TODO(khiemngo): add check for key uniqueness
+        Ok(result)
 
-        Ok(VerifyValidatorStateResult {
-            in_validator_set: Some(in_validator_set),
-            consensus_key_match: Some(consensus_key_match),
-        })
+        // TODO(khiemngo): Check if all keys match locally when compared with the validator infos
     }
 }
diff --git a/testsuite/smoke-test/src/operational_tooling.rs b/testsuite/smoke-test/src/operational_tooling.rs
@@ -1048,11 +1048,10 @@ async fn test_validator_set() {
 
 #[tokio::test]
 async fn test_verify_validator_state() {
-    let (_env, op_tool, backend, mut storage) = launch_swarm_with_op_tool_and_backend(4).await;
+    let (_env, op_tool, backend, mut storage) = launch_swarm_with_op_tool_and_backend(1).await;
 
     let result = op_tool.verify_validator_state(&backend).unwrap();
-    assert_eq!(result.in_validator_set, Some(true));
-    assert_eq!(result.consensus_key_match, Some(true));
+    assert!(result.is_valid_state());
 
     // Rotate consensus key locally, but we do not update it on-chain
     // Verify the local validator state again.
@@ -1061,8 +1060,12 @@ async fn test_verify_validator_state() {
     let result = op_tool.verify_validator_state(&backend).unwrap();
     assert_eq!(result.in_validator_set, Some(true));
     assert_eq!(result.consensus_key_match, Some(false));
+    assert_eq!(result.consensus_key_unique, Some(true));
+    assert_eq!(result.validator_network_key_match, Some(true));
+    assert_eq!(result.fullnode_network_key_match, Some(true));
 
     // TODO(khiemngo): consider adding test where the validator is no longer in set
+    // TODO(khiemngo): consider adding test where consensus key is not unique
 }
 
 /// Creates a new account address and key for testing.