Remote key manager:

- Ignore duplicate keys during import instead of returning an error.
- Fix incorrect listed remote key pubkey attribute key.

Author: povi

keymanager/src/keystores.rs

@@ -10,6 +10,7 @@ use eip_2335::Keystore;
 use futures::lock::{MappedMutexGuard, Mutex, MutexGuard};
 use itertools::Itertools as _;
 use log::{info, warn};
+use serde::Serialize;
 use signer::{KeyOrigin, Signer};
 use slashing_protection::{interchange_format::InterchangeFormat, SlashingProtector};
 use std_ext::ArcExt as _;
@@ -20,7 +21,7 @@ use uuid::Uuid;
 use validator_key_cache::ValidatorKeyCache;
 use zeroize::Zeroizing;
 
-use crate::misc::{Error, OperationStatus, Status, ValidatingPubkey};
+use crate::misc::{Error, OperationStatus, Status};
 
 const KEYSTORE_STORAGE_FILE: &str = "keystores.json";
 
@@ -48,6 +49,12 @@ impl PersistenceConfig {
     }
 }
 
+#[derive(Debug, PartialEq, Eq, Serialize)]
+pub struct ValidatingPubkey {
+    pub validating_pubkey: PublicKeyBytes,
+    pub readonly: bool,
+}
+
 pub struct KeystoreManager {
     signer: Arc<RwLock<Signer>>,
     slashing_protector: Arc<Mutex<SlashingProtector>>,
@@ -250,7 +257,6 @@ impl KeystoreManager {
             .keys_with_origin()
             .map(|(pubkey, origin)| ValidatingPubkey {
                 validating_pubkey: pubkey,
-                url: None,
                 readonly: match origin {
                     KeyOrigin::KeymanagerAPI => false,
                     KeyOrigin::LocalFileSystem | KeyOrigin::Web3Signer => true,
@@ -614,7 +620,6 @@ mod tests {
             manager.list_validating_pubkeys().await,
             vec![ValidatingPubkey {
                 validating_pubkey: expected_pubkey,
-                url: None,
                 readonly: false
             }],
         );
@@ -646,7 +651,6 @@ mod tests {
             manager.list_validating_pubkeys().await,
             vec![ValidatingPubkey {
                 validating_pubkey: expected_pubkey,
-                url: None,
                 readonly: false
             }],
         );
@@ -750,7 +754,6 @@ mod tests {
             manager.list_validating_pubkeys().await,
             vec![ValidatingPubkey {
                 validating_pubkey: expected_pubkey,
-                url: None,
                 readonly: false
             }],
         );
@@ -782,7 +785,6 @@ mod tests {
             manager.list_validating_pubkeys().await,
             vec![ValidatingPubkey {
                 validating_pubkey: expected_pubkey,
-                url: None,
                 readonly: false
             }],
         );

keymanager/src/lib.rs

@@ -1,6 +1,6 @@
-pub use keystores::{load_key_storage, load_key_storage_password};
-pub use misc::{OperationStatus as KeymanagerOperationStatus, ValidatingPubkey};
-pub use remote_keys::RemoteKey;
+pub use keystores::{load_key_storage, load_key_storage_password, ValidatingPubkey};
+pub use misc::OperationStatus as KeymanagerOperationStatus;
+pub use remote_keys::{ListedRemoteKey, RemoteKey};
 
 pub use crate::proposer_configs::ProposerConfigs;
 

keymanager/src/misc.rs

@@ -1,5 +1,4 @@
 use anyhow::Error as AnyhowError;
-use bls::PublicKeyBytes;
 use serde::Serialize;
 use thiserror::Error;
 use types::phase0::primitives::H256;
@@ -28,6 +27,7 @@ pub enum Error {
 #[serde(rename_all = "snake_case")]
 pub enum Status {
     Deleted,
+    Duplicate,
     Error,
     Imported,
 }
@@ -64,11 +64,3 @@ impl From<Status> for OperationStatus {
         }
     }
 }
-
-#[derive(Debug, PartialEq, Eq, Serialize)]
-pub struct ValidatingPubkey {
-    pub validating_pubkey: PublicKeyBytes,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub url: Option<String>,
-    pub readonly: bool,
-}

keymanager/src/remote_keys.rs

@@ -4,12 +4,19 @@ use anyhow::{anyhow, Result};
 use bls::PublicKeyBytes;
 use futures::lock::Mutex;
 use reqwest::Url;
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use signer::{KeyOrigin, Signer};
 use slashing_protection::SlashingProtector;
 use tokio::sync::RwLock;
 
-use crate::misc::{Error, OperationStatus, Status, ValidatingPubkey};
+use crate::misc::{Error, OperationStatus, Status};
+
+#[derive(Debug, PartialEq, Eq, Serialize)]
+pub struct ListedRemoteKey {
+    pub pubkey: PublicKeyBytes,
+    pub url: String,
+    pub readonly: bool,
+}
 
 #[derive(Debug, PartialEq, Eq, Deserialize)]
 pub struct RemoteKey {
@@ -71,7 +78,7 @@ impl RemoteKeyManager {
                                 imported_pubkeys.push(pubkey);
                                 Status::Imported.into()
                             } else {
-                                Error::Duplicate.into()
+                                Status::Duplicate.into()
                             }
                         }
                         Err(error) => anyhow!(error).into(),
@@ -88,14 +95,14 @@ impl RemoteKeyManager {
         Ok(statuses)
     }
 
-    pub async fn list(&self) -> Vec<ValidatingPubkey> {
+    pub async fn list(&self) -> Vec<ListedRemoteKey> {
         self.signer
             .read()
             .await
             .web3signer_keys()
-            .map(|(pubkey, url)| ValidatingPubkey {
-                validating_pubkey: pubkey,
-                url: Some(url.to_string()),
+            .map(|(pubkey, url)| ListedRemoteKey {
+                pubkey,
+                url: url.to_string(),
                 readonly: false,
             })
             .collect()
@@ -169,8 +176,8 @@ mod tests {
                     message: None,
                 },
                 OperationStatus {
-                    status: Status::Error,
-                    message: Some("key already exists".into()),
+                    status: Status::Duplicate,
+                    message: None,
                 },
             ]
         );
@@ -182,9 +189,9 @@ mod tests {
 
         assert_eq!(
             manager.list().await,
-            [ValidatingPubkey {
-                validating_pubkey: PUBKEY_REMOTE,
-                url: Some("https://www.example.com/".into()),
+            [ListedRemoteKey {
+                pubkey: PUBKEY_REMOTE,
+                url: "https://www.example.com/".into(),
                 readonly: false,
             }],
         );

validator/src/api.rs

@@ -28,7 +28,9 @@ use jwt_simple::{
     claims::{JWTClaims, NoCustomClaims},
     reexports::coarsetime::Clock,
 };
-use keymanager::{KeyManager, KeymanagerOperationStatus, RemoteKey, ValidatingPubkey};
+use keymanager::{
+    KeyManager, KeymanagerOperationStatus, ListedRemoteKey, RemoteKey, ValidatingPubkey,
+};
 use log::{debug, info};
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use signer::{Signer, SigningMessage};
@@ -546,7 +548,7 @@ async fn keymanager_delete_keystores(
 /// `GET /eth/v1/remotekeys`
 async fn keymanager_list_remote_keys(
     State(keymanager): State<Arc<KeyManager>>,
-) -> Result<EthResponse<Vec<ValidatingPubkey>>, Error> {
+) -> Result<EthResponse<Vec<ListedRemoteKey>>, Error> {
     let remote_keys = keymanager.remote_keys().list().await;
 
     Ok(EthResponse::json(remote_keys))