Skip to content

Commit

Permalink
chore: apply 10% gas buffer for Arbitrum Nitro txs, some drivebys (#4854
Browse files Browse the repository at this point in the history
)

### 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

<!--
- Fixes #[issue number here]
-->

### Backward compatibility

<!--
Are these changes backward compatible? Are there any infrastructure
implications, e.g. changes that would prohibit deploying older commits
using this infra tooling?

Yes/No
-->

### Testing

<!--
What kind of testing have these changes undergone?

None/Manual/Unit Tests
-->
  • Loading branch information
tkporter authored Nov 14, 2024
1 parent 1866635 commit 4f8245a
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 27 deletions.
2 changes: 1 addition & 1 deletion rust/main/agents/relayer/src/msg/pending_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
);
}
Expand Down
28 changes: 18 additions & 10 deletions rust/main/chains/hyperlane-ethereum/src/contracts/mailbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ where
tx,
self.provider.clone(),
&self.conn.transaction_overrides.clone(),
&self.domain,
)
.await
}
Expand Down Expand Up @@ -382,6 +383,7 @@ where
call,
provider: self.provider.clone(),
transaction_overrides: self.conn.transaction_overrides.clone(),
domain: self.domain.clone(),
}
}
}
Expand Down Expand Up @@ -418,12 +420,18 @@ pub struct SubmittableBatch<M> {
pub call: ContractCall<M, Vec<MulticallResult>>,
provider: Arc<M>,
transaction_overrides: TransactionOverrides,
domain: HyperlaneDomain,
}

impl<M: Middleware + 'static> SubmittableBatch<M> {
pub async fn submit(self) -> ChainResult<TxOutcome> {
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())
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<u8> = vec![];
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
31 changes: 26 additions & 5 deletions rust/main/chains/hyperlane-ethereum/src/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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<U256> {
// 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);
Expand Down Expand Up @@ -92,17 +111,19 @@ pub(crate) async fn fill_tx_gas_params<M, D>(
tx: ContractCall<M, D>,
provider: Arc<M>,
transaction_overrides: &TransactionOverrides,
domain: &HyperlaneDomain,
) -> ChainResult<ContractCall<M, D>>
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
7 changes: 1 addition & 6 deletions rust/main/hyperlane-core/src/types/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down

0 comments on commit 4f8245a

Please sign in to comment.