From bef103b9068882df25a781c44164c55d93211d50 Mon Sep 17 00:00:00 2001 From: Daniel Savu <23065004+daniel-savu@users.noreply.github.com> Date: Thu, 16 Jan 2025 17:01:57 +0000 Subject: [PATCH] fix: do not leak eth specific in vm-agnostic traits --- .../agents/relayer/src/msg/pending_message.rs | 4 +-- .../hyperlane-cosmos/src/mailbox/contract.rs | 1 - .../src/contracts/mailbox.rs | 31 +++++++------------ .../src/contracts/validator_announce.rs | 1 + rust/main/chains/hyperlane-ethereum/src/tx.rs | 14 +++++---- .../main/chains/hyperlane-fuel/src/mailbox.rs | 1 - .../chains/hyperlane-sealevel/src/mailbox.rs | 1 - .../main/hyperlane-core/src/traits/mailbox.rs | 1 - rust/main/hyperlane-test/src/mocks/mailbox.rs | 4 +-- 9 files changed, 24 insertions(+), 34 deletions(-) diff --git a/rust/main/agents/relayer/src/msg/pending_message.rs b/rust/main/agents/relayer/src/msg/pending_message.rs index d6c2dd708e..d63b7b03f3 100644 --- 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 index c361c0e685..5a998aac5a 100644 --- 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 { 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 index 5b504a2f82..242c5d183e 100644 --- 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, + with_gas_estimate_buffer: bool, ) -> ChainResult> { 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( - &self, - tx: ContractCall, - ) -> ChainResult> { 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 SubmittableBatch { 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, ) -> ChainResult { 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 { + // 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 index d693b371ac..099a5c5cae 100644 --- 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 index d967b5f72c..188fdc916d 100644 --- 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( provider: Arc, transaction_overrides: &TransactionOverrides, domain: &HyperlaneDomain, + with_gas_limit_overrides: bool, ) -> ChainResult> 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 index 958b2b2606..fbe951abef 100644 --- 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 { 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 index 3e88ed67c5..66e58131aa 100644 --- 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 { // 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 index 1d5a5d7b01..b4c5bed37e 100644 --- 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; /// 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 index 438ecaedaa..7333e2c8a3 100644 --- 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 {} pub fn process_calldata( @@ -103,9 +102,8 @@ impl Mailbox for MockMailboxContract { &self, message: &HyperlaneMessage, metadata: &[u8], - apply_gas_overrides: bool, ) -> ChainResult { - self.process_estimate_costs(message, metadata, apply_gas_overrides) + self.process_estimate_costs(message, metadata) } fn process_calldata(&self, message: &HyperlaneMessage, metadata: &[u8]) -> Vec {