gr

jayantk · jayantk · commit 1895c053c2dc · 2025-01-12T18:14:53.000-08:00
diff --git a/apps/fortuna/src/api.rs b/apps/fortuna/src/api.rs
@@ -92,9 +92,6 @@ pub struct BlockchainState {
     /// The BlockStatus of the block that is considered to be confirmed on the blockchain.
     /// For eg., Finalized, Safe
     pub confirmed_block_status: BlockStatus,
-
-    /// The maximum percentage that the gas limit can be multiplied by during backoff retries
-    pub backoff_gas_multiplier_cap_pct: u64,
 }
 
 pub enum RestError {
@@ -215,7 +212,6 @@ mod test {
             provider_address: PROVIDER,
             reveal_delay_blocks: 1,
             confirmed_block_status: BlockStatus::Latest,
-            backoff_gas_multiplier_cap_pct: 500,
         };
 
         let metrics_registry = Arc::new(RwLock::new(Registry::default()));
@@ -228,8 +224,7 @@ mod test {
             contract: avax_read.clone(),
             provider_address: PROVIDER,
             reveal_delay_blocks: 2,
-            confirmed_block_status: BlockStatus::Latest,
-            backoff_gas_multiplier_cap_pct: 500,
+            confirmed_block_status: BlockStatus::Latest
         };
 
         let mut chains = HashMap::new();
diff --git a/apps/fortuna/src/command/run.rs b/apps/fortuna/src/command/run.rs
@@ -295,8 +295,7 @@ async fn setup_chain_state(
         contract,
         provider_address: *provider,
         reveal_delay_blocks: chain_config.reveal_delay_blocks,
-        confirmed_block_status: chain_config.confirmed_block_status,
-        backoff_gas_multiplier_cap_pct: chain_config.backoff_gas_multiplier_cap_pct,
+        confirmed_block_status: chain_config.confirmed_block_status
     };
     Ok(state)
 }
diff --git a/apps/fortuna/src/config.rs b/apps/fortuna/src/config.rs
@@ -134,9 +134,8 @@ 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 escalation policy governs how the gas limit and fee are increased during backoff retries.
+    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,25 +169,79 @@ 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<u32>,
+}
+
+#[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 initial fee multiplier to apply to the fee.
+    #[serde(default = "default_initial_fee_multiplier_pct")]
+    pub initial_fee_multiplier_pct: u64,
+
+    /// The fee multiplier to apply to the fee during backoff retries.
+    /// 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,
+}
+
+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)        
+    }
 
-    /// 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,
+    pub fn get_fee_multiplier_pct(&self, num_retries: u64) -> u64 {
+        self.apply_escalation_policy(num_retries, self.initial_fee_multiplier_pct, self.fee_multiplier_pct, self.fee_multiplier_cap_pct)
+    }
 
-    /// The maximum percentage that the gas limit can be multiplied by during backoff retries
-    /// For example, 500 means the gas limit can be increased up to 500% of the original estimate
-    #[serde(default = "default_backoff_gas_multiplier_cap_pct")]
-    pub backoff_gas_multiplier_cap_pct: u64,
+    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)
+    }
 }
 
-fn default_backoff_gas_multiplier_cap_pct() -> u64 {
-    500  // Default to 500% (5x) maximum gas multiplier
+fn default_initial_gas_multiplier_pct() -> u64 {
+    125
 }
 
-fn default_backoff_gas_multiplier_pct() -> u64 {
+fn default_gas_multiplier_pct() -> u64 {
     100
 }
 
+fn default_gas_multiplier_cap_pct() -> u64 {
+    500
+}
+
+fn default_initial_fee_multiplier_pct() -> u64 {
+    100
+}
+
+fn default_fee_multiplier_pct() -> u64 {
+    110
+}
+
+fn default_fee_multiplier_cap_pct() -> u64 {
+    200
+}
+
 /// 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
diff --git a/apps/fortuna/src/keeper.rs b/apps/fortuna/src/keeper.rs
@@ -9,6 +9,7 @@ use {
             traced_client::{RpcMetrics, TracedClient},
         },
         config::EthereumConfig,
+        config::EscalationPolicyConfig,
     },
     anyhow::{anyhow, Result},
     backoff::ExponentialBackoff,
@@ -248,10 +249,6 @@ pub async fn run_keeper_threads(
     let latest_safe_block = get_latest_safe_block(&chain_state).in_current_span().await;
     tracing::info!("latest safe block: {}", &latest_safe_block);
 
-    // Update BlockchainState with the gas multiplier cap from config
-    let mut chain_state = chain_state;
-    chain_state.backoff_gas_multiplier_cap_pct = chain_eth_config.backoff_gas_multiplier_cap_pct;
-
     let contract = Arc::new(
         InstrumentedSignablePythContract::from_config(
             &chain_eth_config,
@@ -276,7 +273,7 @@ pub async fn run_keeper_threads(
             },
             contract.clone(),
             gas_limit,
-            chain_eth_config.backoff_gas_multiplier_pct,
+            chain_eth_config.escalation_policy,
             chain_state.clone(),
             metrics.clone(),
             fulfilled_requests_cache.clone(),
@@ -302,7 +299,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,
             metrics.clone(),
             fulfilled_requests_cache.clone(),
         )
@@ -327,9 +324,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,
@@ -398,7 +400,7 @@ pub async fn process_event_with_backoff(
     chain_state: BlockchainState,
     contract: Arc<InstrumentedSignablePythContract>,
     gas_limit: U256,
-    backoff_gas_multiplier_pct: u64,
+    escalation_policy: EscalationPolicyConfig,
     metrics: Arc<KeeperMetrics>,
 ) {
     let start_time = std::time::Instant::now();
@@ -414,36 +416,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
             );
-            // Calculate new multiplier with backoff, capped by backoff_gas_multiplier_cap_pct
-            let new_multiplier = multiplier.saturating_mul(backoff_gas_multiplier_pct) / 100;
-            current_multiplier.store(
-                std::cmp::min(new_multiplier, chain_state.backoff_gas_multiplier_cap_pct),
-                std::sync::atomic::Ordering::Relaxed,
-            );
+            num_retries.store(retry_number + 1, std::sync::atomic::Ordering::Relaxed);
         },
     )
     .await;
@@ -501,8 +502,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<KeeperMetrics>,
 ) -> Result<(), backoff::Error<anyhow::Error>> {
     // ignore requests that are not for the configured provider
@@ -546,9 +548,7 @@ 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;
-    // Apply the configurable cap from backoff_gas_multiplier_cap_pct
-    let max_gas_estimate = gas_limit.saturating_mul(chain_config.backoff_gas_multiplier_cap_pct.into()) / 100;
-    let gas_estimate = gas_estimate.min(max_gas_estimate);
+
 
     let contract_call = contract
         .reveal_with_callback(
@@ -561,13 +561,19 @@ 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)
         .await
         .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
@@ -883,7 +889,7 @@ pub async fn process_new_blocks(
     mut rx: mpsc::Receiver<BlockRange>,
     contract: Arc<InstrumentedSignablePythContract>,
     gas_limit: U256,
-    backoff_gas_multiplier_pct: u64,
+    escalation_policy: EscalationPolicyConfig,
     metrics: Arc<KeeperMetrics>,
     fulfilled_requests_cache: Arc<RwLock<HashSet<u64>>>,
 ) {
@@ -894,7 +900,7 @@ pub async fn process_new_blocks(
                 block_range,
                 Arc::clone(&contract),
                 gas_limit,
-                backoff_gas_multiplier_pct,
+                escalation_policy,
                 chain_state.clone(),
                 metrics.clone(),
                 fulfilled_requests_cache.clone(),
@@ -911,7 +917,7 @@ pub async fn process_backlog(
     backlog_range: BlockRange,
     contract: Arc<InstrumentedSignablePythContract>,
     gas_limit: U256,
-    backoff_gas_multiplier_pct: u64,
+    escalation_policy: EscalationPolicyConfig,
     chain_state: BlockchainState,
     metrics: Arc<KeeperMetrics>,
     fulfilled_requests_cache: Arc<RwLock<HashSet<u64>>>,
@@ -921,7 +927,7 @@ pub async fn process_backlog(
         backlog_range,
         contract,
         gas_limit,
-        backoff_gas_multiplier_pct,
+        escalation_policy,
         chain_state,
         metrics,
         fulfilled_requests_cache,
