Skip to content

feat: optimize building stage waiting#7244

Draft
kamiyaa wants to merge 4 commits intomainfrom
jeff/building-stage-sleep
Draft

feat: optimize building stage waiting#7244
kamiyaa wants to merge 4 commits intomainfrom
jeff/building-stage-sleep

Conversation

@kamiyaa
Copy link
Copy Markdown
Contributor

@kamiyaa kamiyaa commented Oct 22, 2025

Description

  • we only need to process when queue is non-empty and there is a signal to process

Backward compatibility

Yes

Summary by CodeRabbit

  • Refactor

    • Converted building-stage flow from periodic polling to event-driven notifications, improving responsiveness and reducing idle work.
    • Building stage now awaits external signals and processes available batches until empty; run loop is now stateful to support signaling.
    • Adjusted DB polling cadence when idle to be less frequent.
  • Tests

    • Added and updated tests to drive and verify the new signaling-based lifecycle and batch behaviors.

✏️ Tip: You can customize this high-level summary in your review settings.

- we only need to process when queue is non-empty and there is a signal to process
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Oct 22, 2025

⚠️ No Changeset found

Latest commit: c726e51

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 22, 2025

Codecov Report

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

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #7244   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files          1       1           
  Lines         14      14           
=====================================
  Misses        14      14           
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 22, 2025

📝 Walkthrough

Walkthrough

Adds a Tokio mpsc signaling channel between PayloadDbLoader and BuildingStage. PayloadDbLoader now enqueues payloads via a new async push_back and sends unit signals; BuildingStage receives signals, runs mutably, and processes batches until the queue is drained. Tests and constructors updated to pass/send the channel pair.

Changes

Cohort / File(s) Summary
Core wiring
rust/main/lander/src/dispatcher/core.rs
Create Tokio mpsc channel for the building stage; pass building_stage_receiver into BuildingStage::new and building_stage_sender into PayloadDbLoader::new; adjust imports and make local building stage mutable.
Payload DB loader
rust/main/lander/src/dispatcher/db/payload/loader.rs
Add building_stage_sender: mpsc::Sender<()> field; add pub async fn push_back(&self, item: FullPayload) to enqueue into BuildingStageQueue and conditionally send a unit signal; modify load logic to call push_back for ReadyToSubmit/Retry statuses; add test module hook.
Building stage runtime
rust/main/lander/src/dispatcher/stages/building_stage/building.rs
Add private building_stage_receiver: mpsc::Receiver<()> field; change run signature from pub async fn run(&self) to pub async fn run(&mut self); refactor loop to await recv() then process batches repeatedly until empty (removing prior sleep idle path).
Building stage tests
rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs
Update test helpers to accept building_stage_receiver; change run_building_stage to take &mut BuildingStage; send unit signals in tests to drive processing; adjust many test call sites accordingly.
Dispatcher tests
rust/main/lander/src/dispatcher/tests.rs
Initialize building_stage_sender/building_stage_receiver in tests and pass building_stage_sender into PayloadDbLoader::new; update imports to include tokio::sync::mpsc.
DB loader polling
rust/main/lander/src/dispatcher/db/loader.rs
Increase idle sleep in DbIterator::load_from_db from 10ms to 100ms to reduce polling cadence when no items are available.
New loader tests
rust/main/lander/src/dispatcher/db/payload/loader_tests.rs
Add extensive async tests validating PayloadDbLoader::push_back, notification semantics, queue capacity behavior, mixed-status loading, and a building-stage consumption pattern using mpsc signaling.

Sequence Diagram(s)

sequenceDiagram
    participant Loader as PayloadDbLoader
    participant Channel as mpsc Channel
    participant Builder as BuildingStage
    participant Batch as Batch Processor

    Loader->>Loader: push_back(payload)
    Loader->>Loader: enqueue payload in BuildingStageQueue
    alt capacity allows
        Loader->>Channel: send(())   %% notify builder
    end

    Note over Builder: awaiting Channel.recv()
    Channel->>Builder: recv() returns

    loop process available batches
        Builder->>Builder: pop_n from queue
        alt batch non-empty
            Builder->>Batch: build_transactions(batch)
            Batch-->>Builder: batch result
        else empty
            Builder->>Builder: break -> await next recv()
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Review focus:
    • Correctness of the new push_back signaling semantics and capacity checks in payload/loader.rs.
    • BuildingStage::run mutation and receiver ownership/borrow semantics across production and tests.
    • Test adjustments in building stage tests and new loader tests for flakiness/timing.

Possibly related PRs

Suggested reviewers

  • ameten
  • yjamin

Poem

A wee channel hums, a quiet little bell,
Payloads queue up and wait to tell.
A nudge of unit, builder wakes from rest,
Batches churn onward, doin' their best.
Peace in the loop — tidy, simple, and well. 🧅

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 74.19% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check ❓ Inconclusive The description covers the core change but is missing key sections like drive-by changes, related issues, and testing details required by the template. Expand the description to address drive-by changes (e.g., the 10ms to 100ms sleep adjustment), link related issues if applicable, and specify what testing was performed.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main optimization: introducing event-driven waiting for the building stage instead of sleep-based polling.
✨ 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/building-stage-sleep

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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs (1)

113-128: The assertion checks an empty vec; it doesn’t verify the drop status.

You expect a drop on failed build, but you assert DB state for payload_details_received, which is empty. Assert on the actual payload you queued.

-        let payload_details_received =
-            run_building_stage(1, &mut building_stage, &mut receiver).await;
-        assert_eq!(payload_details_received, vec![]);
-        assert_db_status_for_payloads(
-            &building_stage.state,
-            &payload_details_received,
-            PayloadStatus::Dropped(DropReason::FailedToBuildAsTransaction),
-        )
-        .await;
+        let payload_details_received =
+            run_building_stage(1, &mut building_stage, &mut receiver).await;
+        assert_eq!(payload_details_received, vec![]);
+        assert_db_status_for_payloads(
+            &building_stage.state,
+            &[payload_to_send.details.clone()],
+            PayloadStatus::Dropped(DropReason::FailedToBuildAsTransaction),
+        )
+        .await;
🧹 Nitpick comments (10)
rust/main/lander/src/dispatcher/tests.rs (2)

379-404: Helper spawns an extra loader wired to a different queue/channel than Dispatcher::spawn().

In mock_entrypoint_and_dispatcher, you create a separate building_stage_sender/queue and spawn a payload loader task, but Dispatcher::spawn() constructs its own sender/receiver and queue. Signals from this helper loader won’t wake the building stage started by Dispatcher::spawn(), and items enqueued into this helper queue are never consumed. Drop the helper’s loader (or plumb its channel/queue into Dispatcher) for tests that also call dispatcher.spawn(). Keeps things lean, avoids ghost work.


1-1: Tiny tidy-up: unused import.

VecDeque isn’t used in this module; consider removing it.

rust/main/lander/src/dispatcher/db/payload/loader.rs (1)

1-4: Remove std::sync::mpsc::Sender import to avoid confusion with tokio::mpsc.

Only tokio::sync::mpsc is used. Dropping the std Sender cleans up namespace shadowing.

 use std::{
     fmt::{Debug, Formatter},
-    sync::{mpsc::Sender, Arc},
+    sync::Arc,
 };
rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2)

55-58: Use slices instead of &Vec to please clippy and reduce coupling.

Signature can accept any slice.

-    async fn build_transactions(&self, payloads: &Vec<FullPayload>) {
+    async fn build_transactions(&self, payloads: &[FullPayload]) {

Call sites already pass a Vec; taking a slice is seamless: pass &payloads.


34-46: Optional: pre-drain if queue is non-empty at startup.

If anything can enqueue without signaling (now or future), consider a one-time pre-drain before awaiting the first recv. Otherwise you’re fine as wired via core.rs.

rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs (4)

234-249: Send the notify after enqueue to avoid ordering races (and use try_send).

Signaling before the stage exists/has work can be brittle. Push, then notify — and prefer try_send for a coalesced tick.

-    let (building_stage_sender, building_stage_receiver) = mpsc::channel(1);
-    building_stage_sender
-        .send(())
-        .await
-        .expect("Failed to send signal to building_stage_receiver");
+    let (building_stage_sender, building_stage_receiver) = mpsc::channel(1);
@@
     let payload_to_send = FullPayload::random();
     initialize_payload_db(&building_stage.state.payload_db, &payload_to_send).await;
     queue.push_back(payload_to_send.clone()).await;
+    let _ = building_stage_sender.try_send(());

40-47: Prefer try_send over capacity()+send().await for 1-bit notifications.

This pattern repeats a bunch. try_send coalesces and avoids awaits; if Full, you’re already notified.

-        if building_stage_sender.capacity() > 0 {
-            building_stage_sender
-                .send(())
-                .await
-                .expect("Failed to send signal to building_stage_receiver");
-        }
+        let _ = building_stage_sender.try_send(());

Apply similarly to the other occurrences called out in this comment.

Also applies to: 74-81, 113-120, 268-276, 295-301, 313-319, 355-361


375-402: run_building_stage timeout is tight.

100ms can flake on busy CI. Consider 300–500ms or drive with a bounded loop on received count to reduce timing sensitivity.


341-351: Channel-send failure semantics: confirm desired policy.

When inclusion_stage receiver is dropped, do we drop payloads, requeue, or mark dropped in DB? The current test only checks “no panic” and empty queue. Please assert the DB outcome and align with handle_tx_building_result error path.

Also applies to: 355-361, 367-373

rust/main/lander/src/dispatcher/core.rs (1)

1-12: Remove VecDeque and Mutex imports; re-enable clippy when ready.

The imports at lines 4 and 10 contain unused items. VecDeque and Mutex aren't referenced anywhere in core.rs—the #![allow(dead_code)] is masking them. PathBuf, Arc, mpsc, JoinHandle, and join_all are all actively used, so keep those.

Once you've cleaned these up, you can remove or update the TODO and the allow directive to let clippy run.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8fd3bf7 and 1741654.

📒 Files selected for processing (5)
  • rust/main/lander/src/dispatcher/core.rs (3 hunks)
  • rust/main/lander/src/dispatcher/db/payload/loader.rs (4 hunks)
  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2 hunks)
  • rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs (17 hunks)
  • rust/main/lander/src/dispatcher/tests.rs (3 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/dispatcher/stages/building_stage/building/tests.rs
  • rust/main/lander/src/dispatcher/tests.rs
  • rust/main/lander/src/dispatcher/db/payload/loader.rs
  • rust/main/lander/src/dispatcher/core.rs
  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs
🧠 Learnings (1)
📚 Learning: 2025-07-02T13:58:07.164Z
Learnt from: daniel-savu
PR: hyperlane-xyz/hyperlane-monorepo#6668
File: rust/main/lander/src/dispatcher/stages/building_stage/building.rs:41-41
Timestamp: 2025-07-02T13:58:07.164Z
Learning: In Rust lander BuildingStage, the update_metrics() call should be positioned after checking if payloads are empty to avoid unnecessary metrics updates during idle polling loops. The queue.len() operation is lightweight for VecDeque-based queues.

Applied to files:

  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs
🧬 Code graph analysis (5)
rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs (3)
rust/main/lander/src/tests/test_utils.rs (1)
  • initialize_payload_db (89-91)
rust/main/lander/src/dispatcher/stages/inclusion_stage.rs (1)
  • new (43-56)
rust/main/lander/src/dispatcher/stages/finality_stage.rs (1)
  • new (44-57)
rust/main/lander/src/dispatcher/tests.rs (4)
rust/main/lander/src/dispatcher/core.rs (2)
  • tokio (81-81)
  • tokio (83-83)
rust/main/lander/src/dispatcher/stages/inclusion_stage.rs (1)
  • new (43-56)
rust/main/lander/src/dispatcher/stages/finality_stage.rs (1)
  • new (44-57)
rust/main/lander/src/dispatcher/stages/building_stage/queue.rs (1)
  • new (13-15)
rust/main/lander/src/dispatcher/db/payload/loader.rs (1)
rust/main/lander/src/dispatcher/stages/building_stage/queue.rs (1)
  • push_back (18-20)
rust/main/lander/src/dispatcher/core.rs (3)
rust/main/lander/src/dispatcher/stages/inclusion_stage.rs (1)
  • new (43-56)
rust/main/lander/src/dispatcher/stages/finality_stage.rs (1)
  • new (44-57)
rust/main/lander/src/dispatcher/stages/building_stage/queue.rs (1)
  • new (13-15)
rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2)
rust/main/lander/src/dispatcher/stages/inclusion_stage.rs (1)
  • run (58-82)
rust/main/lander/src/dispatcher/stages/finality_stage.rs (1)
  • run (59-83)
⏰ 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). (51)
  • GitHub Check: infra-test
  • GitHub Check: cli-evm-e2e-matrix (warp-deploy)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-3)
  • GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
  • GitHub Check: cli-evm-e2e-matrix (warp-read)
  • GitHub Check: cli-evm-e2e-matrix (relay)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-2)
  • GitHub Check: cli-evm-e2e-matrix (warp-send)
  • GitHub Check: cli-evm-e2e-matrix (warp-extend-basic)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-1)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-2)
  • GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
  • GitHub Check: cli-evm-e2e-matrix (core-check)
  • GitHub Check: cli-evm-e2e-matrix (warp-extend-recovery)
  • GitHub Check: cli-evm-e2e-matrix (core-read)
  • GitHub Check: cli-evm-e2e-matrix (core-deploy)
  • GitHub Check: cli-evm-e2e-matrix (warp-init)
  • GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-1)
  • GitHub Check: cli-evm-e2e-matrix (core-init)
  • GitHub Check: cli-evm-e2e-matrix (core-apply)
  • GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
  • GitHub Check: env-test-matrix (mainnet3, inevm, core)
  • GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
  • GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
  • GitHub Check: cli-cosmos-e2e-matrix (warp-read)
  • GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
  • GitHub Check: cli-cosmos-e2e-matrix (core-read)
  • GitHub Check: cli-cosmos-e2e-matrix (core-check)
  • GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
  • GitHub Check: cli-cosmos-e2e-matrix (core-apply)
  • GitHub Check: env-test-matrix (testnet4, sepolia, core)
  • GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
  • GitHub Check: env-test-matrix (mainnet3, inevm, 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: env-test-matrix (mainnet3, optimism, core)
  • GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
  • GitHub Check: cosmos-sdk-e2e-run
  • GitHub Check: build-and-push-to-gcr
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (sealevel)
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: lint-rs
  • GitHub Check: test-rs
  • GitHub Check: lander-coverage
🔇 Additional comments (3)
rust/main/lander/src/dispatcher/db/payload/loader.rs (1)

65-75: load() path looks good; using push_back centralizes signaling.

Nice consolidation. Once the send path above is made non-panicking, this becomes robust under stage restarts.

rust/main/lander/src/dispatcher/stages/building_stage/building.rs (1)

33-52: Run loop aligns with event-driven intent; metrics update is in the right spot.

Waiting on recv() then draining the queue in batches is clean. update_metrics() after the empty-check avoids idle churn — just how we like it.
Based on learnings

rust/main/lander/src/dispatcher/core.rs (1)

78-92: Good wiring of the new signal channel.

Receiver into BuildingStage, sender into PayloadDbLoader, and a shared queue — the flow is coherent. Mut borrow for run() is handled correctly.

Comment thread rust/main/lander/src/dispatcher/db/payload/loader.rs
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/dispatcher/stages/building_stage/building.rs (1)

36-52: Clean event-driven pattern with one observability gap.

The nested loop setup makes sense: sit tight until there's work, then churn through the queue. A few observations:

  1. Metrics placement is spot on. You're updating metrics after confirming payloads exist (line 43-45 check before line 48 update). This avoids unnecessary metric updates when idle, which aligns with the established pattern for this queue. Based on learnings.

  2. Silent exit when receiver closes. When the sender gets dropped, the outer loop exits without a peep. Consider adding an info log before the loop ends - helps when troubleshooting why the stage stopped processing.

  3. The metrics comment (lines 46-47) is a touch muddy about being "lower by max_batch_size", but the actual behavior is fine. The metric shows queue length after the pop, which is what you'd expect.

To improve observability when the receiver closes:

     pub async fn run(&mut self) {
         // we can efficiently wait for more payloads if no one has notified us.
         while self.building_stage_receiver.recv().await.is_some() {
             loop {
                 // event-driven by the Building queue
                 let payloads = self
                     .queue
                     .pop_n(self.state.adapter.max_batch_size() as usize)
                     .await;
                 if payloads.is_empty() {
                     break;
                 }
                 // note: this will set the queue length metric to `length - payloads.len()`,
                 // so worst case this will be lower by `max_batch_size`
                 self.update_metrics().await;
 
                 self.build_transactions(&payloads).await;
             }
         }
+        info!("Building stage receiver closed, exiting run loop");
     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1741654 and d224a8f.

📒 Files selected for processing (1)
  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2 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/dispatcher/stages/building_stage/building.rs
🧠 Learnings (1)
📚 Learning: 2025-07-02T13:58:07.164Z
Learnt from: daniel-savu
PR: hyperlane-xyz/hyperlane-monorepo#6668
File: rust/main/lander/src/dispatcher/stages/building_stage/building.rs:41-41
Timestamp: 2025-07-02T13:58:07.164Z
Learning: In Rust lander BuildingStage, the update_metrics() call should be positioned after checking if payloads are empty to avoid unnecessary metrics updates during idle polling loops. The queue.len() operation is lightweight for VecDeque-based queues.

Applied to files:

  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs
🧬 Code graph analysis (1)
rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2)
rust/main/lander/src/dispatcher/stages/inclusion_stage.rs (1)
  • run (58-82)
rust/main/lander/src/dispatcher/stages/finality_stage.rs (1)
  • run (59-83)
⏰ 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). (54)
  • GitHub Check: env-test-matrix (mainnet3, inevm, igp)
  • GitHub Check: env-test-matrix (mainnet3, arbitrum, igp)
  • GitHub Check: env-test-matrix (mainnet3, optimism, igp)
  • GitHub Check: env-test-matrix (testnet4, sepolia, core)
  • GitHub Check: cli-evm-e2e-matrix (warp-rebalancer)
  • GitHub Check: env-test-matrix (mainnet3, ethereum, core)
  • GitHub Check: env-test-matrix (mainnet3, inevm, core)
  • GitHub Check: env-test-matrix (mainnet3, arbitrum, core)
  • GitHub Check: env-test-matrix (mainnet3, ethereum, igp)
  • GitHub Check: cli-evm-e2e-matrix (warp-send)
  • GitHub Check: cli-evm-e2e-matrix (warp-read)
  • GitHub Check: env-test-matrix (mainnet3, optimism, core)
  • GitHub Check: cli-evm-e2e-matrix (warp-init)
  • 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-deploy)
  • GitHub Check: cli-evm-e2e-matrix (core-check)
  • GitHub Check: cli-evm-e2e-matrix (warp-extend-config)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-1)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-1)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-2)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-submitters)
  • GitHub Check: cli-evm-e2e-matrix (warp-bridge-2)
  • GitHub Check: cli-evm-e2e-matrix (warp-bridge-1)
  • GitHub Check: cli-evm-e2e-matrix (warp-check-3)
  • GitHub Check: cli-evm-e2e-matrix (core-apply)
  • GitHub Check: cli-evm-e2e-matrix (core-read)
  • GitHub Check: cli-evm-e2e-matrix (warp-apply-2)
  • GitHub Check: cli-evm-e2e-matrix (relay)
  • GitHub Check: cli-evm-e2e-matrix (core-init)
  • GitHub Check: cli-evm-e2e-matrix (core-deploy)
  • GitHub Check: cosmos-sdk-e2e-run
  • GitHub Check: cli-cosmos-e2e-matrix (core-read)
  • GitHub Check: cli-cross-chain-e2e-matrix (warp-apply)
  • GitHub Check: cli-cross-chain-e2e-matrix (warp-deploy)
  • GitHub Check: cli-cosmos-e2e-matrix (warp-read)
  • GitHub Check: cli-cosmos-e2e-matrix (core-deploy)
  • GitHub Check: cli-cosmos-e2e-matrix (warp-deploy)
  • GitHub Check: coverage-run
  • GitHub Check: cli-cosmos-e2e-matrix (core-apply)
  • GitHub Check: cli-cosmos-e2e-matrix (core-check)
  • GitHub Check: cli-install-test-run
  • GitHub Check: yarn-test-run
  • GitHub Check: build-and-push-to-gcr
  • GitHub Check: e2e-matrix (sealevel)
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: yarn-install
  • GitHub Check: lander-coverage
  • GitHub Check: test-rs
  • GitHub Check: lint-rs
🔇 Additional comments (2)
rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2)

24-25: Nice addition for event-driven processing.

The receiver field turns this stage into something that actually waits for work instead of constantly checking. Much more efficient than spinning or sleeping on a timer.


34-34: Correct signature for receiver consumption.

Taking &mut self is the right move here since recv() needs mutable access. Keeps things straightforward.

@kamiyaa kamiyaa marked this pull request as draft December 8, 2025 17:38
@kamiyaa kamiyaa marked this pull request as ready for review December 11, 2025 21:45
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/dispatcher/db/payload/loader.rs (1)

33-41: Consider using try_send for better coalescing behavior.

Right now you're checking capacity and then using send().await, which is better than the panic but still blocks. For a simple wakeup signal, try_send() would be cleaner - it naturally coalesces when the buffer's full (no await needed), and you can log if the channel closes instead of silently ignoring with let _ =.

Something like:

match self.building_stage_sender.try_send(()) {
    Ok(_) | Err(mpsc::error::TrySendError::Full(_)) => {},
    Err(mpsc::error::TrySendError::Closed(_)) => {
        tracing::warn!("building_stage signal channel closed");
    }
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d224a8f and c726e51.

📒 Files selected for processing (7)
  • rust/main/lander/src/dispatcher/core.rs (3 hunks)
  • rust/main/lander/src/dispatcher/db/loader.rs (1 hunks)
  • rust/main/lander/src/dispatcher/db/payload/loader.rs (5 hunks)
  • rust/main/lander/src/dispatcher/db/payload/loader_tests.rs (1 hunks)
  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2 hunks)
  • rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs (17 hunks)
  • rust/main/lander/src/dispatcher/tests.rs (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • rust/main/lander/src/dispatcher/core.rs
  • rust/main/lander/src/dispatcher/tests.rs
🧰 Additional context used
📓 Path-based instructions (1)
rust/main/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Gas price escalation in transaction management must follow the formula: Max(Min(Max(Escalate(oldGasPrice), newEstimatedGasPrice), gasPriceCapMultiplier × newEstimatedGasPrice), oldGasPrice) to prevent indefinite escalation while maintaining competitiveness and RBF compatibility, with configurable gasPriceCapMultiplier per chain in transactionOverrides (default: 3)

Files:

  • rust/main/lander/src/dispatcher/db/payload/loader_tests.rs
  • rust/main/lander/src/dispatcher/db/payload/loader.rs
  • rust/main/lander/src/dispatcher/db/loader.rs
  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs
  • rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs
🧠 Learnings (2)
📓 Common learnings
Learnt from: daniel-savu
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6668
File: rust/main/lander/src/dispatcher/stages/building_stage/building.rs:41-41
Timestamp: 2025-07-02T13:58:07.164Z
Learning: In Rust lander BuildingStage, the update_metrics() call should be positioned after checking if payloads are empty to avoid unnecessary metrics updates during idle polling loops. The queue.len() operation is lightweight for VecDeque-based queues.
📚 Learning: 2025-07-02T13:58:07.164Z
Learnt from: daniel-savu
Repo: hyperlane-xyz/hyperlane-monorepo PR: 6668
File: rust/main/lander/src/dispatcher/stages/building_stage/building.rs:41-41
Timestamp: 2025-07-02T13:58:07.164Z
Learning: In Rust lander BuildingStage, the update_metrics() call should be positioned after checking if payloads are empty to avoid unnecessary metrics updates during idle polling loops. The queue.len() operation is lightweight for VecDeque-based queues.

Applied to files:

  • rust/main/lander/src/dispatcher/db/payload/loader_tests.rs
  • rust/main/lander/src/dispatcher/db/payload/loader.rs
  • rust/main/lander/src/dispatcher/db/loader.rs
  • rust/main/lander/src/dispatcher/stages/building_stage/building.rs
  • rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs
🧬 Code graph analysis (4)
rust/main/lander/src/dispatcher/db/payload/loader_tests.rs (2)
rust/main/hyperlane-base/src/db/rocks/mod.rs (1)
  • from_path (37-66)
rust/main/lander/src/dispatcher/db/loader.rs (2)
  • new (45-86)
  • new (230-237)
rust/main/lander/src/dispatcher/db/payload/loader.rs (3)
rust/main/lander/src/dispatcher/core.rs (2)
  • tokio (87-87)
  • tokio (89-89)
rust/main/agents/relayer/src/relayer.rs (1)
  • mpsc (336-336)
rust/main/lander/src/dispatcher/stages/building_stage/queue.rs (1)
  • push_back (18-20)
rust/main/lander/src/dispatcher/db/loader.rs (1)
rust/main/hyperlane-base/src/cache/moka/mod.rs (1)
  • sleep (130-132)
rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2)
rust/main/lander/src/dispatcher/stages/finality_stage.rs (1)
  • run (59-83)
rust/main/lander/src/dispatcher/stages/inclusion_stage.rs (1)
  • run (58-86)
⏰ 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 (sealevel)
  • GitHub Check: e2e-matrix (evm)
  • GitHub Check: e2e-matrix (radix)
  • GitHub Check: e2e-matrix (starknet)
  • GitHub Check: e2e-matrix (cosmosnative)
  • GitHub Check: e2e-matrix (cosmwasm)
  • GitHub Check: lint-rs
  • GitHub Check: test-rs
  • GitHub Check: lander-coverage
🔇 Additional comments (7)
rust/main/lander/src/dispatcher/db/loader.rs (1)

146-146: Looks good, no functional changes here.

This line remains unchanged in behavior - just the sleep call for the idle loop when no items are available to process.

rust/main/lander/src/dispatcher/db/payload/loader.rs (1)

8-8: Nice wiring of the signaling channel.

The integration of the mpsc sender field and its usage in the load path looks solid. Payloads that are ready or need retry will trigger notifications properly.

Also applies to: 22-22, 65-65, 76-77

rust/main/lander/src/dispatcher/stages/building_stage/building.rs (2)

34-52: Event-driven flow looks solid.

The outer loop waiting on the receiver and the inner loop draining the queue is a clean pattern. The metrics update happens after checking for empty payloads (line 48), which aligns with past learnings about avoiding unnecessary metric updates during idle polling.

One thing to keep in mind: if the receiver channel closes (returns None), the whole run loop exits. Make sure that's the intended shutdown behavior.


25-25: Good addition of the receiver field.

Wiring the signaling channel into the stage structure enables the event-driven processing pattern.

rust/main/lander/src/dispatcher/db/payload/loader_tests.rs (1)

1-345: Thorough test coverage for the signaling mechanism.

The tests cover the important scenarios: capacity-based notification behavior, status-based loading, rapid pushes, and even a simulation of the building stage consumption pattern. The timeout-based assertions are appropriate for verifying async channel behavior.

rust/main/lander/src/dispatcher/stages/building_stage/building/tests.rs (2)

15-19: Tests properly updated for the signaling flow.

All the test updates consistently wire in the new channel and send signals before expecting the building stage to process payloads. The pattern of checking capacity before sending (like lines 40, 75, 113, etc.) mirrors what the loader does.

Also applies to: 31-45, 66-80, 105-118, 138-160, 189-203, 234-238, 263-275, 286-300, 312-318, 341-360


377-377: Good updates to helpers and signatures.

The run_building_stage now takes &mut BuildingStage, and the setup functions properly thread through the building_stage_receiver parameter. This aligns well with the changes to BuildingStage::run.

Also applies to: 407-407, 425-425, 444-449

@hyper-gonk
Copy link
Copy Markdown
Contributor

hyper-gonk Bot commented Dec 11, 2025

🦀 Rust Agent Docker Image Built Successfully

Image Tags:

gcr.io/abacus-labs-dev/hyperlane-agent:pr-7244
gcr.io/abacus-labs-dev/hyperlane-agent:c726e51-20251211-214545

@kamiyaa kamiyaa marked this pull request as draft December 12, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

1 participant