diff --git a/apps/fortuna/src/api/metrics.rs b/apps/fortuna/src/api/metrics.rs index 7977c58d96..1d2f8cd545 100644 --- a/apps/fortuna/src/api/metrics.rs +++ b/apps/fortuna/src/api/metrics.rs @@ -11,7 +11,10 @@ pub async fn metrics(State(state): State) -> impl IntoResp // Should not fail if the metrics are valid and there is memory available // to write to the buffer. - encode(&mut buffer, ®istry).unwrap(); + if let Err(e) = encode(&mut buffer, ®istry) { + tracing::error!("Failed to encode metrics: {}", e); + return axum::http::StatusCode::INTERNAL_SERVER_ERROR.into_response(); + } - buffer + buffer.into_response() } diff --git a/apps/fortuna/src/chain/ethereum.rs b/apps/fortuna/src/chain/ethereum.rs index fb60abe809..b337a1f904 100644 --- a/apps/fortuna/src/chain/ethereum.rs +++ b/apps/fortuna/src/chain/ethereum.rs @@ -82,7 +82,12 @@ impl SignablePythContractInner { .await? { // Extract Log from TransactionReceipt. - let l: RawLog = r.logs[0].clone().into(); + let l: RawLog = r + .logs + .first() + .ok_or_else(|| anyhow!("No logs in receipt"))? + .clone() + .into(); if let PythRandomEvents::Requested1Filter(r) = PythRandomEvents::decode_log(&l)? { Ok(r.request.sequence_number) } else { @@ -112,7 +117,12 @@ impl SignablePythContractInner { .await? { // Extract Log from TransactionReceipt. - let l: RawLog = r.logs[0].clone().into(); + let l: RawLog = r + .logs + .first() + .ok_or_else(|| anyhow!("No logs in receipt"))? + .clone() + .into(); if let PythRandomEvents::RequestedWithCallbackFilter(r) = PythRandomEvents::decode_log(&l)? { @@ -147,9 +157,13 @@ impl SignablePythContractInner { .await? .await? { - if let PythRandomEvents::Revealed1Filter(r) = - PythRandomEvents::decode_log(&r.logs[0].clone().into())? - { + if let PythRandomEvents::Revealed1Filter(r) = PythRandomEvents::decode_log( + &r.logs + .first() + .ok_or_else(|| anyhow!("No logs in receipt"))? + .clone() + .into(), + )? { Ok(r.random_number) } else { Err(anyhow!("No log with randomnumber")) @@ -165,8 +179,10 @@ impl SignablePythContractInner { provider: Provider, ) -> Result> { let chain_id = provider.get_chainid().await?; - let gas_oracle = - EthProviderOracle::new(provider.clone(), chain_config.priority_fee_multiplier_pct); + let gas_oracle = EthProviderOracle::new( + provider.clone(), + chain_config.priority_fee_multiplier_pct.value(), + ); let wallet__ = private_key .parse::()? .with_chain_id(chain_id.as_u64()); diff --git a/apps/fortuna/src/command/inspect.rs b/apps/fortuna/src/command/inspect.rs index bc92856c0c..ed58e579b9 100644 --- a/apps/fortuna/src/command/inspect.rs +++ b/apps/fortuna/src/command/inspect.rs @@ -37,9 +37,7 @@ async fn inspect_chain( let multicall_exists = rpc_provider .get_code(ethers::contract::MULTICALL_ADDRESS, None) .await - .expect("Failed to get code") - .len() - > 0; + .is_ok_and(|x| x.len() > 0); let contract = PythContract::from_config(chain_config)?; let entropy_provider = contract.get_default_provider().call().await?; @@ -97,9 +95,9 @@ async fn process_request( let block = rpc_provider .get_block(request.block_number) .await? - .expect("Block not found"); + .ok_or_else(|| anyhow::anyhow!("Block not found"))?; let datetime = chrono::DateTime::from_timestamp(block.timestamp.as_u64() as i64, 0) - .expect("Invalid timestamp"); + .ok_or_else(|| anyhow::anyhow!("Invalid timestamp"))?; println!( "{} sequence_number:{} block_number:{} requester:{}", datetime, request.sequence_number, request.block_number, request.requester diff --git a/apps/fortuna/src/command/run.rs b/apps/fortuna/src/command/run.rs index 3590c8e6d0..9ccd99a5b6 100644 --- a/apps/fortuna/src/command/run.rs +++ b/apps/fortuna/src/command/run.rs @@ -107,6 +107,9 @@ pub async fn run(opts: &RunOptions) -> Result<()> { let provider_config = config.provider.clone(); spawn(async move { loop { + // Note that there are conditions that can cause the chain setup to fail indefinitely + // (e.g., if the on-chain commitment doesn't match our expectations). However, if those + // conditions trigger, then we will see alerts because FIXME WHY? let setup_result = setup_chain_and_run_keeper( provider_config.clone(), &chain_id, @@ -135,7 +138,8 @@ pub async fn run(opts: &RunOptions) -> Result<()> { // Listen for Ctrl+C so we can set the exit flag and wait for a graceful shutdown. spawn(async move { tracing::info!("Registered shutdown signal handler..."); - tokio::signal::ctrl_c().await.unwrap(); + // No need to care about the result here -- if it returns, we shut down the service + let _r = tokio::signal::ctrl_c().await; tracing::info!("Shut down signal received, waiting for tasks..."); // no need to handle error here, as it will only occur when all the // receiver has been dropped and that's what we want to do @@ -220,7 +224,7 @@ async fn setup_chain_state( let last_prior_commitment = provider_commitments.last(); if last_prior_commitment.is_some() && last_prior_commitment - .unwrap() + .ok_or_else(|| anyhow!("No prior commitment found"))? .original_commitment_sequence_number >= provider_info.original_commitment_sequence_number { diff --git a/apps/fortuna/src/config.rs b/apps/fortuna/src/config.rs index 6ceeff35e4..368e70fc67 100644 --- a/apps/fortuna/src/config.rs +++ b/apps/fortuna/src/config.rs @@ -3,6 +3,7 @@ use { api::ChainId, chain::reader::{BlockNumber, BlockStatus}, eth_utils::utils::EscalationPolicy, + types::Percentage, }, anyhow::{anyhow, Result}, clap::{crate_authors, crate_description, crate_name, crate_version, Args, Parser}, @@ -86,10 +87,12 @@ impl Config { // Run correctness checks for the config and fail if there are any issues. for (chain_id, config) in config.chains.iter() { - if !(config.min_profit_pct <= config.target_profit_pct - && config.target_profit_pct <= config.max_profit_pct) + if !(config.min_profit_multiplier_pct.value() + <= config.target_profit_multiplier_pct.value() + && config.target_profit_multiplier_pct.value() + <= config.max_profit_multiplier_pct.value()) { - return Err(anyhow!("chain id {:?} configuration is invalid. Config must satisfy min_profit_pct <= target_profit_pct <= max_profit_pct.", chain_id)); + return Err(anyhow!("chain id {:?} configuration is invalid. Config must satisfy min_profit_multiplier_pct <= target_profit_multiplier_pct <= max_profit_multiplier_pct.", chain_id)); } } @@ -137,30 +140,33 @@ pub struct EthereumConfig { /// The percentage multiplier to apply to priority fee estimates (100 = no change, e.g. 150 = 150% of base fee) #[serde(default = "default_priority_fee_multiplier_pct")] - pub priority_fee_multiplier_pct: u64, + pub priority_fee_multiplier_pct: Percentage, /// The escalation policy governs how the gas limit and fee are increased during backoff retries. #[serde(default)] pub escalation_policy: EscalationPolicyConfig, /// The minimum percentage profit to earn as a function of the callback cost. - /// For example, 20 means a profit of 20% over the cost of a callback that uses the full gas limit. + /// A callback using the full gas limit has a cost of 100. + /// Thus, 120 means a profit of 20% over the cost of a callback that uses the full gas limit. /// The fee will be raised if the profit is less than this number. - /// The minimum value for this is -100. If set to < 0, it means the keeper may lose money on callbacks that use the full gas limit. - pub min_profit_pct: i64, + /// The minimum value for this is 0. If set to < 100, it means the keeper may lose money on callbacks that use the full gas limit. + pub min_profit_multiplier_pct: Percentage, /// The target percentage profit to earn as a function of the callback cost. - /// For example, 20 means a profit of 20% over the cost of a callback that uses the full gas limit. + /// A callback using the full gas limit has a cost of 100. + /// Thus, 120 means a profit of 20% over the cost of a callback that uses the full gas limit. /// The fee will be set to this target whenever it falls outside the min/max bounds. - /// The minimum value for this is -100. If set to < 0, it means the keeper may lose money on callbacks that use the full gas limit. - pub target_profit_pct: i64, + /// The minimum value for this is 0. If set to < 100, it means the keeper may lose money on callbacks that use the full gas limit. + pub target_profit_multiplier_pct: Percentage, /// The maximum percentage profit to earn as a function of the callback cost. - /// For example, 100 means a profit of 100% over the cost of a callback that uses the full gas limit. + /// A callback using the full gas limit has a cost of 100. + /// Thus, 200 means a profit of 100% over the cost of a callback that uses the full gas limit. /// The fee will be lowered if it is more profitable than specified here. - /// Must be larger than min_profit_pct. - /// The minimum value for this is -100. If set to < 0, it means the keeper may lose money on callbacks that use the full gas limit. - pub max_profit_pct: i64, + /// Must be larger than min_profit_multiplier_pct. + /// The minimum value for this is 0. If set to < 100, it means the keeper may lose money on callbacks that use the full gas limit. + pub max_profit_multiplier_pct: Percentage, /// Minimum wallet balance for the keeper. If the balance falls below this level, the keeper will /// withdraw fees from the contract to top up. This functionality requires the keeper to be the fee @@ -190,8 +196,8 @@ fn default_block_delays() -> Vec { vec![5] } -fn default_priority_fee_multiplier_pct() -> u64 { - 100 +fn default_priority_fee_multiplier_pct() -> Percentage { + Percentage::new(100) } fn default_backlog_range() -> u64 { @@ -203,51 +209,51 @@ pub struct EscalationPolicyConfig { // The keeper will perform the callback as long as the tx is within this percentage of the configured gas limit. // Default value is 110, meaning a 10% tolerance over the configured value. #[serde(default = "default_gas_limit_tolerance_pct")] - pub gas_limit_tolerance_pct: u64, + pub gas_limit_tolerance_pct: Percentage, /// The initial gas multiplier to apply to the tx gas estimate #[serde(default = "default_initial_gas_multiplier_pct")] - pub initial_gas_multiplier_pct: u64, + pub initial_gas_multiplier_pct: Percentage, /// The gas multiplier to apply to the tx gas estimate during backoff retries. /// The gas on each successive retry is multiplied by this value, with the maximum multiplier capped at `gas_multiplier_cap_pct`. #[serde(default = "default_gas_multiplier_pct")] - pub gas_multiplier_pct: u64, + pub gas_multiplier_pct: Percentage, /// The maximum gas multiplier to apply to the tx gas estimate during backoff retries. #[serde(default = "default_gas_multiplier_cap_pct")] - pub gas_multiplier_cap_pct: u64, + pub gas_multiplier_cap_pct: Percentage, /// The fee multiplier to apply to the fee during backoff retries. /// The initial fee is 100% of the estimate (which itself may be padded based on our chain configuration) /// The fee on each successive retry is multiplied by this value, with the maximum multiplier capped at `fee_multiplier_cap_pct`. #[serde(default = "default_fee_multiplier_pct")] - pub fee_multiplier_pct: u64, + pub fee_multiplier_pct: Percentage, #[serde(default = "default_fee_multiplier_cap_pct")] - pub fee_multiplier_cap_pct: u64, + pub fee_multiplier_cap_pct: Percentage, } -fn default_gas_limit_tolerance_pct() -> u64 { - 110 +fn default_gas_limit_tolerance_pct() -> Percentage { + Percentage::new(110) } -fn default_initial_gas_multiplier_pct() -> u64 { - 125 +fn default_initial_gas_multiplier_pct() -> Percentage { + Percentage::new(125) } -fn default_gas_multiplier_pct() -> u64 { - 110 +fn default_gas_multiplier_pct() -> Percentage { + Percentage::new(110) } -fn default_gas_multiplier_cap_pct() -> u64 { - 600 +fn default_gas_multiplier_cap_pct() -> Percentage { + Percentage::new(600) } -fn default_fee_multiplier_pct() -> u64 { - 110 +fn default_fee_multiplier_pct() -> Percentage { + Percentage::new(110) } -fn default_fee_multiplier_cap_pct() -> u64 { - 200 +fn default_fee_multiplier_cap_pct() -> Percentage { + Percentage::new(200) } impl Default for EscalationPolicyConfig { @@ -266,12 +272,12 @@ impl Default for EscalationPolicyConfig { impl EscalationPolicyConfig { pub fn to_policy(&self) -> EscalationPolicy { EscalationPolicy { - gas_limit_tolerance_pct: self.gas_limit_tolerance_pct, - initial_gas_multiplier_pct: self.initial_gas_multiplier_pct, - gas_multiplier_pct: self.gas_multiplier_pct, - gas_multiplier_cap_pct: self.gas_multiplier_cap_pct, - fee_multiplier_pct: self.fee_multiplier_pct, - fee_multiplier_cap_pct: self.fee_multiplier_cap_pct, + gas_limit_tolerance_pct: self.gas_limit_tolerance_pct.value(), + initial_gas_multiplier_pct: self.initial_gas_multiplier_pct.value(), + gas_multiplier_pct: self.gas_multiplier_pct.value(), + gas_multiplier_cap_pct: self.gas_multiplier_cap_pct.value(), + fee_multiplier_pct: self.fee_multiplier_pct.value(), + fee_multiplier_cap_pct: self.fee_multiplier_cap_pct.value(), } } } diff --git a/apps/fortuna/src/eth_utils/eth_gas_oracle.rs b/apps/fortuna/src/eth_utils/eth_gas_oracle.rs index 4f1d2c9da5..135d416498 100644 --- a/apps/fortuna/src/eth_utils/eth_gas_oracle.rs +++ b/apps/fortuna/src/eth_utils/eth_gas_oracle.rs @@ -108,14 +108,14 @@ pub fn eip1559_default_estimator(base_fee_per_gas: U256, rewards: Vec> fn estimate_priority_fee(rewards: Vec>) -> U256 { let mut rewards: Vec = rewards .iter() - .map(|r| r[0]) + .map(|r| r.first().copied().unwrap_or_default()) .filter(|r| *r > U256::zero()) .collect(); if rewards.is_empty() { return U256::zero(); } if rewards.len() == 1 { - return rewards[0]; + return rewards.first().copied().unwrap_or_default(); } // Sort the rewards as we will eventually take the median. rewards.sort(); @@ -128,33 +128,35 @@ fn estimate_priority_fee(rewards: Vec>) -> U256 { let mut percentage_change: Vec = rewards .iter() .zip(rewards_copy.iter()) - .map(|(a, b)| { - let a = I256::try_from(*a).expect("priority fee overflow"); - let b = I256::try_from(*b).expect("priority fee overflow"); - ((b - a) * 100) / a + .filter_map(|(a, b)| { + let a = I256::try_from(*a); + let b = I256::try_from(*b); + match (a, b) { + (Ok(a), Ok(b)) if a > I256::zero() => Some(((b - a) * 100) / a), + _ => None, + } }) .collect(); percentage_change.pop(); // Fetch the max of the percentage change, and that element's index. - let max_change = percentage_change.iter().max().unwrap(); + let max_change = percentage_change.iter().max(); let max_change_index = percentage_change .iter() - .position(|&c| c == *max_change) - .unwrap(); - - // If we encountered a big change in fees at a certain position, then consider only - // the values >= it. - let values = if *max_change >= EIP1559_FEE_ESTIMATION_THRESHOLD_MAX_CHANGE.into() - && (max_change_index >= (rewards.len() / 2)) - { - rewards[max_change_index..].to_vec() - } else { - rewards + .position(|&c| max_change.is_some_and(|m| c == *m)); + + let values = match (max_change, max_change_index) { + (Some(max_change), Some(max_change_index)) + if *max_change >= EIP1559_FEE_ESTIMATION_THRESHOLD_MAX_CHANGE.into() + && (max_change_index >= (rewards.len() / 2)) => + { + rewards.get(max_change_index..).unwrap_or(&rewards).to_vec() + } + _ => rewards, }; // Return the median. - values[values.len() / 2] + values.get(values.len() / 2).copied().unwrap_or_default() } fn base_fee_surged(base_fee_per_gas: U256) -> U256 { diff --git a/apps/fortuna/src/eth_utils/traced_client.rs b/apps/fortuna/src/eth_utils/traced_client.rs index 184c972eca..91e1645418 100644 --- a/apps/fortuna/src/eth_utils/traced_client.rs +++ b/apps/fortuna/src/eth_utils/traced_client.rs @@ -120,7 +120,7 @@ impl TracedClient { let client = Client::builder() .timeout(Duration::from_secs(10)) .build() - .expect("Failed to create HTTP client"); + .map_err(|e| anyhow::anyhow!("Failed to create HTTP client: {}", e))?; Ok(Provider::new(TracedClient { inner: Http::new_with_client(url, client), chain_id, diff --git a/apps/fortuna/src/keeper.rs b/apps/fortuna/src/keeper.rs index c585ff3908..f2490acb3c 100644 --- a/apps/fortuna/src/keeper.rs +++ b/apps/fortuna/src/keeper.rs @@ -143,13 +143,9 @@ pub async fn run_keeper_threads( // In the unlikely event that the keeper fees aren't sufficient, the solution to this is to configure the target // fee percentage to be higher on that specific chain. chain_eth_config.gas_limit, - // NOTE: unwrap() here so we panic early if someone configures these values below -100. - u64::try_from(100 + chain_eth_config.min_profit_pct) - .expect("min_profit_pct must be >= -100"), - u64::try_from(100 + chain_eth_config.target_profit_pct) - .expect("target_profit_pct must be >= -100"), - u64::try_from(100 + chain_eth_config.max_profit_pct) - .expect("max_profit_pct must be >= -100"), + chain_eth_config.min_profit_multiplier_pct.value(), + chain_eth_config.target_profit_multiplier_pct.value(), + chain_eth_config.max_profit_multiplier_pct.value(), chain_eth_config.fee, metrics.clone(), ) diff --git a/apps/fortuna/src/keeper/track.rs b/apps/fortuna/src/keeper/track.rs index c5e06981da..90657cd138 100644 --- a/apps/fortuna/src/keeper/track.rs +++ b/apps/fortuna/src/keeper/track.rs @@ -56,12 +56,16 @@ pub async fn track_block_timestamp_lag( Ok(block) => match block { Some(block) => { let block_timestamp = block.timestamp; - let server_timestamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_secs(); - let lag: i64 = (server_timestamp as i64) - (block_timestamp.as_u64() as i64); - lag + match SystemTime::now().duration_since(UNIX_EPOCH) { + Ok(duration) => { + let server_timestamp = duration.as_secs(); + (server_timestamp as i64) - (block_timestamp.as_u64() as i64) + } + Err(e) => { + tracing::error!("Failed to get system time: {}", e); + INF_LAG + } + } } None => { tracing::error!("Block is None"); diff --git a/apps/fortuna/src/lib.rs b/apps/fortuna/src/lib.rs index 7bc6f8566b..d1f4c54481 100644 --- a/apps/fortuna/src/lib.rs +++ b/apps/fortuna/src/lib.rs @@ -1,3 +1,17 @@ +#![cfg_attr( + not(test), + deny( + clippy::expect_used, + clippy::indexing_slicing, + clippy::panic, + clippy::panic_in_result_fn, + clippy::todo, + clippy::unimplemented, + clippy::unreachable, + clippy::unwrap_used + ) +)] + pub mod api; pub mod chain; pub mod command; @@ -5,3 +19,4 @@ pub mod config; pub mod eth_utils; pub mod keeper; pub mod state; +pub mod types; diff --git a/apps/fortuna/src/main.rs b/apps/fortuna/src/main.rs index 453a397bf6..20dd403f94 100644 --- a/apps/fortuna/src/main.rs +++ b/apps/fortuna/src/main.rs @@ -1,4 +1,17 @@ #![allow(clippy::just_underscores_and_digits)] +#![cfg_attr( + not(test), + deny( + clippy::expect_used, + clippy::indexing_slicing, + clippy::panic, + clippy::panic_in_result_fn, + clippy::todo, + clippy::unimplemented, + clippy::unreachable, + clippy::unwrap_used + ) +)] use { anyhow::Result, diff --git a/apps/fortuna/src/state.rs b/apps/fortuna/src/state.rs index 99839dcff6..d237bb4f92 100644 --- a/apps/fortuna/src/state.rs +++ b/apps/fortuna/src/state.rs @@ -94,7 +94,7 @@ impl PebbleHashChain { let sample_interval: usize = sample_interval.try_into()?; let hash_chain = spawn_blocking(move || Self::new(secret, chain_length, sample_interval)) .await - .expect("Failed to make hash chain"); + .map_err(|e| anyhow::anyhow!("Failed to make hash chain: {}", e))?; Ok(hash_chain) } @@ -106,7 +106,14 @@ impl PebbleHashChain { // actually at the *front* of the list. Thus, it's easier to compute indexes from the end of the list. let index_from_end_of_subsampled_list = ((self.len() - 1) - i) / self.sample_interval; let mut i_index = self.len() - 1 - index_from_end_of_subsampled_list * self.sample_interval; - let mut val = self.hash[self.hash.len() - 1 - index_from_end_of_subsampled_list]; + let mut val = *self + .hash + .get( + self.hash + .len() + .saturating_sub(1 + index_from_end_of_subsampled_list), + ) + .ok_or_else(|| anyhow::anyhow!("Index out of bounds in hash chain"))?; while i_index > i { val = Keccak256::digest(val).into(); @@ -148,7 +155,15 @@ impl HashChainState { .ok_or(anyhow::anyhow!( "Hash chain for the requested sequence number is not available." ))?; - self.hash_chains[chain_index].reveal_ith(sequence_number - self.offsets[chain_index]) + let chain = self + .hash_chains + .get(chain_index) + .ok_or_else(|| anyhow::anyhow!("Chain index out of bounds"))?; + let offset = self + .offsets + .get(chain_index) + .ok_or_else(|| anyhow::anyhow!("Offset index out of bounds"))?; + chain.reveal_ith(sequence_number - offset) } } diff --git a/apps/fortuna/src/types/mod.rs b/apps/fortuna/src/types/mod.rs new file mode 100644 index 0000000000..fa497c1e66 --- /dev/null +++ b/apps/fortuna/src/types/mod.rs @@ -0,0 +1,2 @@ +pub mod percentage; +pub use percentage::Percentage; diff --git a/apps/fortuna/src/types/percentage.rs b/apps/fortuna/src/types/percentage.rs new file mode 100644 index 0000000000..7d4c9c40a8 --- /dev/null +++ b/apps/fortuna/src/types/percentage.rs @@ -0,0 +1,40 @@ +use anyhow::Result; + +/// A type representing a percentage value that must be >= 0 +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub struct Percentage(u64); + +impl Percentage { + pub fn new(value: u64) -> Self { + Self(value) + } + + pub fn value(&self) -> u64 { + self.0 + } +} + +impl serde::Serialize for Percentage { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_u64(self.0) + } +} + +impl<'de> serde::Deserialize<'de> for Percentage { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let value = u64::deserialize(deserializer)?; + Ok(Self(value)) + } +} + +impl From for u64 { + fn from(p: Percentage) -> Self { + p.0 + } +}