fix: do not leak eth specific in vm-agnostic traits

hyperlane-xyz · Jan 16, 2025 · bef103b · bef103b
1 parent 02c74ee
commit bef103b
Show file tree

Hide file tree

Showing 9 changed files with 24 additions and 34 deletions.
diff --git a/rust/main/agents/relayer/src/msg/pending_message.rs b/rust/main/agents/relayer/src/msg/pending_message.rs
@@ -284,7 +284,7 @@ impl PendingOperation for PendingMessage {
         let tx_cost_estimate = match self
             .ctx
             .destination_mailbox
-            .process_estimate_costs(&self.message, &metadata, false)
+            .process_estimate_costs(&self.message, &metadata)
             .await
         {
             Ok(tx_cost_estimate) => tx_cost_estimate,
@@ -355,7 +355,7 @@ impl PendingOperation for PendingMessage {
             if self
                 .ctx
                 .destination_mailbox
-                .process_estimate_costs(&self.message, metadata, true)
+                .process_estimate_costs(&self.message, metadata)
                 .await
                 .is_err()
             {

diff --git a/rust/main/chains/hyperlane-cosmos/src/mailbox/contract.rs b/rust/main/chains/hyperlane-cosmos/src/mailbox/contract.rs
@@ -183,7 +183,6 @@ impl Mailbox for CosmosMailbox {
         &self,
         message: &HyperlaneMessage,
         metadata: &[u8],
-        _apply_gas_overrides: bool,
     ) -> ChainResult<TxCostEstimate> {
         let process_message = ProcessMessageRequest {
             process: ProcessMessageRequestInner {

diff --git a/rust/main/chains/hyperlane-ethereum/src/contracts/mailbox.rs b/rust/main/chains/hyperlane-ethereum/src/contracts/mailbox.rs
@@ -7,7 +7,7 @@ use std::sync::Arc;
 
 use async_trait::async_trait;
 use derive_new::new;
-use ethers::abi::{AbiEncode, Detokenize};
+use ethers::abi::AbiEncode;
 use ethers::prelude::Middleware;
 use ethers_contract::builders::ContractCall;
 use ethers_contract::{Multicall, MulticallResult};
@@ -315,8 +315,8 @@ where
         &self,
         message: &HyperlaneMessage,
         metadata: &[u8],
-        apply_gas_overrides: bool,
         tx_gas_estimate: Option<U256>,
+        with_gas_estimate_buffer: bool,
     ) -> ChainResult<ContractCall<M, ()>> {
         let mut tx = self.contract.process(
             metadata.to_vec().into(),
@@ -326,22 +326,12 @@ where
             tx = tx.gas(gas_estimate);
         }
 
-        if apply_gas_overrides {
-            self.add_gas_overrides(tx).await
-        } else {
-            Ok(tx)
-        }
-    }
-
-    async fn add_gas_overrides<D: Detokenize>(
-        &self,
-        tx: ContractCall<M, D>,
-    ) -> ChainResult<ContractCall<M, D>> {
         fill_tx_gas_params(
             tx,
             self.provider.clone(),
             &self.conn.transaction_overrides.clone(),
             &self.domain,
+            with_gas_estimate_buffer,
         )
         .await
     }
@@ -437,6 +427,7 @@ impl<M: Middleware + 'static> SubmittableBatch<M> {
             self.provider,
             &self.transaction_overrides,
             &self.domain,
+            true,
         )
         .await?;
         let outcome = report_tx(call_with_gas_overrides).await?;
@@ -510,7 +501,7 @@ where
         tx_gas_limit: Option<U256>,
     ) -> ChainResult<TxOutcome> {
         let contract_call = self
-            .process_contract_call(message, metadata, true, tx_gas_limit)
+            .process_contract_call(message, metadata, tx_gas_limit, true)
             .await?;
         let receipt = report_tx(contract_call).await?;
         Ok(receipt.into())
@@ -534,8 +525,8 @@ where
                 self.process_contract_call(
                     &batch_item.data,
                     &batch_item.submission_data.metadata,
-                    true,
                     Some(batch_item.submission_data.gas_limit),
+                    true,
                 )
                 .await
             })
@@ -554,10 +545,12 @@ where
         &self,
         message: &HyperlaneMessage,
         metadata: &[u8],
-        apply_gas_overrides: bool,
     ) -> ChainResult<TxCostEstimate> {
+        // this function is used to get an accurate gas estimate for the transaction
+        // rather than a gas amount that will guarantee inclusion, so we use `false`
+        // for the `with_gas_estimate_buffer` arg in `process_contract_call`
         let contract_call = self
-            .process_contract_call(message, metadata, apply_gas_overrides, None)
+            .process_contract_call(message, metadata, None, false)
             .await?;
         let gas_limit = contract_call
             .tx
@@ -711,7 +704,7 @@ mod test {
         mock_provider.push(gas_limit).unwrap();
 
         let tx_cost_estimate = mailbox
-            .process_estimate_costs(&message, &metadata, true)
+            .process_estimate_costs(&message, &metadata)
             .await
             .unwrap();
 
@@ -761,7 +754,7 @@ mod test {
         mock_provider.push(gas_limit).unwrap();
 
         let tx_cost_estimate = mailbox
-            .process_estimate_costs(&message, &metadata, true)
+            .process_estimate_costs(&message, &metadata)
             .await
             .unwrap();
 

diff --git a/rust/main/chains/hyperlane-ethereum/src/contracts/validator_announce.rs b/rust/main/chains/hyperlane-ethereum/src/contracts/validator_announce.rs
@@ -97,6 +97,7 @@ where
             self.provider.clone(),
             &self.conn.transaction_overrides,
             &self.domain,
+            true,
         )
         .await
     }

diff --git a/rust/main/chains/hyperlane-ethereum/src/tx.rs b/rust/main/chains/hyperlane-ethereum/src/tx.rs
@@ -115,6 +115,7 @@ pub(crate) async fn fill_tx_gas_params<M, D>(
     provider: Arc<M>,
     transaction_overrides: &TransactionOverrides,
     domain: &HyperlaneDomain,
+    with_gas_limit_overrides: bool,
 ) -> ChainResult<ContractCall<M, D>>
 where
     M: Middleware + 'static,
@@ -126,12 +127,13 @@ where
         None => tx.estimate_gas().await?.into(),
     };
 
-    estimated_gas_limit = apply_gas_estimate_buffer(estimated_gas_limit, domain)?;
-    let gas_limit: U256 = if let Some(gas_limit) = transaction_overrides.gas_limit {
-        estimated_gas_limit.max(gas_limit)
-    } else {
-        estimated_gas_limit
-    };
+    if with_gas_limit_overrides {
+        estimated_gas_limit = apply_gas_estimate_buffer(estimated_gas_limit, domain)?;
+        if let Some(gas_limit) = transaction_overrides.gas_limit {
+            estimated_gas_limit = estimated_gas_limit.max(gas_limit)
+        }
+    }
+    let gas_limit = estimated_gas_limit;
 
     // Cap the gas limit to the block gas limit
     let latest_block = provider

diff --git a/rust/main/chains/hyperlane-fuel/src/mailbox.rs b/rust/main/chains/hyperlane-fuel/src/mailbox.rs
@@ -190,7 +190,6 @@ impl Mailbox for FuelMailbox {
         &self,
         message: &HyperlaneMessage,
         metadata: &[u8],
-        _apply_gas_overrides: bool,
     ) -> ChainResult<TxCostEstimate> {
         let call_res = self
             .contract

diff --git a/rust/main/chains/hyperlane-sealevel/src/mailbox.rs b/rust/main/chains/hyperlane-sealevel/src/mailbox.rs
@@ -657,7 +657,6 @@ impl Mailbox for SealevelMailbox {
         &self,
         _message: &HyperlaneMessage,
         _metadata: &[u8],
-        _apply_gas_overrides: bool,
     ) -> ChainResult<TxCostEstimate> {
         // TODO use correct data upon integrating IGP support
         Ok(TxCostEstimate {

diff --git a/rust/main/hyperlane-core/src/traits/mailbox.rs b/rust/main/hyperlane-core/src/traits/mailbox.rs
@@ -69,7 +69,6 @@ pub trait Mailbox: HyperlaneContract + Send + Sync + Debug {
         &self,
         message: &HyperlaneMessage,
         metadata: &[u8],
-        apply_gas_overrides: bool,
     ) -> ChainResult<TxCostEstimate>;
 
     /// Get the calldata for a transaction to process a message with a proof

diff --git a/rust/main/hyperlane-test/src/mocks/mailbox.rs b/rust/main/hyperlane-test/src/mocks/mailbox.rs
@@ -48,7 +48,6 @@ mock! {
             &self,
             message: &HyperlaneMessage,
             metadata: &[u8],
-            apply_gas_overrides: bool,
         ) -> ChainResult<TxCostEstimate> {}
 
         pub fn process_calldata(
@@ -103,9 +102,8 @@ impl Mailbox for MockMailboxContract {
         &self,
         message: &HyperlaneMessage,
         metadata: &[u8],
-        apply_gas_overrides: bool,
     ) -> ChainResult<TxCostEstimate> {
-        self.process_estimate_costs(message, metadata, apply_gas_overrides)
+        self.process_estimate_costs(message, metadata)
     }
 
     fn process_calldata(&self, message: &HyperlaneMessage, metadata: &[u8]) -> Vec<u8> {
-Original file line number
+Diff line change
@@ Expand Up / @@ -97,6 +97,7 @@ where @@
                 self.provider.clone(),
                 &self.conn.transaction_overrides,
                 &self.domain,
+                true,
             )
             .await
         }
@@ Expand Down @@