From 3ea2ac734e833c9e1429f502911b13a8c4582ee4 Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 13 Jan 2025 09:14:43 -0800 Subject: [PATCH] feat(fortuna): configurable escalation policy for transactions. (#2244) * feat: add backoff_gas_multiplier_cap_pct to fortuna config Co-Authored-By: Jayant Krishnamurthy * feat: replace hardcoded gas ceiling with configurable cap in process_event Co-Authored-By: Jayant Krishnamurthy * feat: apply gas multiplier cap in process_event_with_backoff Co-Authored-By: Jayant Krishnamurthy * fix: initialize backoff_gas_multiplier_cap_pct in BlockchainState Co-Authored-By: Jayant Krishnamurthy * gr * make this sane * config defaults --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Jayant Krishnamurthy Co-authored-by: Jayant Krishnamurthy --- apps/fortuna/Cargo.lock | 2 +- apps/fortuna/Cargo.toml | 2 +- apps/fortuna/config.sample.yaml | 27 ++++++-- apps/fortuna/src/config.rs | 112 ++++++++++++++++++++++++++++---- apps/fortuna/src/keeper.rs | 73 +++++++++++++-------- 5 files changed, 167 insertions(+), 49 deletions(-) diff --git a/apps/fortuna/Cargo.lock b/apps/fortuna/Cargo.lock index d9add40278..02e0ad04a4 100644 --- a/apps/fortuna/Cargo.lock +++ b/apps/fortuna/Cargo.lock @@ -1503,7 +1503,7 @@ dependencies = [ [[package]] name = "fortuna" -version = "6.8.1" +version = "7.0.0" dependencies = [ "anyhow", "axum", diff --git a/apps/fortuna/Cargo.toml b/apps/fortuna/Cargo.toml index ba0577b4e9..454156d64c 100644 --- a/apps/fortuna/Cargo.toml +++ b/apps/fortuna/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "fortuna" -version = "6.8.1" +version = "7.0.0" edition = "2021" [dependencies] diff --git a/apps/fortuna/config.sample.yaml b/apps/fortuna/config.sample.yaml index 3d5911d5ff..80cc0af3f7 100644 --- a/apps/fortuna/config.sample.yaml +++ b/apps/fortuna/config.sample.yaml @@ -6,19 +6,32 @@ chains: # Keeper configuration for the chain reveal_delay_blocks: 0 gas_limit: 500000 - # Increase the transaction gas limit by 10% each time the callback fails - # defaults to 100 (i.e., don't change the gas limit) if not specified. - backoff_gas_multiplier_pct: 110 + + # Multiplier for the priority fee estimate, as a percentage (i.e., 100 = no change). + # Defaults to 100 if the field is omitted. + priority_fee_multiplier_pct: 100 + + escalation_policy: + # Pad the first callback transaction's gas estimate by 25%, + # then multiply each successive callback transaction's gas estimate by 10% until the cap is reached. + # All numbers are expressed as percentages where 100 = no change. + initial_gas_multiplier_pct: 125 + gas_multiplier_pct: 110 + gas_multiplier_cap_pct: 600 + + # Multiply successive callback transaction's fees by 10% until the cap is reached. + # All numbers are expressed as percentages where 100 = no change. + # (See also priority_fee_multiplier_pct above to generically adjust the priority fee estimates for the chain -- + # adjusting that parameter will influence the fee of the first transaction, in addition to other things) + fee_multiplier_pct: 110 + fee_multiplier_cap_pct: 200 + min_keeper_balance: 100000000000000000 # Provider configuration # How much to charge in fees fee: 1500000000000000 - # Multiplier for the priority fee estimate, as a percentage (i.e., 100 = no change). - # Defaults to 100 if the field is omitted. - priority_fee_multiplier_pct: 100 - # Configuration for dynamic fees under high gas prices. The keeper will set # on-chain fees to make between [min_profit_pct, max_profit_pct] of the max callback # cost in profit per transaction. diff --git a/apps/fortuna/src/config.rs b/apps/fortuna/src/config.rs index 1ffd47a602..0687d7846b 100644 --- a/apps/fortuna/src/config.rs +++ b/apps/fortuna/src/config.rs @@ -134,9 +134,13 @@ pub struct EthereumConfig { /// The gas limit to use for entropy callback transactions. pub gas_limit: u64, - /// The percentage multiplier to apply to the gas limit for each backoff. - #[serde(default = "default_backoff_gas_multiplier_pct")] - pub backoff_gas_multiplier_pct: u64, + /// 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, + + /// 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 the callback. @@ -170,16 +174,104 @@ pub struct EthereumConfig { /// Maximum number of hashes to record in a request. /// This should be set according to the maximum gas limit the provider supports for callbacks. pub max_num_hashes: Option, - - /// The percentage multiplier to apply to the priority fee (100 = no change, e.g. 150 = 150% of base fee) - #[serde(default = "default_priority_fee_multiplier_pct")] - pub priority_fee_multiplier_pct: u64, } -fn default_backoff_gas_multiplier_pct() -> u64 { +fn default_priority_fee_multiplier_pct() -> u64 { 100 } +#[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] +pub struct EscalationPolicyConfig { + /// The initial gas multiplier to apply to the gas limit. + #[serde(default = "default_initial_gas_multiplier_pct")] + pub initial_gas_multiplier_pct: u64, + + /// The gas multiplier to apply to the gas limit 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, + /// The maximum gas multiplier to apply to the gas limit during backoff retries. + #[serde(default = "default_gas_multiplier_cap_pct")] + pub gas_multiplier_cap_pct: u64, + + /// 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, + #[serde(default = "default_fee_multiplier_cap_pct")] + pub fee_multiplier_cap_pct: u64, +} + +fn default_initial_gas_multiplier_pct() -> u64 { + 125 +} + +fn default_gas_multiplier_pct() -> u64 { + 110 +} + +fn default_gas_multiplier_cap_pct() -> u64 { + 600 +} + +fn default_fee_multiplier_pct() -> u64 { + 110 +} + +fn default_fee_multiplier_cap_pct() -> u64 { + 200 +} + +impl Default for EscalationPolicyConfig { + fn default() -> Self { + Self { + initial_gas_multiplier_pct: default_initial_gas_multiplier_pct(), + gas_multiplier_pct: default_gas_multiplier_pct(), + gas_multiplier_cap_pct: default_gas_multiplier_cap_pct(), + fee_multiplier_pct: default_fee_multiplier_pct(), + fee_multiplier_cap_pct: default_fee_multiplier_cap_pct(), + } + } +} + +impl EscalationPolicyConfig { + pub fn get_gas_multiplier_pct(&self, num_retries: u64) -> u64 { + self.apply_escalation_policy( + num_retries, + self.initial_gas_multiplier_pct, + self.gas_multiplier_pct, + self.gas_multiplier_cap_pct, + ) + } + + pub fn get_fee_multiplier_pct(&self, num_retries: u64) -> u64 { + self.apply_escalation_policy( + num_retries, + 100, + self.fee_multiplier_pct, + self.fee_multiplier_cap_pct, + ) + } + + fn apply_escalation_policy( + &self, + num_retries: u64, + initial: u64, + multiplier: u64, + cap: u64, + ) -> u64 { + let mut current = initial; + let mut i = 0; + while i < num_retries && current < cap { + current = current.saturating_mul(multiplier) / 100; + i += 1; + } + + current.min(cap) + } +} + /// A commitment that the provider used to generate random numbers at some point in the past. /// These historical commitments need to be stored in the configuration to support transition points where /// the commitment changes. In theory, this information is stored on the blockchain, but unfortunately it @@ -227,10 +319,6 @@ fn default_chain_sample_interval() -> u64 { 1 } -fn default_priority_fee_multiplier_pct() -> u64 { - 100 -} - /// Configuration values for the keeper service that are shared across chains. #[derive(Clone, Debug, serde::Serialize, serde::Deserialize)] pub struct KeeperConfig { diff --git a/apps/fortuna/src/keeper.rs b/apps/fortuna/src/keeper.rs index 2c47dd9fa4..27aa0c47d6 100644 --- a/apps/fortuna/src/keeper.rs +++ b/apps/fortuna/src/keeper.rs @@ -8,6 +8,7 @@ use { reader::{BlockNumber, RequestedWithCallbackEvent}, traced_client::{RpcMetrics, TracedClient}, }, + config::EscalationPolicyConfig, config::EthereumConfig, }, anyhow::{anyhow, Result}, @@ -55,8 +56,6 @@ const UPDATE_COMMITMENTS_INTERVAL: Duration = Duration::from_secs(30); const UPDATE_COMMITMENTS_THRESHOLD_FACTOR: f64 = 0.95; /// Rety last N blocks const RETRY_PREVIOUS_BLOCKS: u64 = 100; -/// By default, we scale the gas estimate by 25% when submitting the tx. -const DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT: u64 = 125; #[derive(Clone, Debug, Hash, PartialEq, Eq, EncodeLabelSet)] pub struct AccountLabel { @@ -272,7 +271,7 @@ pub async fn run_keeper_threads( }, contract.clone(), gas_limit, - chain_eth_config.backoff_gas_multiplier_pct, + chain_eth_config.escalation_policy.clone(), chain_state.clone(), metrics.clone(), fulfilled_requests_cache.clone(), @@ -298,7 +297,7 @@ pub async fn run_keeper_threads( rx, Arc::clone(&contract), gas_limit, - chain_eth_config.backoff_gas_multiplier_pct, + chain_eth_config.escalation_policy.clone(), metrics.clone(), fulfilled_requests_cache.clone(), ) @@ -323,9 +322,14 @@ pub async fn run_keeper_threads( chain_state.provider_address, ADJUST_FEE_INTERVAL, chain_eth_config.legacy_tx, - // NOTE: we adjust fees based on the maximum gas that the keeper will submit a callback with. - // This number is *larger* than the configured gas limit, as we pad gas on transaction submission for reliability. - (chain_eth_config.gas_limit * DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT) / 100, + // NOTE: we are adjusting the fees based on the maximum configured gas for user transactions. + // However, the keeper will pad the gas limit for transactions (per the escalation policy) to ensure reliable submission. + // Consequently, fees can be adjusted such that transactions are still unprofitable. + // While we could scale up this value based on the padding, that ends up overcharging users as most transactions cost nowhere + // near the maximum gas limit. + // 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, chain_eth_config.min_profit_pct, chain_eth_config.target_profit_pct, chain_eth_config.max_profit_pct, @@ -394,7 +398,7 @@ pub async fn process_event_with_backoff( chain_state: BlockchainState, contract: Arc, gas_limit: U256, - backoff_gas_multiplier_pct: u64, + escalation_policy: EscalationPolicyConfig, metrics: Arc, ) { let start_time = std::time::Instant::now(); @@ -410,34 +414,35 @@ pub async fn process_event_with_backoff( ..Default::default() }; - let current_multiplier = Arc::new(AtomicU64::new(DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT)); + let num_retries = Arc::new(AtomicU64::new(0)); let success = backoff::future::retry_notify( backoff, || async { - let multiplier = current_multiplier.load(std::sync::atomic::Ordering::Relaxed); + let num_retries = num_retries.load(std::sync::atomic::Ordering::Relaxed); + + let gas_multiplier_pct = escalation_policy.get_gas_multiplier_pct(num_retries); + let fee_multiplier_pct = escalation_policy.get_fee_multiplier_pct(num_retries); process_event( &event, &chain_state, &contract, gas_limit, - multiplier, + gas_multiplier_pct, + fee_multiplier_pct, metrics.clone(), ) .await }, |e, dur| { - let multiplier = current_multiplier.load(std::sync::atomic::Ordering::Relaxed); + let retry_number = num_retries.load(std::sync::atomic::Ordering::Relaxed); tracing::error!( - "Error at duration {:?} with gas multiplier {}: {}", + "Error on retry {} at duration {:?}: {}", + retry_number, dur, - multiplier, e ); - current_multiplier.store( - multiplier.saturating_mul(backoff_gas_multiplier_pct) / 100, - std::sync::atomic::Ordering::Relaxed, - ); + num_retries.store(retry_number + 1, std::sync::atomic::Ordering::Relaxed); }, ) .await; @@ -495,8 +500,9 @@ pub async fn process_event( chain_config: &BlockchainState, contract: &InstrumentedSignablePythContract, gas_limit: U256, - // A value of 100 submits the tx with the same gas as the estimate. + // A value of 100 submits the tx with the same gas/fee as the estimate. gas_estimate_multiplier_pct: u64, + fee_estimate_multiplier_pct: u64, metrics: Arc, ) -> Result<(), backoff::Error> { // ignore requests that are not for the configured provider @@ -540,7 +546,6 @@ pub async fn process_event( // Pad the gas estimate after checking it against the simulation gas limit, ensuring that // the padded gas estimate doesn't exceed the maximum amount of gas we are willing to use. let gas_estimate = gas_estimate.saturating_mul(gas_estimate_multiplier_pct.into()) / 100; - let gas_estimate = gas_estimate.min((gas_limit * DEFAULT_GAS_ESTIMATE_MULTIPLIER_PCT) / 100); let contract_call = contract .reveal_with_callback( @@ -553,6 +558,7 @@ pub async fn process_event( let client = contract.client(); let mut transaction = contract_call.tx.clone(); + // manually fill the tx with the gas info, so we can log the details in case of error client .fill_transaction(&mut transaction, None) @@ -560,6 +566,17 @@ pub async fn process_event( .map_err(|e| { backoff::Error::transient(anyhow!("Error filling the reveal transaction: {:?}", e)) })?; + + // Apply the fee escalation policy. Note: the unwrap_or_default should never default as we have a gas oracle + // in the client that sets the gas price. + transaction.set_gas_price( + transaction + .gas_price() + .unwrap_or_default() + .saturating_mul(fee_estimate_multiplier_pct.into()) + / 100, + ); + let pending_tx = client .send_transaction(transaction.clone(), None) .await @@ -654,7 +671,7 @@ pub async fn process_block_range( block_range: BlockRange, contract: Arc, gas_limit: U256, - backoff_gas_multiplier_pct: u64, + escalation_policy: EscalationPolicyConfig, chain_state: api::BlockchainState, metrics: Arc, fulfilled_requests_cache: Arc>>, @@ -678,7 +695,7 @@ pub async fn process_block_range( }, contract.clone(), gas_limit, - backoff_gas_multiplier_pct, + escalation_policy.clone(), chain_state.clone(), metrics.clone(), fulfilled_requests_cache.clone(), @@ -701,7 +718,7 @@ pub async fn process_single_block_batch( block_range: BlockRange, contract: Arc, gas_limit: U256, - backoff_gas_multiplier_pct: u64, + escalation_policy: EscalationPolicyConfig, chain_state: api::BlockchainState, metrics: Arc, fulfilled_requests_cache: Arc>>, @@ -728,7 +745,7 @@ pub async fn process_single_block_batch( chain_state.clone(), contract.clone(), gas_limit, - backoff_gas_multiplier_pct, + escalation_policy.clone(), metrics.clone(), ) .in_current_span(), @@ -875,7 +892,7 @@ pub async fn process_new_blocks( mut rx: mpsc::Receiver, contract: Arc, gas_limit: U256, - backoff_gas_multiplier_pct: u64, + escalation_policy: EscalationPolicyConfig, metrics: Arc, fulfilled_requests_cache: Arc>>, ) { @@ -886,7 +903,7 @@ pub async fn process_new_blocks( block_range, Arc::clone(&contract), gas_limit, - backoff_gas_multiplier_pct, + escalation_policy.clone(), chain_state.clone(), metrics.clone(), fulfilled_requests_cache.clone(), @@ -903,7 +920,7 @@ pub async fn process_backlog( backlog_range: BlockRange, contract: Arc, gas_limit: U256, - backoff_gas_multiplier_pct: u64, + escalation_policy: EscalationPolicyConfig, chain_state: BlockchainState, metrics: Arc, fulfilled_requests_cache: Arc>>, @@ -913,7 +930,7 @@ pub async fn process_backlog( backlog_range, contract, gas_limit, - backoff_gas_multiplier_pct, + escalation_policy, chain_state, metrics, fulfilled_requests_cache,