Share reqwest::Client between validators when using Web3Signer (#3335)

## Issue Addressed

#3302

## Proposed Changes

Move the `reqwest::Client` from being initialized per-validator, to being initialized per distinct Web3Signer. 
This is done by placing the `Client` into a `HashMap` keyed by the definition of the Web3Signer as specified by the `ValidatorDefintion`. This will allow multiple Web3Signers to be used with a single VC and also maintains backwards compatibility.

## Additional Info

This was done to reduce the memory used by the VC when connecting to a Web3Signer.

I set up a local testnet using [a custom script](https://github.com/macladson/lighthouse/tree/web3signer-local-test/scripts/local_testnet_web3signer) and ran a VC with 200 validator keys:


VC with Web3Signer:
- `unstable`: ~200MB
- With fix: ~50MB



VC with Local Signer:
- `unstable`: ~35MB
- With fix: ~35MB 


> I'm seeing some fragmentation with the VC using the Web3Signer, but not when using a local signer (this is most likely due to making lots of http requests and dealing with lots of JSON objects). I tested the above using `MALLOC_ARENA_MAX=1` to try to reduce the fragmentation. Without it, the values are around +50MB for both `unstable` and the fix.
This commit is contained in:
Mac L 2022-07-19 05:48:05 +00:00
parent e5e4e62758
commit 7dbc59efeb
5 changed files with 124 additions and 75 deletions

View File

@ -45,6 +45,29 @@ pub enum Error {
UnableToCreateValidatorDir(PathBuf),
}
#[derive(Clone, PartialEq, Serialize, Deserialize, Hash, Eq)]
pub struct Web3SignerDefinition {
pub url: String,
/// Path to a .pem file.
#[serde(skip_serializing_if = "Option::is_none")]
pub root_certificate_path: Option<PathBuf>,
/// Specifies a request timeout.
///
/// The timeout is applied from when the request starts connecting until the response body has finished.
#[serde(skip_serializing_if = "Option::is_none")]
pub request_timeout_ms: Option<u64>,
/// Path to a PKCS12 file.
#[serde(skip_serializing_if = "Option::is_none")]
pub client_identity_path: Option<PathBuf>,
/// Password for the PKCS12 file.
///
/// An empty password will be used if this is omitted.
#[serde(skip_serializing_if = "Option::is_none")]
pub client_identity_password: Option<String>,
}
/// Defines how the validator client should attempt to sign messages for this validator.
#[derive(Clone, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type")]
@ -62,27 +85,7 @@ pub enum SigningDefinition {
///
/// https://github.com/ConsenSys/web3signer
#[serde(rename = "web3signer")]
Web3Signer {
url: String,
/// Path to a .pem file.
#[serde(skip_serializing_if = "Option::is_none")]
root_certificate_path: Option<PathBuf>,
/// Specifies a request timeout.
///
/// The timeout is applied from when the request starts connecting until the response body has finished.
#[serde(skip_serializing_if = "Option::is_none")]
request_timeout_ms: Option<u64>,
/// Path to a PKCS12 file.
#[serde(skip_serializing_if = "Option::is_none")]
client_identity_path: Option<PathBuf>,
/// Password for the PKCS12 file.
///
/// An empty password will be used if this is omitted.
#[serde(skip_serializing_if = "Option::is_none")]
client_identity_password: Option<String>,
},
Web3Signer(Web3SignerDefinition),
}
impl SigningDefinition {

View File

@ -15,7 +15,7 @@
#[cfg(all(test, unix, not(debug_assertions)))]
mod tests {
use account_utils::validator_definitions::{
SigningDefinition, ValidatorDefinition, ValidatorDefinitions,
SigningDefinition, ValidatorDefinition, ValidatorDefinitions, Web3SignerDefinition,
};
use eth2_keystore::KeystoreBuilder;
use eth2_network_config::Eth2NetworkConfig;
@ -376,13 +376,13 @@ mod tests {
graffiti: None,
suggested_fee_recipient: None,
description: String::default(),
signing_definition: SigningDefinition::Web3Signer {
signing_definition: SigningDefinition::Web3Signer(Web3SignerDefinition {
url: signer_rig.url.to_string(),
root_certificate_path: Some(root_certificate_path()),
request_timeout_ms: None,
client_identity_path: Some(client_identity_path()),
client_identity_password: Some(client_identity_password()),
},
}),
};
ValidatorStoreRig::new(vec![validator_definition], spec).await
};

View File

@ -7,7 +7,7 @@ mod tests;
use crate::ValidatorStore;
use account_utils::{
mnemonic_from_phrase,
validator_definitions::{SigningDefinition, ValidatorDefinition},
validator_definitions::{SigningDefinition, ValidatorDefinition, Web3SignerDefinition},
};
pub use api_secret::ApiSecret;
use create_validator::{create_validators_mnemonic, create_validators_web3signer};
@ -470,13 +470,16 @@ pub fn serve<T: 'static + SlotClock + Clone, E: EthSpec>(
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,
client_identity_path: web3signer.client_identity_path,
client_identity_password: web3signer.client_identity_password,
},
signing_definition: SigningDefinition::Web3Signer(
Web3SignerDefinition {
url: web3signer.url,
root_certificate_path: web3signer.root_certificate_path,
request_timeout_ms: web3signer.request_timeout_ms,
client_identity_path: web3signer.client_identity_path,
client_identity_password: web3signer
.client_identity_password,
},
),
})
.collect();
handle.block_on(create_validators_web3signer(

View File

@ -1,6 +1,8 @@
//! Implementation of the standard remotekey management API.
use crate::{initialized_validators::Error, InitializedValidators, ValidatorStore};
use account_utils::validator_definitions::{SigningDefinition, ValidatorDefinition};
use account_utils::validator_definitions::{
SigningDefinition, ValidatorDefinition, Web3SignerDefinition,
};
use eth2::lighthouse_vc::std_types::{
DeleteRemotekeyStatus, DeleteRemotekeysRequest, DeleteRemotekeysResponse,
ImportRemotekeyStatus, ImportRemotekeysRequest, ImportRemotekeysResponse,
@ -31,11 +33,13 @@ pub fn list<T: SlotClock + 'static, E: EthSpec>(
match &def.signing_definition {
SigningDefinition::LocalKeystore { .. } => None,
SigningDefinition::Web3Signer { url, .. } => Some(SingleListRemotekeysResponse {
pubkey: validating_pubkey,
url: url.clone(),
readonly: false,
}),
SigningDefinition::Web3Signer(Web3SignerDefinition { url, .. }) => {
Some(SingleListRemotekeysResponse {
pubkey: validating_pubkey,
url: url.clone(),
readonly: false,
})
}
}
})
.collect::<Vec<_>>();
@ -120,13 +124,13 @@ fn import_single_remotekey<T: SlotClock + 'static, E: EthSpec>(
graffiti: None,
suggested_fee_recipient: None,
description: String::from("Added by remotekey API"),
signing_definition: SigningDefinition::Web3Signer {
signing_definition: SigningDefinition::Web3Signer(Web3SignerDefinition {
url,
root_certificate_path: None,
request_timeout_ms: None,
client_identity_path: None,
client_identity_password: None,
},
}),
};
handle
.block_on(validator_store.add_validator(web3signer_validator))

View File

@ -10,7 +10,8 @@ use crate::signing_method::SigningMethod;
use account_utils::{
read_password, read_password_from_user,
validator_definitions::{
self, SigningDefinition, ValidatorDefinition, ValidatorDefinitions, CONFIG_FILENAME,
self, SigningDefinition, ValidatorDefinition, ValidatorDefinitions, Web3SignerDefinition,
CONFIG_FILENAME,
},
ZeroizeString,
};
@ -155,6 +156,7 @@ impl InitializedValidator {
def: ValidatorDefinition,
key_cache: &mut KeyCache,
key_stores: &mut HashMap<PathBuf, Keystore>,
web3_signer_client_map: &mut Option<HashMap<Web3SignerDefinition, Client>>,
) -> Result<Self, Error> {
if !def.enabled {
return Err(Error::UnableToInitializeDisabledValidator);
@ -239,46 +241,45 @@ impl InitializedValidator {
voting_keypair: Arc::new(voting_keypair),
}
}
SigningDefinition::Web3Signer {
url,
root_certificate_path,
request_timeout_ms,
client_identity_path,
client_identity_password,
} => {
let signing_url = build_web3_signer_url(&url, &def.voting_public_key)
SigningDefinition::Web3Signer(web3_signer) => {
let signing_url = build_web3_signer_url(&web3_signer.url, &def.voting_public_key)
.map_err(|e| Error::InvalidWeb3SignerUrl(e.to_string()))?;
let request_timeout = request_timeout_ms
let request_timeout = web3_signer
.request_timeout_ms
.map(Duration::from_millis)
.unwrap_or(DEFAULT_REMOTE_SIGNER_REQUEST_TIMEOUT);
let builder = Client::builder().timeout(request_timeout);
let builder = if let Some(path) = root_certificate_path {
let certificate = load_pem_certificate(path)?;
builder.add_root_certificate(certificate)
} else {
builder
};
let builder = if let Some(path) = client_identity_path {
let identity = load_pkcs12_identity(
path,
&client_identity_password
.ok_or(Error::MissingWeb3SignerClientIdentityPassword)?,
)?;
builder.identity(identity)
} else {
if client_identity_password.is_some() {
return Err(Error::MissingWeb3SignerClientIdentityCertificateFile);
// Check if a client has already been initialized for this remote signer url.
let http_client = if let Some(client_map) = web3_signer_client_map {
match client_map.get(&web3_signer) {
Some(client) => client.clone(),
None => {
let client = build_web3_signer_client(
web3_signer.root_certificate_path.clone(),
web3_signer.client_identity_path.clone(),
web3_signer.client_identity_password.clone(),
request_timeout,
)?;
client_map.insert(web3_signer, client.clone());
client
}
}
builder
} else {
// There are no clients in the map.
let mut new_web3_signer_client_map: HashMap<Web3SignerDefinition, Client> =
HashMap::new();
let client = build_web3_signer_client(
web3_signer.root_certificate_path.clone(),
web3_signer.client_identity_path.clone(),
web3_signer.client_identity_password.clone(),
request_timeout,
)?;
new_web3_signer_client_map.insert(web3_signer, client.clone());
*web3_signer_client_map = Some(new_web3_signer_client_map);
client
};
let http_client = builder
.build()
.map_err(Error::UnableToBuildWeb3SignerClient)?;
SigningMethod::Web3Signer {
signing_url,
http_client,
@ -332,6 +333,39 @@ fn build_web3_signer_url(base_url: &str, voting_public_key: &PublicKey) -> Resul
Url::parse(base_url)?.join(&format!("api/v1/eth2/sign/{}", voting_public_key))
}
fn build_web3_signer_client(
root_certificate_path: Option<PathBuf>,
client_identity_path: Option<PathBuf>,
client_identity_password: Option<String>,
request_timeout: Duration,
) -> Result<Client, Error> {
let builder = Client::builder().timeout(request_timeout);
let builder = if let Some(path) = root_certificate_path {
let certificate = load_pem_certificate(path)?;
builder.add_root_certificate(certificate)
} else {
builder
};
let builder = if let Some(path) = client_identity_path {
let identity = load_pkcs12_identity(
path,
&client_identity_password.ok_or(Error::MissingWeb3SignerClientIdentityPassword)?,
)?;
builder.identity(identity)
} else {
if client_identity_password.is_some() {
return Err(Error::MissingWeb3SignerClientIdentityCertificateFile);
}
builder
};
builder
.build()
.map_err(Error::UnableToBuildWeb3SignerClient)
}
/// Try to unlock `keystore` at `keystore_path` by prompting the user via `stdin`.
fn unlock_keystore_via_stdin_password(
keystore: &Keystore,
@ -382,6 +416,8 @@ pub struct InitializedValidators {
validators_dir: PathBuf,
/// The canonical set of validators.
validators: HashMap<PublicKeyBytes, InitializedValidator>,
/// The clients used for communications with a remote signer.
web3_signer_client_map: Option<HashMap<Web3SignerDefinition, Client>>,
/// For logging via `slog`.
log: Logger,
}
@ -397,6 +433,7 @@ impl InitializedValidators {
validators_dir,
definitions,
validators: HashMap::default(),
web3_signer_client_map: None,
log,
};
this.update_validators().await?;
@ -826,6 +863,7 @@ impl InitializedValidators {
def.clone(),
&mut key_cache,
&mut key_stores,
&mut None,
)
.await
{
@ -870,11 +908,12 @@ impl InitializedValidators {
}
}
}
SigningDefinition::Web3Signer { .. } => {
SigningDefinition::Web3Signer(Web3SignerDefinition { .. }) => {
match InitializedValidator::from_definition(
def.clone(),
&mut key_cache,
&mut key_stores,
&mut self.web3_signer_client_map,
)
.await
{