Skip to content

feat: fallback to single tx submission if batching fails#7247

Merged
kamiyaa merged 15 commits intomainfrom
jeff/multicall
Nov 5, 2025
Merged

feat: fallback to single tx submission if batching fails#7247
kamiyaa merged 15 commits intomainfrom
jeff/multicall

Conversation

@kamiyaa
Copy link
Copy Markdown
Contributor

@kamiyaa kamiyaa commented Oct 23, 2025

Description

  • if batching fails, we should revert to single tx submission

Related issues

Testing

  • unittest

Summary by CodeRabbit

  • New Features

    • Per-domain batched-transaction metrics added and exposed on the Ethereum adapter (with an accessor).
  • Bug Fixes

    • Batched transactions now reliably fall back to single transactions on failure.
    • Clearer chain communication errors including an explicit "contract not found" case and improved failure reporting.
  • Tests

    • Reorganized and expanded tests covering batching, resubmission logic, and VM-specific metrics.
  • Chores

    • Test utilities made publicly accessible for reuse.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Oct 23, 2025

⚠️ No Changeset found

Latest commit: e8fa4a7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (ba380c5) to head (e8fa4a7).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff      @@
##   main   #7247   +/-   ##
============================
============================
Components Coverage Δ
core ∅ <ø> (∅)
hooks ∅ <ø> (∅)
isms ∅ <ø> (∅)
token ∅ <ø> (∅)
middlewares ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 23, 2025

📝 Walkthrough

Walkthrough

This diff replaces eyre-based errors with ChainResult/ChainCommunicationError for multicall/provider paths, adds domain-aware batched-transaction metrics and wiring through Dispatcher → EthereumAdapter → NonceManager, changes batched build to return ChainResult with a fallback to single-tx builds, and reorganizes adapter tests into focused modules.

Changes

Cohort / File(s) Summary
Multicall & provider error migration
\rust/main/chains/hyperlane-ethereum/src/contracts/multicall.rs`, `rust/main/chains/hyperlane-ethereum/src/rpc_clients/provider.rs``
Change return types from eyre::Result to ChainResult. Replace eyre errors with ChainCommunicationError variants (ContractNotFound, CustomError) and adjust logging/returns.
Core error enum
\rust/main/hyperlane-core/src/error.rs``
Add ContractNotFound(String) variant to ChainCommunicationError with display "Address is not a contract {0}".
Dispatcher & adapter metrics
\rust/main/lander/src/dispatcher/metrics.rs`, `rust/main/lander/src/adapter/chains/ethereum/metrics.rs``
Add batched_transactions: IntCounterVec to DispatcherMetrics; make EthereumAdapterMetrics domain-aware, add batched_transaction counter, increment_batched_transactions() and label constants (success/failed), plus getter.
Adapter batching & wiring
\rust/main/lander/src/adapter/chains/ethereum/adapter.rs`, `rust/main/lander/src/tests/evm/test_utils.rs``
build_batched_transaction now returns ChainResult<Vec<TxBuildingResult>>; EthereumAdapter gains a public metrics field and metrics() accessor. Batched build increments success/failure metrics and falls back to per-tx builds on failure; metrics cloned into NonceManager.
Tests: organization
\rust/main/lander/src/adapter/chains/ethereum/adapter/tests.rs`**
Replace large inline test bodies with three module declarations: check_if_resubmission_makes_sense, tests_build, vm_specific_metrics.
Tests: new/relocated suites
\rust/main/lander/src/adapter/chains/ethereum/adapter/tests/check_if_resubmission_makes_sense.rs`, `rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs`, `rust/main/lander/src/adapter/chains/ethereum/adapter/tests/vm_specific_metrics.rs``
Add focused tests for resubmission logic, batched build scenarios (happy path, missing contract, custom error), and VM-specific metric extraction for Legacy/EIP-1559/EIP-2930.
Test visibility change
\rust/main/lander/src/tests/evm.rs``
Make test_utils module public (pub mod test_utils;) for reuse across tests.

Sequence Diagram(s)

sequenceDiagram
    %% Batched build with metrics and fallback (high-level)
    participant Caller
    participant EthereumAdapter
    participant BatchBuilder as build_batched_transaction
    participant SingleBuilder as single_tx_builder
    participant Metrics

    Caller->>EthereumAdapter: build_transactions(precursors)
    EthereumAdapter->>BatchBuilder: attempt batched build (async)
    alt Batch Success
        BatchBuilder-->>EthereumAdapter: ChainResult::Ok(batched_results)
        EthereumAdapter->>Metrics: increment_batched_transactions("success", count)
        EthereumAdapter-->>Caller: return batched_results
    else Batch Fails
        BatchBuilder-->>EthereumAdapter: ChainResult::Err(err)
        EthereumAdapter->>Metrics: increment_batched_transactions("failed", count)
        EthereumAdapter->>SingleBuilder: build each tx individually (fallback)
        SingleBuilder-->>EthereumAdapter: vec(single_results)
        EthereumAdapter-->>Caller: return per-tx results
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay extra attention to:
    • adapter.rs: batching fallback logic, correct metric increments and logging placement
    • multicall.rs / provider.rs: error conversions to ChainCommunicationError and propagation
    • metrics.rs wiring and cloning semantics through Dispatcher → Adapter → NonceManager
    • tests: imports/visibility after making test_utils public and reorganized modules

Possibly related PRs

Suggested reviewers

  • yjamin
  • daniel-savu

"A little tidy, swamp-friendly patch in tow,
Errors swapped proper, metrics set to grow.
Batches try first, then fall back with care,
Tests split neat — less mess everywhere.
Code walks cleaner now, like morning slow."

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: implementing fallback to single transaction submission when batching fails, which is the core objective of this PR.
Description check ✅ Passed The description includes key sections (Description, Related issues, Testing) but lacks Drive-by changes and Backward compatibility sections from the template.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jeff/multicall

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba0f1fd and 0f8760e.

📒 Files selected for processing (1)
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: infra-test
  • GitHub Check: build-and-push-to-gcr
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (sealevel)
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: lint-rs
  • GitHub Check: test-rs
  • GitHub Check: lander-coverage

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests/check_if_resubmission_makes_sense.rs (1)

15-402: Suggest extracting helper functions to reduce test duplication.

The test functions contain significant duplication in their setup code. Each test manually constructs TypedTransaction variants with identical boilerplate fields. Consider extracting helper functions like setup_tx_with_eip1559_gas_price and setup_tx_with_legacy_gas_price to improve maintainability and readability.

Example refactor:

fn setup_eip1559_tx_with_gas(
    status: crate::TransactionStatus,
    max_fee: u64,
    max_priority_fee: u64,
) -> Transaction {
    let mut tx = dummy_evm_tx(
        ExpectedTxType::Eip1559,
        vec![],
        status,
        H160::random(),
    );
    
    if let VmSpecificTxData::Evm(ethereum_tx_precursor) = &mut tx.vm_specific_data {
        ethereum_tx_precursor.tx = TypedTransaction::Eip1559(Eip1559TransactionRequest {
            from: Some(H160::random()),
            to: Some(H160::random().into()),
            nonce: Some(0.into()),
            gas: Some(21000.into()),
            max_fee_per_gas: Some(max_fee.into()),
            max_priority_fee_per_gas: Some(max_priority_fee.into()),
            value: Some(1.into()),
            ..Default::default()
        });
    }
    tx
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4011a45 and 4ed9d36.

📒 Files selected for processing (8)
  • rust/main/chains/hyperlane-ethereum/src/contracts/multicall.rs (3 hunks)
  • rust/main/chains/hyperlane-ethereum/src/rpc_clients/provider.rs (1 hunks)
  • rust/main/hyperlane-core/src/error.rs (1 hunks)
  • rust/main/lander/src/adapter/chains/ethereum/adapter.rs (4 hunks)
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests.rs (1 hunks)
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/check_if_resubmission_makes_sense.rs (1 hunks)
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (1 hunks)
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/vm_specific_metrics.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
rust/main/{hyperlane-core,hyperlane-base}/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Keep shared Rust core crates in rust/main/{hyperlane-core,hyperlane-base}

Files:

  • rust/main/hyperlane-core/src/error.rs
rust/main/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Run cargo clippy for Rust code linting

Files:

  • rust/main/hyperlane-core/src/error.rs
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests.rs
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/vm_specific_metrics.rs
  • rust/main/chains/hyperlane-ethereum/src/contracts/multicall.rs
  • rust/main/lander/src/adapter/chains/ethereum/adapter.rs
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/check_if_resubmission_makes_sense.rs
  • rust/main/chains/hyperlane-ethereum/src/rpc_clients/provider.rs
rust/main/chains/{hyperlane-ethereum,hyperlane-cosmos,hyperlane-sealevel,hyperlane-fuel}/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Keep chain support implementations within rust/main/chains/{hyperlane-ethereum,hyperlane-cosmos,hyperlane-sealevel,hyperlane-fuel}

Files:

  • rust/main/chains/hyperlane-ethereum/src/contracts/multicall.rs
  • rust/main/chains/hyperlane-ethereum/src/rpc_clients/provider.rs
🧬 Code graph analysis (5)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests.rs (1)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (1)
  • check_if_resubmission_makes_sense (164-195)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (2)
rust/main/lander/src/tests/test_utils.rs (1)
  • tmp_dbs (41-51)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (1)
  • new (73-126)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests/vm_specific_metrics.rs (2)
rust/main/lander/src/adapter/chains/ethereum/tests.rs (1)
  • dummy_evm_tx (116-138)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (1)
  • extract_vm_specific_metrics (302-321)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (3)
rust/main/lander/src/adapter/chains/ethereum/precursor.rs (2)
  • new (48-50)
  • from_payload (52-59)
rust/main/agents/relayer/src/msg/pending_message.rs (1)
  • payload (550-560)
rust/main/hyperlane-core/src/traits/pending_operation.rs (1)
  • payload (175-175)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests/check_if_resubmission_makes_sense.rs (2)
rust/main/lander/src/adapter/chains/ethereum/tests.rs (1)
  • dummy_evm_tx (116-138)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (3)
  • tx (265-274)
  • tx (667-673)
  • check_if_resubmission_makes_sense (164-195)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (55)
  • GitHub Check: infra-test
  • GitHub Check: cli-evm-e2e-matrix (warp-send)
  • GitHub Check: cli-evm-e2e-matrix (warp-read)
  • GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
  • GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
  • GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
  • GitHub Check: cli-evm-e2e-matrix (warp-deploy)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-3)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-1)
  • GitHub Check: cli-evm-e2e-matrix (warp-init)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-2)
  • GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
  • GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
  • GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-2)
  • GitHub Check: cli-evm-e2e-matrix (core-read)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-ism-updates)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-1)
  • GitHub Check: cli-evm-e2e-matrix (core-deploy)
  • GitHub Check: cli-evm-e2e-matrix (relay)
  • GitHub Check: cli-evm-e2e-matrix (core-apply)
  • GitHub Check: cli-evm-e2e-matrix (core-init)
  • GitHub Check: env-test-matrix (mainnet3, inevm, igp)
  • GitHub Check: cli-evm-e2e-matrix (core-check)
  • GitHub Check: env-test-matrix (mainnet3, optimism, core)
  • GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
  • GitHub Check: env-test-matrix (mainnet3, inevm, core)
  • GitHub Check: env-test-matrix (testnet4, sepolia, core)
  • GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
  • GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
  • GitHub Check: env-test-matrix (mainnet3, optimism, igp)
  • GitHub Check: env-test-matrix (mainnet3, ethereum, core)
  • GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
  • GitHub Check: cosmos-sdk-e2e-run
  • GitHub Check: cli-cosmos-e2e-matrix (warp-read)
  • GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
  • GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
  • GitHub Check: cli-cosmos-e2e-matrix (core-read)
  • GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
  • GitHub Check: cli-cosmos-e2e-matrix (core-check)
  • GitHub Check: cli-cosmos-e2e-matrix (core-apply)
  • GitHub Check: cli-install-test-run
  • GitHub Check: yarn-test-run
  • GitHub Check: build-and-push-to-gcr
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (sealevel)
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: agent-configs (testnet4)
  • GitHub Check: test-rs
  • GitHub Check: lint-rs
  • GitHub Check: lander-coverage
🔇 Additional comments (10)
rust/main/hyperlane-core/src/error.rs (1)

75-77: LGTM!

The new ContractNotFound error variant properly handles the case when a contract address doesn't contain deployed code. The error message is clear and includes the address for debugging.

rust/main/chains/hyperlane-ethereum/src/rpc_clients/provider.rs (1)

336-348: LGTM!

The return type change from eyre::Result to ChainResult aligns nicely with the broader error handling improvements across the codebase. Error propagation through the ? operator works as expected.

rust/main/lander/src/adapter/chains/ethereum/adapter/tests/vm_specific_metrics.rs (1)

11-110: Test coverage looks solid.

The tests properly verify VM-specific metrics extraction for all three transaction types (Legacy, EIP-1559, EIP-2930). The assertions correctly validate that gas_price, priority_fee, and gas_limit are extracted as expected for each variant.

rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (1)

32-125: Test correctly validates fallback behavior.

The test properly simulates the batch contract missing scenario and verifies that the adapter falls back to building individual transactions. The assertions confirm both the correct number of results and that transactions were successfully built.

rust/main/lander/src/adapter/chains/ethereum/adapter/tests.rs (1)

1-3: Nice test organization.

Breaking out the tests into separate modules improves maintainability and makes the test structure clearer.

rust/main/lander/src/adapter/chains/ethereum/adapter.rs (3)

14-14: LGTM!

The ChainResult import enables proper error propagation throughout the adapter.


337-363: Well-structured error propagation.

The signature change to return ChainResult<Vec<TxBuildingResult>> properly propagates batch-building errors up the call chain. The ? operator on line 354 cleanly handles failures from provider.batch.


440-467: Fallback logic implements the core feature correctly.

When batch building fails, the code appropriately falls back to constructing individual transactions for each payload. The warnings provide helpful context for debugging. The logic correctly iterates over payloads and builds single transactions using build_single_transaction.

rust/main/chains/hyperlane-ethereum/src/contracts/multicall.rs (2)

10-12: LGTM!

The updated imports support the new ChainResult-based error handling.


34-78: Error handling is well-structured.

The changes properly use ChainResult and ChainCommunicationError variants. The ContractNotFound error at line 62 correctly encodes the address in hex format for debugging. The CustomError path (lines 70-73) provides a descriptive message and logs the error before returning. Good stuff.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4ed9d36 and 78e0755.

📒 Files selected for processing (4)
  • rust/main/lander/src/adapter/chains/ethereum/adapter.rs (6 hunks)
  • rust/main/lander/src/adapter/chains/ethereum/metrics.rs (3 hunks)
  • rust/main/lander/src/adapter/chains/ethereum/nonce/state/core.rs (1 hunks)
  • rust/main/lander/src/dispatcher/metrics.rs (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
rust/main/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Run cargo clippy for Rust code linting

Files:

  • rust/main/lander/src/adapter/chains/ethereum/adapter.rs
  • rust/main/lander/src/adapter/chains/ethereum/nonce/state/core.rs
  • rust/main/lander/src/adapter/chains/ethereum/metrics.rs
  • rust/main/lander/src/dispatcher/metrics.rs
🧬 Code graph analysis (3)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (5)
rust/main/lander/src/adapter/chains/ethereum/nonce/state/core.rs (2)
  • metrics (34-36)
  • new (20-32)
rust/main/lander/src/adapter/chains/ethereum/metrics.rs (1)
  • new (25-37)
rust/main/lander/src/adapter/chains/ethereum/precursor.rs (2)
  • new (48-50)
  • from_payload (52-59)
rust/main/lander/src/adapter/chains/ethereum/transaction/factory.rs (1)
  • build (11-23)
rust/main/hyperlane-core/src/traits/pending_operation.rs (1)
  • payload (175-175)
rust/main/lander/src/adapter/chains/ethereum/metrics.rs (4)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (1)
  • new (76-131)
rust/main/lander/src/adapter/chains/ethereum/nonce/state/core.rs (1)
  • new (20-32)
rust/main/lander/src/dispatcher/metrics.rs (1)
  • new (61-209)
rust/main/hyperlane-core/src/chain.rs (1)
  • new_test_domain (335-343)
rust/main/lander/src/dispatcher/metrics.rs (1)
rust/main/hyperlane-base/src/metrics/core.rs (1)
  • registry (374-376)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: build-and-push-to-gcr
  • GitHub Check: lander-coverage
  • GitHub Check: lint-rs
  • GitHub Check: test-rs
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (sealevel)
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: yarn-install
🔇 Additional comments (8)
rust/main/lander/src/dispatcher/metrics.rs (2)

33-33: Looks good, mate!

The new batched_transactions metric is properly registered and wired up. It follows the same pattern as the other metrics in this file.

Also applies to: 102-109, 196-196


298-300: Getter looks fine.

Returns a clone of the IntCounterVec, which aligns with how other metric accessors work.

rust/main/lander/src/adapter/chains/ethereum/nonce/state/core.rs (1)

34-36: Simple accessor, all good.

The metrics() getter exposes the internal metrics field cleanly.

rust/main/lander/src/adapter/chains/ethereum/adapter.rs (2)

343-368: Error propagation is well done.

Changing the return type to ChainResult allows batch failures to be properly caught and handled by the caller. The ? operator on line 359 cleanly propagates errors from the provider's batch operation.


446-488: Fallback logic handles batch failures properly.

When batching fails, the code:

  1. Increments the failure metric with the domain label
  2. Logs clear warnings about the failure and fallback
  3. Falls back to building individual transactions for each payload

This preserves the original single-transaction behavior when batching isn't available.

rust/main/lander/src/adapter/chains/ethereum/metrics.rs (3)

15-17: Domain-aware metrics properly added.

The domain and batched_transaction fields are cleanly integrated into the struct and constructor.

Also applies to: 25-37


47-51: Metric increment method works correctly.

The increment_batched_transactions method properly labels the counter with domain and status, then increments by the specified amount.


63-64: Test fixture updated correctly.

The dummy_instance properly constructs the new metrics with a test domain and batched transactions counter.

Comment thread rust/main/lander/src/adapter/chains/ethereum/metrics.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (1)

446-488: Well done on the fallback logic!

The error handling is solid - tries batching first, tracks success, and gracefully falls back to individual transactions when the batch contract isn't available or fails. The metrics tracking on both paths gives good observability.

One tiny consistency thing: line 479 uses precursors.len() for the capacity, but you're iterating over payloads in the loop. They're the same length so no harm done, but using payloads.len() would read a bit clearer.

Optional diff for consistency:

-                let mut results = Vec::with_capacity(precursors.len());
+                let mut results = Vec::with_capacity(payloads.len());
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba8eb61 and f517311.

📒 Files selected for processing (1)
  • rust/main/lander/src/adapter/chains/ethereum/adapter.rs (6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
rust/main/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Run cargo clippy for Rust code linting

Files:

  • rust/main/lander/src/adapter/chains/ethereum/adapter.rs
🧠 Learnings (1)
📚 Learning: 2025-09-02T18:44:06.598Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T18:44:06.598Z
Learning: Applies to rust/main/chains/{hyperlane-ethereum,hyperlane-cosmos,hyperlane-sealevel,hyperlane-fuel}/**/src/**/*.rs : Keep chain support implementations within rust/main/chains/{hyperlane-ethereum,hyperlane-cosmos,hyperlane-sealevel,hyperlane-fuel}

Applied to files:

  • rust/main/lander/src/adapter/chains/ethereum/adapter.rs
🧬 Code graph analysis (1)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (4)
rust/main/lander/src/adapter/chains/ethereum/nonce/state/core.rs (2)
  • metrics (34-36)
  • new (20-32)
rust/main/lander/src/adapter/chains/ethereum/metrics.rs (1)
  • new (25-37)
rust/main/lander/src/adapter/chains/ethereum/precursor.rs (2)
  • new (50-52)
  • from_payload (54-61)
rust/main/lander/src/adapter/chains/ethereum/transaction/factory.rs (1)
  • build (11-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: infra-test
  • GitHub Check: build-and-push-to-gcr
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: e2e-matrix (sealevel)
  • GitHub Check: lint-rs
  • GitHub Check: test-rs
  • GitHub Check: lander-coverage
🔇 Additional comments (4)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (4)

37-39: Looks good! Metrics labels properly imported.

These labels are used correctly in the batched transaction success/failure paths below.


104-105: Domain and batched transaction metrics properly wired.

The metrics initialization correctly passes domain and batched_transaction counter to enable domain-aware tracking.


347-368: Proper error propagation with ChainResult.

The signature change and ? operator correctly propagate batching errors to the caller, allowing the fallback logic to kick in. Much cleaner than the old approach!


1-784: Sandbox can't run cargo—you'll need to verify clippy locally.

The sandbox environment doesn't have the necessary setup to execute cargo commands. You'll want to run this on your machine to make sure there are no linting issues lurking in that adapter file:

cd rust/main/lander
cargo clippy -- -D warnings

If that runs clean without warnings, you're good to go. If clippy grumbles about something, well, that's what it's there for—best to sort it out now before it becomes someone else's headache.

Comment thread rust/main/lander/src/adapter/chains/ethereum/nonce/state/core.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (1)

1-36: Clean up unused and suspicious imports.

Several imports appear unused or misplaced for Ethereum adapter tests:

  • Line 9: eyre::Result is imported but not used
  • Line 18: solana_client::client_error::reqwest seems out of place in Ethereum tests
  • Line 19: tokio::sync::Mutex is not used
  • Lines 28-30: Imports from radix adapter tests (MockRadixProvider, MAILBOX_ADDRESS, TEST_PRIVATE_KEY) appear unrelated to Ethereum adapter tests

Remove unused imports to keep the codebase tidy.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fe8642e and eb3850f.

📒 Files selected for processing (1)
  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
rust/main/**/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Run cargo clippy for Rust code linting

Files:

  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs
🧠 Learnings (1)
📚 Learning: 2025-09-02T18:44:06.598Z
Learnt from: CR
Repo: hyperlane-xyz/hyperlane-monorepo PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T18:44:06.598Z
Learning: Applies to rust/main/chains/{hyperlane-ethereum,hyperlane-cosmos,hyperlane-sealevel,hyperlane-fuel}/**/src/**/*.rs : Keep chain support implementations within rust/main/chains/{hyperlane-ethereum,hyperlane-cosmos,hyperlane-sealevel,hyperlane-fuel}

Applied to files:

  • rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs
🧬 Code graph analysis (1)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (4)
rust/main/lander/src/adapter/chains/ethereum/adapter.rs (2)
  • metrics (372-374)
  • new (77-133)
rust/main/lander/src/tests/evm/test_utils.rs (1)
  • mock_ethereum_adapter (95-149)
rust/main/lander/src/tests/test_utils.rs (1)
  • tmp_dbs (43-53)
rust/main/lander/src/adapter/chains/ethereum/metrics.rs (2)
  • new (25-37)
  • get_batched_transactions (53-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
  • GitHub Check: build-and-push-to-gcr
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (sealevel)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: test-rs
  • GitHub Check: lander-coverage
  • GitHub Check: lint-rs
🔇 Additional comments (2)
rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs (2)

145-230: Good test coverage for ContractNotFound fallback.

This test properly verifies that when batching fails due to a missing contract, the system falls back to single transaction submission and records the appropriate failure metrics.


238-245: Verify that ContractError is a realistic failure scenario.

A past review comment suggested that provider.batch throws ChainCommunicationError::CustomError rather than ContractError. Please confirm whether testing ContractError here reflects actual error conditions that occur in production, or if additional error types should be covered.

Based on past review comments.

Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs Outdated
Comment thread rust/main/lander/src/adapter/chains/ethereum/adapter/tests/tests_build.rs Outdated
Comment thread rust/main/chains/hyperlane-ethereum/src/contracts/multicall.rs
@kamiyaa kamiyaa enabled auto-merge November 5, 2025 15:13
@kamiyaa kamiyaa added this pull request to the merge queue Nov 5, 2025
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 5, 2025
@kamiyaa kamiyaa added this pull request to the merge queue Nov 5, 2025
Merged via the queue into main with commit 92037cb Nov 5, 2025
40 checks passed
@kamiyaa kamiyaa deleted the jeff/multicall branch November 5, 2025 15:47
@github-project-automation github-project-automation Bot moved this from In Review to Done in Hyperlane Tasks Nov 5, 2025
This was referenced Nov 5, 2025
@github-actions github-actions Bot mentioned this pull request Nov 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants