From 4f8245a1ed8a48fd7d3f72990b05570a4ef994ac Mon Sep 17 00:00:00 2001 From: Trevor Porter Date: Thu, 14 Nov 2024 10:29:47 +0000 Subject: [PATCH] chore: apply 10% gas buffer for Arbitrum Nitro txs, some drivebys (#4854) ### Description - Applies a 10% buffer to arbitrum nitro txs, see https://discord.com/channels/935678348330434570/1306264065110310987 for context ### Drive-by changes - Some drive-bys -- imo the Display impl of HyperlaneMessage isn't useful at all and should be the same as Debug (or we should stop using the Display impl) - Updates a sealevel comment that was inaccurate as flagged by some partners - makes sure that relayer pods update when there's a configmap change ### Related issues ### Backward compatibility ### Testing --- .../agents/relayer/src/msg/pending_message.rs | 2 +- .../src/contracts/mailbox.rs | 28 +++++++++++------ .../src/contracts/validator_announce.rs | 8 ++++- rust/main/chains/hyperlane-ethereum/src/tx.rs | 31 ++++++++++++++++--- .../templates/relayer-statefulset.yaml | 1 + rust/main/hyperlane-core/src/types/message.rs | 7 +---- .../hyperlane-sealevel-token/src/processor.rs | 7 ++--- 7 files changed, 57 insertions(+), 27 deletions(-) diff --git a/rust/main/agents/relayer/src/msg/pending_message.rs b/rust/main/agents/relayer/src/msg/pending_message.rs index cee00aed7..d63b7b03f 100644 --- a/rust/main/agents/relayer/src/msg/pending_message.rs +++ b/rust/main/agents/relayer/src/msg/pending_message.rs @@ -468,7 +468,7 @@ impl PendingOperation for PendingMessage { actual_gas_for_message = ?gas_used_by_operation, message_gas_estimate = ?operation_estimate, submission_gas_estimate = ?submission_estimated_cost, - message = ?self.message, + hyp_message = ?self.message, "Gas used by message submission" ); } diff --git a/rust/main/chains/hyperlane-ethereum/src/contracts/mailbox.rs b/rust/main/chains/hyperlane-ethereum/src/contracts/mailbox.rs index a9141e7b7..cec2b9b45 100644 --- a/rust/main/chains/hyperlane-ethereum/src/contracts/mailbox.rs +++ b/rust/main/chains/hyperlane-ethereum/src/contracts/mailbox.rs @@ -334,6 +334,7 @@ where tx, self.provider.clone(), &self.conn.transaction_overrides.clone(), + &self.domain, ) .await } @@ -382,6 +383,7 @@ where call, provider: self.provider.clone(), transaction_overrides: self.conn.transaction_overrides.clone(), + domain: self.domain.clone(), } } } @@ -418,12 +420,18 @@ pub struct SubmittableBatch { pub call: ContractCall>, provider: Arc, transaction_overrides: TransactionOverrides, + domain: HyperlaneDomain, } impl SubmittableBatch { pub async fn submit(self) -> ChainResult { - let call_with_gas_overrides = - fill_tx_gas_params(self.call, self.provider, &self.transaction_overrides).await?; + let call_with_gas_overrides = fill_tx_gas_params( + self.call, + self.provider, + &self.transaction_overrides, + &self.domain, + ) + .await?; let outcome = report_tx(call_with_gas_overrides).await?; Ok(outcome.into()) } @@ -615,10 +623,10 @@ mod test { TxCostEstimate, H160, H256, U256, }; - use crate::{contracts::EthereumMailbox, ConnectionConf, RpcConnectionConf}; - - /// An amount of gas to add to the estimated gas - const GAS_ESTIMATE_BUFFER: u32 = 75_000; + use crate::{ + contracts::EthereumMailbox, tx::apply_gas_estimate_buffer, ConnectionConf, + RpcConnectionConf, + }; fn get_test_mailbox( domain: HyperlaneDomain, @@ -650,9 +658,9 @@ mod test { #[tokio::test] async fn test_process_estimate_costs_sets_l2_gas_limit_for_arbitrum() { + let domain = HyperlaneDomain::Known(KnownHyperlaneDomain::PlumeTestnet); // An Arbitrum Nitro chain - let (mailbox, mock_provider) = - get_test_mailbox(HyperlaneDomain::Known(KnownHyperlaneDomain::PlumeTestnet)); + let (mailbox, mock_provider) = get_test_mailbox(domain.clone()); let message = HyperlaneMessage::default(); let metadata: Vec = vec![]; @@ -696,8 +704,8 @@ mod test { .await .unwrap(); - // The TxCostEstimate's gas limit includes the buffer - let estimated_gas_limit = gas_limit.saturating_add(GAS_ESTIMATE_BUFFER.into()); + // The TxCostEstimate's gas limit includes a buffer + let estimated_gas_limit = apply_gas_estimate_buffer(gas_limit, &domain).unwrap(); assert_eq!( tx_cost_estimate, 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 2da1078f9..f37c6edb8 100644 --- a/rust/main/chains/hyperlane-ethereum/src/contracts/validator_announce.rs +++ b/rust/main/chains/hyperlane-ethereum/src/contracts/validator_announce.rs @@ -92,7 +92,13 @@ where announcement.value.storage_location, serialized_signature.into(), ); - fill_tx_gas_params(tx, self.provider.clone(), &self.conn.transaction_overrides).await + fill_tx_gas_params( + tx, + self.provider.clone(), + &self.conn.transaction_overrides, + &self.domain, + ) + .await } } diff --git a/rust/main/chains/hyperlane-ethereum/src/tx.rs b/rust/main/chains/hyperlane-ethereum/src/tx.rs index 05cc4da44..31975643b 100644 --- a/rust/main/chains/hyperlane-ethereum/src/tx.rs +++ b/rust/main/chains/hyperlane-ethereum/src/tx.rs @@ -16,7 +16,8 @@ use ethers_core::{ }, }; use hyperlane_core::{ - utils::bytes_to_hex, ChainCommunicationError, ChainResult, ReorgPeriod, H256, U256, + utils::bytes_to_hex, ChainCommunicationError, ChainResult, HyperlaneDomain, ReorgPeriod, H256, + U256, }; use tracing::{debug, error, info, warn}; @@ -25,8 +26,26 @@ use crate::{EthereumReorgPeriod, Middleware, TransactionOverrides}; /// An amount of gas to add to the estimated gas pub const GAS_ESTIMATE_BUFFER: u32 = 75_000; -pub fn apply_gas_estimate_buffer(gas: U256) -> U256 { - gas.saturating_add(GAS_ESTIMATE_BUFFER.into()) +// A multiplier to apply to the estimated gas, i.e. 10%. +pub const GAS_ESTIMATE_MULTIPLIER_NUMERATOR: u32 = 11; +pub const GAS_ESTIMATE_MULTIPLIER_DENOMINATOR: u32 = 10; + +pub fn apply_gas_estimate_buffer(gas: U256, domain: &HyperlaneDomain) -> ChainResult { + // Arbitrum Nitro chains use 2d fees are are especially prone to costs increasing + // by the time the transaction lands on chain, requiring a higher gas limit. + // In this case, we apply a multiplier to the gas estimate. + let gas = if domain.is_arbitrum_nitro() { + gas.saturating_mul(GAS_ESTIMATE_MULTIPLIER_NUMERATOR.into()) + .checked_div(GAS_ESTIMATE_MULTIPLIER_DENOMINATOR.into()) + .ok_or_else(|| { + ChainCommunicationError::from_other_str("Gas estimate buffer divide by zero") + })? + } else { + gas + }; + + // Always add a flat buffer + Ok(gas.saturating_add(GAS_ESTIMATE_BUFFER.into())) } const PENDING_TRANSACTION_POLLING_INTERVAL: Duration = Duration::from_secs(2); @@ -92,17 +111,19 @@ pub(crate) async fn fill_tx_gas_params( tx: ContractCall, provider: Arc, transaction_overrides: &TransactionOverrides, + domain: &HyperlaneDomain, ) -> ChainResult> where M: Middleware + 'static, D: Detokenize, { // either use the pre-estimated gas limit or estimate it - let estimated_gas_limit: U256 = match tx.tx.gas() { + let mut estimated_gas_limit: U256 = match tx.tx.gas() { Some(&estimate) => estimate.into(), None => tx.estimate_gas().await?.into(), }; - let estimated_gas_limit = apply_gas_estimate_buffer(estimated_gas_limit); + + 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 { diff --git a/rust/main/helm/hyperlane-agent/templates/relayer-statefulset.yaml b/rust/main/helm/hyperlane-agent/templates/relayer-statefulset.yaml index d317a0c84..da69543d8 100644 --- a/rust/main/helm/hyperlane-agent/templates/relayer-statefulset.yaml +++ b/rust/main/helm/hyperlane-agent/templates/relayer-statefulset.yaml @@ -17,6 +17,7 @@ spec: metadata: annotations: checksum/configmap: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }} + checksum/relayer-configmap: {{ include (print $.Template.BasePath "/relayer-configmap.yaml") . | sha256sum }} {{- with .Values.podAnnotations }} {{- toYaml . | nindent 8 }} {{- end }} diff --git a/rust/main/hyperlane-core/src/types/message.rs b/rust/main/hyperlane-core/src/types/message.rs index 576e1eb48..359c024c1 100644 --- a/rust/main/hyperlane-core/src/types/message.rs +++ b/rust/main/hyperlane-core/src/types/message.rs @@ -74,12 +74,7 @@ impl Debug for HyperlaneMessage { impl Display for HyperlaneMessage { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!( - f, - "HyperlaneMessage {{ id: {:?}, nonce: {}, .. }}", - self.id(), - self.nonce - ) + Debug::fmt(self, f) } } diff --git a/rust/sealevel/programs/hyperlane-sealevel-token/src/processor.rs b/rust/sealevel/programs/hyperlane-sealevel-token/src/processor.rs index f203a1e4e..3e7314bc6 100644 --- a/rust/sealevel/programs/hyperlane-sealevel-token/src/processor.rs +++ b/rust/sealevel/programs/hyperlane-sealevel-token/src/processor.rs @@ -120,10 +120,9 @@ fn initialize(program_id: &Pubkey, accounts: &[AccountInfo], init: Init) -> Prog /// 12. `[]` OPTIONAL - The Overhead IGP program, if the configured IGP is an Overhead IGP. /// 13. `[writeable]` The IGP account. /// ---- End if ---- -/// 14. `[signer]` The token sender. -/// 15. `[executable]` The spl_token_2022 program. -/// 16. `[writeable]` The mint / mint authority PDA account. -/// 17. `[writeable]` The token sender's associated token account, from which tokens will be burned. +/// 14. `[executable]` The spl_token_2022 program. +/// 15. `[writeable]` The mint / mint authority PDA account. +/// 16. `[writeable]` The token sender's associated token account, from which tokens will be burned. fn transfer_remote( program_id: &Pubkey, accounts: &[AccountInfo],