Add remotekey API support (#3162)

## Issue Addressed

#3068

## Proposed Changes

Adds support for remote key API.

## Additional Info

Needed to add `is_local_keystore`  argument to `delete_definition_and_keystore` to know if we want to delete local or remote key. Previously this wasn't necessary because remotekeys(web3signers) could be deleted.

common/eth2/src/lighthouse_vc/http_client.rs

@@ -476,6 +476,16 @@ impl ValidatorClientHttpClient {
         Ok(url)
     }
 
+    fn make_remotekeys_url(&self) -> Result<Url, Error> {
+        let mut url = self.server.full.clone();
+        url.path_segments_mut()
+            .map_err(|()| Error::InvalidUrl(self.server.clone()))?
+            .push("eth")
+            .push("v1")
+            .push("remotekeys");
+        Ok(url)
+    }
+
     /// `GET lighthouse/auth`
     pub async fn get_auth(&self) -> Result<AuthResponse, Error> {
         let mut url = self.server.full.clone();
@@ -509,6 +519,30 @@ impl ValidatorClientHttpClient {
         let url = self.make_keystores_url()?;
         self.delete_with_unsigned_response(url, req).await
     }
+
+    /// `GET eth/v1/remotekeys`
+    pub async fn get_remotekeys(&self) -> Result<ListRemotekeysResponse, Error> {
+        let url = self.make_remotekeys_url()?;
+        self.get_unsigned(url).await
+    }
+
+    /// `POST eth/v1/remotekeys`
+    pub async fn post_remotekeys(
+        &self,
+        req: &ImportRemotekeysRequest,
+    ) -> Result<ImportRemotekeysResponse, Error> {
+        let url = self.make_remotekeys_url()?;
+        self.post_with_unsigned_response(url, req).await
+    }
+
+    /// `DELETE eth/v1/remotekeys`
+    pub async fn delete_remotekeys(
+        &self,
+        req: &DeleteRemotekeysRequest,
+    ) -> Result<DeleteRemotekeysResponse, Error> {
+        let url = self.make_remotekeys_url()?;
+        self.delete_with_unsigned_response(url, req).await
+    }
 }
 
 /// Returns `Ok(response)` if the response is a `200 OK` response. Otherwise, creates an

common/eth2/src/lighthouse_vc/std_types.rs

@@ -102,3 +102,59 @@ pub enum DeleteKeystoreStatus {
     NotFound,
     Error,
 }
+
+#[derive(Debug, Deserialize, Serialize, PartialEq)]
+pub struct ListRemotekeysResponse {
+    pub data: Vec<SingleListRemotekeysResponse>,
+}
+
+#[derive(Debug, Deserialize, Serialize, PartialEq)]
+pub struct SingleListRemotekeysResponse {
+    pub pubkey: PublicKeyBytes,
+    pub url: String,
+    pub readonly: bool,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct ImportRemotekeysRequest {
+    pub remote_keys: Vec<SingleImportRemotekeysRequest>,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
+pub struct SingleImportRemotekeysRequest {
+    pub pubkey: PublicKeyBytes,
+    pub url: String,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)]
+#[serde(rename_all = "lowercase")]
+pub enum ImportRemotekeyStatus {
+    Imported,
+    Duplicate,
+    Error,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct ImportRemotekeysResponse {
+    pub data: Vec<Status<ImportRemotekeyStatus>>,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct DeleteRemotekeysRequest {
+    pub pubkeys: Vec<PublicKeyBytes>,
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)]
+#[serde(rename_all = "snake_case")]
+pub enum DeleteRemotekeyStatus {
+    Deleted,
+    NotFound,
+    Error,
+}
+
+#[derive(Debug, Deserialize, Serialize)]
+pub struct DeleteRemotekeysResponse {
+    pub data: Vec<Status<DeleteRemotekeyStatus>>,
+}

validator_client/src/http_api/create_validator.rs

@@ -1,5 +1,5 @@
 use crate::ValidatorStore;
-use account_utils::validator_definitions::{SigningDefinition, ValidatorDefinition};
+use account_utils::validator_definitions::ValidatorDefinition;
 use account_utils::{
     eth2_wallet::{bip39::Mnemonic, WalletBuilder},
     random_mnemonic, random_password, ZeroizeString,
@@ -164,24 +164,12 @@ pub async fn create_validators_mnemonic<P: AsRef<Path>, T: 'static + SlotClock,
 }
 
 pub async fn create_validators_web3signer<T: 'static + SlotClock, E: EthSpec>(
-    validator_requests: &[api_types::Web3SignerValidatorRequest],
+    validators: Vec<ValidatorDefinition>,
     validator_store: &ValidatorStore<T, E>,
 ) -> Result<(), warp::Rejection> {
-    for request in validator_requests {
+    for validator in validators {
-        let validator_definition = ValidatorDefinition {
-            enabled: request.enable,
-            voting_public_key: request.voting_public_key.clone(),
-            graffiti: request.graffiti.clone(),
-            suggested_fee_recipient: request.suggested_fee_recipient,
-            description: request.description.clone(),
-            signing_definition: SigningDefinition::Web3Signer {
-                url: request.url.clone(),
-                root_certificate_path: request.root_certificate_path.clone(),
-                request_timeout_ms: request.request_timeout_ms,
-            },
-        };
         validator_store
-            .add_validator(validator_definition)
+            .add_validator(validator)
             .await
             .map_err(|e| {
                 warp_utils::reject::custom_server_error(format!(

validator_client/src/http_api/keystores.rs

@@ -1,5 +1,8 @@
 //! Implementation of the standard keystore management API.
-use crate::{signing_method::SigningMethod, InitializedValidators, ValidatorStore};
+use crate::{
+    initialized_validators::Error, signing_method::SigningMethod, InitializedValidators,
+    ValidatorStore,
+};
 use account_utils::ZeroizeString;
 use eth2::lighthouse_vc::std_types::{
     DeleteKeystoreStatus, DeleteKeystoresRequest, DeleteKeystoresResponse, ImportKeystoreStatus,
@@ -282,9 +285,14 @@ fn delete_single_keystore(
             .decompress()
             .map_err(|e| format!("invalid pubkey, {:?}: {:?}", pubkey_bytes, e))?;
 
-        runtime
+        match runtime.block_on(initialized_validators.delete_definition_and_keystore(&pubkey, true))
-            .block_on(initialized_validators.delete_definition_and_keystore(&pubkey))
+        {
-            .map_err(|e| format!("unable to disable and delete: {:?}", e))
+            Ok(_) => Ok(DeleteKeystoreStatus::Deleted),
+            Err(e) => match e {
+                Error::ValidatorNotInitialized(_) => Ok(DeleteKeystoreStatus::NotFound),
+                _ => Err(format!("unable to disable and delete: {:?}", e)),
+            },
+        }
     } else {
         Err("validator client shutdown".into())
     }

validator_client/src/http_api/mod.rs

@@ -1,10 +1,14 @@
 mod api_secret;
 mod create_validator;
 mod keystores;
+mod remotekeys;
 mod tests;
 
 use crate::ValidatorStore;
-use account_utils::mnemonic_from_phrase;
+use account_utils::{
+    mnemonic_from_phrase,
+    validator_definitions::{SigningDefinition, ValidatorDefinition},
+};
 use create_validator::{create_validators_mnemonic, create_validators_web3signer};
 use eth2::lighthouse_vc::{
     std_types::AuthResponse,
@@ -459,7 +463,25 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
              runtime: Weak<Runtime>| {
                 blocking_signed_json_task(signer, move || {
                     if let Some(runtime) = runtime.upgrade() {
-                        runtime.block_on(create_validators_web3signer(&body, &validator_store))?;
+                        let web3signers: Vec<ValidatorDefinition> = body
+                            .into_iter()
+                            .map(|web3signer| ValidatorDefinition {
+                                enabled: web3signer.enable,
+                                voting_public_key: web3signer.voting_public_key,
+                                graffiti: web3signer.graffiti,
+                                suggested_fee_recipient: web3signer.suggested_fee_recipient,
+                                description: web3signer.description,
+                                signing_definition: SigningDefinition::Web3Signer {
+                                    url: web3signer.url,
+                                    root_certificate_path: web3signer.root_certificate_path,
+                                    request_timeout_ms: web3signer.request_timeout_ms,
+                                },
+                            })
+                            .collect();
+                        runtime.block_on(create_validators_web3signer(
+                            web3signers,
+                            &validator_store,
+                        ))?;
                         Ok(())
                     } else {
                         Err(warp_utils::reject::custom_server_error(
@@ -536,6 +558,7 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
     // Standard key-manager endpoints.
     let eth_v1 = warp::path("eth").and(warp::path("v1"));
     let std_keystores = eth_v1.and(warp::path("keystores")).and(warp::path::end());
+    let std_remotekeys = eth_v1.and(warp::path("remotekeys")).and(warp::path::end());
 
     // GET /eth/v1/keystores
     let get_std_keystores = std_keystores
@@ -564,16 +587,50 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
     // DELETE /eth/v1/keystores
     let delete_std_keystores = std_keystores
         .and(warp::body::json())
-        .and(signer)
+        .and(signer.clone())
-        .and(validator_store_filter)
+        .and(validator_store_filter.clone())
-        .and(runtime_filter)
+        .and(runtime_filter.clone())
-        .and(log_filter)
+        .and(log_filter.clone())
         .and_then(|request, signer, validator_store, runtime, log| {
             blocking_signed_json_task(signer, move || {
                 keystores::delete(request, validator_store, runtime, log)
             })
         });
 
+    // GET /eth/v1/remotekeys
+    let get_std_remotekeys = std_remotekeys
+        .and(signer.clone())
+        .and(validator_store_filter.clone())
+        .and_then(|signer, validator_store: Arc<ValidatorStore<T, E>>| {
+            blocking_signed_json_task(signer, move || Ok(remotekeys::list(validator_store)))
+        });
+
+    // POST /eth/v1/remotekeys
+    let post_std_remotekeys = std_remotekeys
+        .and(warp::body::json())
+        .and(signer.clone())
+        .and(validator_store_filter.clone())
+        .and(runtime_filter.clone())
+        .and(log_filter.clone())
+        .and_then(|request, signer, validator_store, runtime, log| {
+            blocking_signed_json_task(signer, move || {
+                remotekeys::import(request, validator_store, runtime, log)
+            })
+        });
+
+    // DELETE /eth/v1/remotekeys
+    let delete_std_remotekeys = std_remotekeys
+        .and(warp::body::json())
+        .and(signer)
+        .and(validator_store_filter)
+        .and(runtime_filter)
+        .and(log_filter.clone())
+        .and_then(|request, signer, validator_store, runtime, log| {
+            blocking_signed_json_task(signer, move || {
+                remotekeys::delete(request, validator_store, runtime, log)
+            })
+        });
+
     let routes = warp::any()
         .and(authorization_header_filter)
         // Note: it is critical that the `authorization_header_filter` is applied to all routes.
@@ -588,17 +645,19 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
                         .or(get_lighthouse_spec)
                         .or(get_lighthouse_validators)
                         .or(get_lighthouse_validators_pubkey)
-                        .or(get_std_keystores),
+                        .or(get_std_keystores)
+                        .or(get_std_remotekeys),
                 )
                 .or(warp::post().and(
                     post_validators
                         .or(post_validators_keystore)
                         .or(post_validators_mnemonic)
                         .or(post_validators_web3signer)
-                        .or(post_std_keystores),
+                        .or(post_std_keystores)
+                        .or(post_std_remotekeys),
                 ))
                 .or(warp::patch().and(patch_validators))
-                .or(warp::delete().and(delete_std_keystores)),
+                .or(warp::delete().and(delete_std_keystores.or(delete_std_remotekeys))),
         )
         // The auth route is the only route that is allowed to be accessed without the API token.
         .or(warp::get().and(get_auth))

validator_client/src/http_api/remotekeys.rs

@@ -0,0 +1,211 @@
+//! Implementation of the standard remotekey management API.
+use crate::{initialized_validators::Error, InitializedValidators, ValidatorStore};
+use account_utils::validator_definitions::{SigningDefinition, ValidatorDefinition};
+use eth2::lighthouse_vc::std_types::{
+    DeleteRemotekeyStatus, DeleteRemotekeysRequest, DeleteRemotekeysResponse,
+    ImportRemotekeyStatus, ImportRemotekeysRequest, ImportRemotekeysResponse,
+    ListRemotekeysResponse, SingleListRemotekeysResponse, Status,
+};
+use slog::{info, warn, Logger};
+use slot_clock::SlotClock;
+use std::sync::{Arc, Weak};
+use tokio::runtime::Runtime;
+use types::{EthSpec, PublicKeyBytes};
+use url::Url;
+use warp::Rejection;
+use warp_utils::reject::custom_server_error;
+
+pub fn list<T: SlotClock + 'static, E: EthSpec>(
+    validator_store: Arc<ValidatorStore<T, E>>,
+) -> ListRemotekeysResponse {
+    let initialized_validators_rwlock = validator_store.initialized_validators();
+    let initialized_validators = initialized_validators_rwlock.read();
+
+    let keystores = initialized_validators
+        .validator_definitions()
+        .iter()
+        .filter(|def| def.enabled)
+        .filter_map(|def| {
+            let validating_pubkey = def.voting_public_key.compress();
+
+            match &def.signing_definition {
+                SigningDefinition::LocalKeystore { .. } => None,
+                SigningDefinition::Web3Signer { url, .. } => Some(SingleListRemotekeysResponse {
+                    pubkey: validating_pubkey,
+                    url: url.clone(),
+                    readonly: false,
+                }),
+            }
+        })
+        .collect::<Vec<_>>();
+
+    ListRemotekeysResponse { data: keystores }
+}
+
+pub fn import<T: SlotClock + 'static, E: EthSpec>(
+    request: ImportRemotekeysRequest,
+    validator_store: Arc<ValidatorStore<T, E>>,
+    runtime: Weak<Runtime>,
+    log: Logger,
+) -> Result<ImportRemotekeysResponse, Rejection> {
+    info!(
+        log,
+        "Importing remotekeys via standard HTTP API";
+        "count" => request.remote_keys.len(),
+    );
+    // Import each remotekey. Some remotekeys may fail to be imported, so we record a status for each.
+    let mut statuses = Vec::with_capacity(request.remote_keys.len());
+
+    for remotekey in request.remote_keys {
+        let status = if let Some(runtime) = runtime.upgrade() {
+            // Import the keystore.
+            match import_single_remotekey(
+                remotekey.pubkey,
+                remotekey.url,
+                &validator_store,
+                runtime,
+            ) {
+                Ok(status) => Status::ok(status),
+                Err(e) => {
+                    warn!(
+                        log,
+                        "Error importing keystore, skipped";
+                        "pubkey" => remotekey.pubkey.to_string(),
+                        "error" => ?e,
+                    );
+                    Status::error(ImportRemotekeyStatus::Error, e)
+                }
+            }
+        } else {
+            Status::error(
+                ImportRemotekeyStatus::Error,
+                "validator client shutdown".into(),
+            )
+        };
+        statuses.push(status);
+    }
+    Ok(ImportRemotekeysResponse { data: statuses })
+}
+
+fn import_single_remotekey<T: SlotClock + 'static, E: EthSpec>(
+    pubkey: PublicKeyBytes,
+    url: String,
+    validator_store: &ValidatorStore<T, E>,
+    runtime: Arc<Runtime>,
+) -> Result<ImportRemotekeyStatus, String> {
+    if let Err(url_err) = Url::parse(&url) {
+        return Err(format!("failed to parse remotekey URL: {}", url_err));
+    }
+
+    let pubkey = pubkey
+        .decompress()
+        .map_err(|_| format!("invalid pubkey: {}", pubkey))?;
+
+    if let Some(def) = validator_store
+        .initialized_validators()
+        .read()
+        .validator_definitions()
+        .iter()
+        .find(|def| def.voting_public_key == pubkey)
+    {
+        if def.signing_definition.is_local_keystore() {
+            return Err("Pubkey already present in local keystore.".into());
+        } else if def.enabled {
+            return Ok(ImportRemotekeyStatus::Duplicate);
+        }
+    }
+
+    // Remotekeys are stored as web3signers.
+    // The remotekey API provides less confgiuration option than the web3signer API.
+    let web3signer_validator = ValidatorDefinition {
+        enabled: true,
+        voting_public_key: pubkey,
+        graffiti: None,
+        suggested_fee_recipient: None,
+        description: String::from("Added by remotekey API"),
+        signing_definition: SigningDefinition::Web3Signer {
+            url,
+            root_certificate_path: None,
+            request_timeout_ms: None,
+        },
+    };
+    runtime
+        .block_on(validator_store.add_validator(web3signer_validator))
+        .map_err(|e| format!("failed to initialize validator: {:?}", e))?;
+
+    Ok(ImportRemotekeyStatus::Imported)
+}
+
+pub fn delete<T: SlotClock + 'static, E: EthSpec>(
+    request: DeleteRemotekeysRequest,
+    validator_store: Arc<ValidatorStore<T, E>>,
+    runtime: Weak<Runtime>,
+    log: Logger,
+) -> Result<DeleteRemotekeysResponse, Rejection> {
+    info!(
+        log,
+        "Deleting remotekeys via standard HTTP API";
+        "count" => request.pubkeys.len(),
+    );
+    // Remove from initialized validators.
+    let initialized_validators_rwlock = validator_store.initialized_validators();
+    let mut initialized_validators = initialized_validators_rwlock.write();
+
+    let statuses = request
+        .pubkeys
+        .iter()
+        .map(|pubkey_bytes| {
+            match delete_single_remotekey(
+                pubkey_bytes,
+                &mut initialized_validators,
+                runtime.clone(),
+            ) {
+                Ok(status) => Status::ok(status),
+                Err(error) => {
+                    warn!(
+                        log,
+                        "Error deleting keystore";
+                        "pubkey" => ?pubkey_bytes,
+                        "error" => ?error,
+                    );
+                    Status::error(DeleteRemotekeyStatus::Error, error)
+                }
+            }
+        })
+        .collect::<Vec<_>>();
+
+    // Use `update_validators` to update the key cache. It is safe to let the key cache get a bit out
+    // of date as it resets when it can't be decrypted. We update it just a single time to avoid
+    // continually resetting it after each key deletion.
+    if let Some(runtime) = runtime.upgrade() {
+        runtime
+            .block_on(initialized_validators.update_validators())
+            .map_err(|e| custom_server_error(format!("unable to update key cache: {:?}", e)))?;
+    }
+
+    Ok(DeleteRemotekeysResponse { data: statuses })
+}
+
+fn delete_single_remotekey(
+    pubkey_bytes: &PublicKeyBytes,
+    initialized_validators: &mut InitializedValidators,
+    runtime: Weak<Runtime>,
+) -> Result<DeleteRemotekeyStatus, String> {
+    if let Some(runtime) = runtime.upgrade() {
+        let pubkey = pubkey_bytes
+            .decompress()
+            .map_err(|e| format!("invalid pubkey, {:?}: {:?}", pubkey_bytes, e))?;
+
+        match runtime
+            .block_on(initialized_validators.delete_definition_and_keystore(&pubkey, false))
+        {
+            Ok(_) => Ok(DeleteRemotekeyStatus::Deleted),
+            Err(e) => match e {
+                Error::ValidatorNotInitialized(_) => Ok(DeleteRemotekeyStatus::NotFound),
+                _ => Err(format!("unable to disable and delete: {:?}", e)),
+            },
+        }
+    } else {
+        Err("validator client shutdown".into())
+    }
+}

validator_client/src/initialized_validators.rs

@@ -14,7 +14,6 @@ use account_utils::{
     },
     ZeroizeString,
 };
-use eth2::lighthouse_vc::std_types::DeleteKeystoreStatus;
 use eth2_keystore::Keystore;
 use lighthouse_metrics::set_gauge;
 use lockfile::{Lockfile, LockfileError};
@@ -90,8 +89,8 @@ pub enum Error {
     InvalidWeb3SignerRootCertificateFile(io::Error),
     InvalidWeb3SignerRootCertificate(ReqwestError),
     UnableToBuildWeb3SignerClient(ReqwestError),
-    /// Unable to apply an action to a validator because it is using a remote signer.
+    /// Unable to apply an action to a validator.
-    InvalidActionOnRemoteValidator,
+    InvalidActionOnValidator,
 }
 
 impl From<LockfileError> for Error {
@@ -443,7 +442,8 @@ impl InitializedValidators {
     pub async fn delete_definition_and_keystore(
         &mut self,
         pubkey: &PublicKey,
-    ) -> Result<DeleteKeystoreStatus, Error> {
+        is_local_keystore: bool,
+    ) -> Result<(), Error> {
         // 1. Disable the validator definition.
         //
         // We disable before removing so that in case of a crash the auto-discovery mechanism
@@ -454,16 +454,19 @@ impl InitializedValidators {
             .iter_mut()
             .find(|def| &def.voting_public_key == pubkey)
         {
-            if def.signing_definition.is_local_keystore() {
+            // Update definition for local keystore
+            if def.signing_definition.is_local_keystore() && is_local_keystore {
                 def.enabled = false;
                 self.definitions
                     .save(&self.validators_dir)
                     .map_err(Error::UnableToSaveDefinitions)?;
+            } else if !def.signing_definition.is_local_keystore() && !is_local_keystore {
+                def.enabled = false;
             } else {
-                return Err(Error::InvalidActionOnRemoteValidator);
+                return Err(Error::InvalidActionOnValidator);
             }
         } else {
-            return Ok(DeleteKeystoreStatus::NotFound);
+            return Err(Error::ValidatorNotInitialized(pubkey.clone()));
         }
 
         // 2. Delete from `self.validators`, which holds the signing method.
@@ -491,7 +494,7 @@ impl InitializedValidators {
             .save(&self.validators_dir)
             .map_err(Error::UnableToSaveDefinitions)?;
 
-        Ok(DeleteKeystoreStatus::Deleted)
+        Ok(())
     }
 
     /// Attempt to delete the voting keystore file, or its entire validator directory.