add execution-timeout-multiplier flag to optionally increase timeouts (#3631)

## Issue Addressed
Add flag to lengthen execution layer timeouts

Which issue # does this PR address?

#3607 

## Proposed Changes

Added execution-timeout-multiplier flag and a cli test to ensure the execution layer config has the multiplier set correctly.

Please list or describe the changes introduced by this PR.
Add execution_timeout_multiplier to the execution layer config as Option<u32> and pass the u32 to HttpJsonRpc.

## Additional Info
Not certain that this is the best way to implement it so I'd appreciate any feedback.

Please provide any additional information. For example, future considerations
or information useful for reviewers.
This commit is contained in:
GeemoCandama 2022-10-18 04:02:07 +00:00
parent edf23bb40e
commit c5cd0d9b3f
7 changed files with 91 additions and 24 deletions

View File

@ -290,6 +290,7 @@ pub struct Config {
pub max_blocks_per_update: Option<usize>,
/// If set to true, the eth1 caches are wiped clean when the eth1 service starts.
pub purge_cache: bool,
pub execution_timeout_multiplier: u32,
}
impl Config {
@ -347,6 +348,7 @@ impl Default for Config {
max_log_requests_per_update: Some(5_000),
max_blocks_per_update: Some(8_192),
purge_cache: false,
execution_timeout_multiplier: 1,
}
}
}
@ -361,11 +363,13 @@ pub fn endpoint_from_config(config: &Config) -> Result<HttpJsonRpc, String> {
} => {
let auth = Auth::new_with_path(jwt_path, jwt_id, jwt_version)
.map_err(|e| format!("Failed to initialize jwt auth: {:?}", e))?;
HttpJsonRpc::new_with_auth(endpoint, auth)
HttpJsonRpc::new_with_auth(endpoint, auth, Some(config.execution_timeout_multiplier))
.map_err(|e| format!("Failed to create eth1 json rpc client: {:?}", e))
}
Eth1Endpoint::NoAuth(endpoint) => {
HttpJsonRpc::new(endpoint, Some(config.execution_timeout_multiplier))
.map_err(|e| format!("Failed to create eth1 json rpc client: {:?}", e))
}
Eth1Endpoint::NoAuth(endpoint) => HttpJsonRpc::new(endpoint)
.map_err(|e| format!("Failed to create eth1 json rpc client: {:?}", e)),
}
}

View File

@ -493,7 +493,8 @@ mod deposit_tree {
let mut deposit_roots = vec![];
let mut deposit_counts = vec![];
let client = HttpJsonRpc::new(SensitiveUrl::parse(&eth1.endpoint()).unwrap()).unwrap();
let client =
HttpJsonRpc::new(SensitiveUrl::parse(&eth1.endpoint()).unwrap(), None).unwrap();
// Perform deposits to the smart contract, recording it's state along the way.
for deposit in &deposits {
@ -597,7 +598,8 @@ mod http {
.expect("should start eth1 environment");
let deposit_contract = &eth1.deposit_contract;
let web3 = eth1.web3();
let client = HttpJsonRpc::new(SensitiveUrl::parse(&eth1.endpoint()).unwrap()).unwrap();
let client =
HttpJsonRpc::new(SensitiveUrl::parse(&eth1.endpoint()).unwrap(), None).unwrap();
let block_number = get_block_number(&web3).await;
let logs = blocking_deposit_logs(&client, &eth1, 0..block_number).await;
@ -711,7 +713,8 @@ mod fast {
MainnetEthSpec::default_spec(),
)
.unwrap();
let client = HttpJsonRpc::new(SensitiveUrl::parse(&eth1.endpoint()).unwrap()).unwrap();
let client =
HttpJsonRpc::new(SensitiveUrl::parse(&eth1.endpoint()).unwrap(), None).unwrap();
let n = 10;
let deposits: Vec<_> = (0..n).map(|_| random_deposit_data()).collect();
for deposit in &deposits {

View File

@ -518,22 +518,32 @@ pub mod deposit_methods {
pub struct HttpJsonRpc {
pub client: Client,
pub url: SensitiveUrl,
pub execution_timeout_multiplier: u32,
auth: Option<Auth>,
}
impl HttpJsonRpc {
pub fn new(url: SensitiveUrl) -> Result<Self, Error> {
pub fn new(
url: SensitiveUrl,
execution_timeout_multiplier: Option<u32>,
) -> Result<Self, Error> {
Ok(Self {
client: Client::builder().build()?,
url,
execution_timeout_multiplier: execution_timeout_multiplier.unwrap_or(1),
auth: None,
})
}
pub fn new_with_auth(url: SensitiveUrl, auth: Auth) -> Result<Self, Error> {
pub fn new_with_auth(
url: SensitiveUrl,
auth: Auth,
execution_timeout_multiplier: Option<u32>,
) -> Result<Self, Error> {
Ok(Self {
client: Client::builder().build()?,
url,
execution_timeout_multiplier: execution_timeout_multiplier.unwrap_or(1),
auth: Some(auth),
})
}
@ -590,7 +600,11 @@ impl std::fmt::Display for HttpJsonRpc {
impl HttpJsonRpc {
pub async fn upcheck(&self) -> Result<(), Error> {
let result: serde_json::Value = self
.rpc_request(ETH_SYNCING, json!([]), ETH_SYNCING_TIMEOUT)
.rpc_request(
ETH_SYNCING,
json!([]),
ETH_SYNCING_TIMEOUT * self.execution_timeout_multiplier,
)
.await?;
/*
@ -614,7 +628,7 @@ impl HttpJsonRpc {
self.rpc_request(
ETH_GET_BLOCK_BY_NUMBER,
params,
ETH_GET_BLOCK_BY_NUMBER_TIMEOUT,
ETH_GET_BLOCK_BY_NUMBER_TIMEOUT * self.execution_timeout_multiplier,
)
.await
}
@ -625,8 +639,12 @@ impl HttpJsonRpc {
) -> Result<Option<ExecutionBlock>, Error> {
let params = json!([block_hash, RETURN_FULL_TRANSACTION_OBJECTS]);
self.rpc_request(ETH_GET_BLOCK_BY_HASH, params, ETH_GET_BLOCK_BY_HASH_TIMEOUT)
.await
self.rpc_request(
ETH_GET_BLOCK_BY_HASH,
params,
ETH_GET_BLOCK_BY_HASH_TIMEOUT * self.execution_timeout_multiplier,
)
.await
}
pub async fn get_block_by_hash_with_txns<T: EthSpec>(
@ -634,8 +652,12 @@ impl HttpJsonRpc {
block_hash: ExecutionBlockHash,
) -> Result<Option<ExecutionBlockWithTransactions<T>>, Error> {
let params = json!([block_hash, true]);
self.rpc_request(ETH_GET_BLOCK_BY_HASH, params, ETH_GET_BLOCK_BY_HASH_TIMEOUT)
.await
self.rpc_request(
ETH_GET_BLOCK_BY_HASH,
params,
ETH_GET_BLOCK_BY_HASH_TIMEOUT * self.execution_timeout_multiplier,
)
.await
}
pub async fn new_payload_v1<T: EthSpec>(
@ -645,7 +667,11 @@ impl HttpJsonRpc {
let params = json!([JsonExecutionPayloadV1::from(execution_payload)]);
let response: JsonPayloadStatusV1 = self
.rpc_request(ENGINE_NEW_PAYLOAD_V1, params, ENGINE_NEW_PAYLOAD_TIMEOUT)
.rpc_request(
ENGINE_NEW_PAYLOAD_V1,
params,
ENGINE_NEW_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier,
)
.await?;
Ok(response.into())
@ -658,7 +684,11 @@ impl HttpJsonRpc {
let params = json!([JsonPayloadIdRequest::from(payload_id)]);
let response: JsonExecutionPayloadV1<T> = self
.rpc_request(ENGINE_GET_PAYLOAD_V1, params, ENGINE_GET_PAYLOAD_TIMEOUT)
.rpc_request(
ENGINE_GET_PAYLOAD_V1,
params,
ENGINE_GET_PAYLOAD_TIMEOUT * self.execution_timeout_multiplier,
)
.await?;
Ok(response.into())
@ -678,7 +708,7 @@ impl HttpJsonRpc {
.rpc_request(
ENGINE_FORKCHOICE_UPDATED_V1,
params,
ENGINE_FORKCHOICE_UPDATED_TIMEOUT,
ENGINE_FORKCHOICE_UPDATED_TIMEOUT * self.execution_timeout_multiplier,
)
.await?;
@ -695,7 +725,8 @@ impl HttpJsonRpc {
.rpc_request(
ENGINE_EXCHANGE_TRANSITION_CONFIGURATION_V1,
params,
ENGINE_EXCHANGE_TRANSITION_CONFIGURATION_V1_TIMEOUT,
ENGINE_EXCHANGE_TRANSITION_CONFIGURATION_V1_TIMEOUT
* self.execution_timeout_multiplier,
)
.await?;
@ -732,13 +763,13 @@ mod test {
let echo_auth =
Auth::new(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap(), None, None);
(
Arc::new(HttpJsonRpc::new_with_auth(rpc_url, rpc_auth).unwrap()),
Arc::new(HttpJsonRpc::new_with_auth(echo_url, echo_auth).unwrap()),
Arc::new(HttpJsonRpc::new_with_auth(rpc_url, rpc_auth, None).unwrap()),
Arc::new(HttpJsonRpc::new_with_auth(echo_url, echo_auth, None).unwrap()),
)
} else {
(
Arc::new(HttpJsonRpc::new(rpc_url).unwrap()),
Arc::new(HttpJsonRpc::new(echo_url).unwrap()),
Arc::new(HttpJsonRpc::new(rpc_url, None).unwrap()),
Arc::new(HttpJsonRpc::new(echo_url, None).unwrap()),
)
};

View File

@ -159,6 +159,7 @@ pub struct Config {
pub default_datadir: PathBuf,
/// The minimum value of an external payload for it to be considered in a proposal.
pub builder_profit_threshold: u128,
pub execution_timeout_multiplier: Option<u32>,
}
/// Provides access to one execution engine and provides a neat interface for consumption by the
@ -180,6 +181,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
jwt_version,
default_datadir,
builder_profit_threshold,
execution_timeout_multiplier,
} = config;
if urls.len() > 1 {
@ -224,7 +226,8 @@ impl<T: EthSpec> ExecutionLayer<T> {
let engine: Engine = {
let auth = Auth::new(jwt_key, jwt_id, jwt_version);
debug!(log, "Loaded execution endpoint"; "endpoint" => %execution_url, "jwt_path" => ?secret_file.as_path());
let api = HttpJsonRpc::new_with_auth(execution_url, auth).map_err(Error::ApiError)?;
let api = HttpJsonRpc::new_with_auth(execution_url, auth, execution_timeout_multiplier)
.map_err(Error::ApiError)?;
Engine::new(api, executor.clone(), &log)
};

View File

@ -503,7 +503,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.requires("execution-endpoint")
.takes_value(true)
)
.arg(
Arg::with_name("execution-timeout-multiplier")
.long("execution-timeout-multiplier")
.value_name("NUM")
.help("Unsigned integer to multiply the default execution timeouts by.")
.default_value("1")
.takes_value(true)
)
/*
* Database purging and compaction.
*/

View File

@ -335,6 +335,9 @@ pub fn get_config<E: EthSpec>(
el_config.default_datadir = client_config.data_dir.clone();
el_config.builder_profit_threshold =
clap_utils::parse_required(cli_args, "builder-profit-threshold")?;
let execution_timeout_multiplier =
clap_utils::parse_required(cli_args, "execution-timeout-multiplier")?;
el_config.execution_timeout_multiplier = Some(execution_timeout_multiplier);
// If `--execution-endpoint` is provided, we should ignore any `--eth1-endpoints` values and
// use `--execution-endpoint` instead. Also, log a deprecation warning.

View File

@ -407,6 +407,22 @@ fn run_execution_jwt_secret_key_is_persisted() {
});
}
#[test]
fn execution_timeout_multiplier_flag() {
let dir = TempDir::new().expect("Unable to create temporary directory");
CommandLineTest::new()
.flag("execution-endpoint", Some("http://meow.cats"))
.flag(
"execution-jwt",
dir.path().join("jwt-file").as_os_str().to_str(),
)
.flag("execution-timeout-multiplier", Some("3"))
.run_with_zero_port()
.with_config(|config| {
let config = config.execution_layer.as_ref().unwrap();
assert_eq!(config.execution_timeout_multiplier, Some(3));
});
}
#[test]
fn merge_execution_endpoints_flag() {
run_merge_execution_endpoints_flag_test("execution-endpoints")
}