Fix messaging bridge when using forked network by Kelvyne · Pull Request #882 · 0xSpaceShard/starknet-devnet

Kelvyne · 2025-10-30T11:12:32Z

Usage related changes

When using the messaging, devnet has to scan L1 for messages to include on L2. On forked network, it results in a eth_getBlocks call with a very large range, which is problematic for most (if not all) RPC.

Also, pre confirmed block can cause some messages to be missed, because they are marked as scanned when flushing. But subsequent transactions can be included in the same block number, which will cause the logic to miss messages in those transactions. I don't think it makes sense to include messages from the pre confirmed block anyway, because realistically only mined block's messages can be included on L1. I patched the logic to never include the pre confirmed block.

Development related changes

None

Checklist:

Summary by CodeRabbit

Bug Fixes
- Improved message synchronization efficiency by starting from the current blockchain state at startup instead of reprocessing historical blocks.
- Enhanced block boundary handling in message collection to ensure accurate range processing.

coderabbitai · 2025-10-30T11:12:45Z

Walkthrough

The PR modifies Ethereum message handling in two key areas. First, it changes EthereumMessaging::new to retrieve and store the current block number as the initial value for last_fetched_block, rather than initializing it to 0. This causes subsequent message fetching to start from the current head block instead of block 1. Second, it refactors the message collection logic in collect_messages_to_l1 by replacing the set_latest_local_block read-only accessor with a mutable state-mutation method, and introduces explicit block range boundaries (computed from_block and to_block) to tighten the range of blocks processed during message collection.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

API signature change: The modification to set_latest_local_block from a read-only method returning Option<String> to a mutable method accepting u64 parameter requires verification of all call sites
Block range logic: New to_block boundary computation and conditional from_block selection logic (using last_local_block if non-zero, otherwise starting_block_number) need validation for off-by-one errors
Initialization behavior: Change in block initialization strategy may affect message synchronization semantics and requires tracing through the fetch workflow to ensure no critical messages are skipped

Possibly related PRs

Enable custom account in deploying L1-L2 messaging contract #758: Both PRs modify EthereumMessaging::new in the same file; this PR changes block initialization logic while the related PR adds deployer key and address validation

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title 'Fix messaging bridge when using forked network' directly matches the main objective of the PR. The changeset addresses a specific issue with the messaging bridge functionality on forked networks by adjusting block boundary logic and preventing large RPC calls. The title accurately summarizes the primary change from a user perspective without being vague or overly broad.
Description check	✅ Passed	The pull request description follows the template structure with Usage-related changes and Development-related changes sections, includes a completed checklist with most items checked, and provides clear context about the changes.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.}

3alpha · 2025-11-03T14:45:12Z

@coderabbitai review

coderabbitai · 2025-11-03T14:45:26Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 10665c3 and b265031.

📒 Files selected for processing (2)

crates/starknet-devnet-core/src/messaging/ethereum.rs (2 hunks)
crates/starknet-devnet-core/src/messaging/mod.rs (2 hunks)

🔇 Additional comments (3)

crates/starknet-devnet-core/src/messaging/ethereum.rs (1)

119-136: LGTM! Initialization optimization for forked networks.

The change to initialize last_fetched_block from the current block number effectively prevents scanning from genesis on forked networks, which addresses the large block range issue mentioned in the PR objectives. This means fetch_messages (line 176) will start from the current block + 1 instead of block 1, significantly reducing RPC load on forked networks.

crates/starknet-devnet-core/src/messaging/mod.rs (2)

124-126: LGTM! API corrected to match naming convention.

The signature change from a getter to a setter is semantically correct - the set_ prefix should indicate a setter method. This allows external callers to explicitly update the last processed block number.

Note: This is a breaking API change.

137-148: Edge case is properly handled—no issues found.

The edge case you identified (when to_block < from_block) is explicitly tested and handled correctly. The test at lines 607-613 confirms that get_blocks(Some(BlockId::Number(10)), Some(BlockId::Number(2))) returns an empty result without error. The range check in is_block_number_in_range (line 159-160) ensures no blocks pass the filter when start > end, resulting in a valid empty Vec. The messaging code will safely handle this by receiving an empty block collection.

crates/starknet-devnet-core/src/messaging/mod.rs

3alpha · 2025-11-06T12:27:31Z

Thanks, LGTM!

There are 6 commits in the main branch after your fork. Please rebase/merge and we are good to go to run tests.

In case of a forked environment, fetching from zero can cause getBlocks RPC calls with a range too big to be handled. Instead, start fetching messages for blocks created after the devnet was launched.

…process

Kelvyne · 2025-11-06T13:50:16Z

Thanks, LGTM!

There are 6 commits in the main branch after your fork. Please rebase/merge and we are good to go to run tests.

Done, thank you !

* Do not fetch from messages from genesis In case of a forked environment, fetching from zero can cause getBlocks RPC calls with a range too big to be handled. Instead, start fetching messages for blocks created after the devnet was launched. * Do not include L2->L1 messages from pre-confirmed block * fix: only collect L2->L1 messages if there are L2 accepted blocks to process --------- Co-authored-by: Kelvyne <lakhdar@argent.xyz>

3alpha self-requested a review October 30, 2025 19:47

3alpha self-assigned this Nov 3, 2025

coderabbitai bot reviewed Nov 3, 2025

View reviewed changes

crates/starknet-devnet-core/src/messaging/mod.rs Outdated Show resolved Hide resolved

3alpha added the performance label Nov 6, 2025

Kelvyne added 3 commits November 6, 2025 14:47

Do not fetch from messages from genesis

061c1a4

In case of a forked environment, fetching from zero can cause getBlocks RPC calls with a range too big to be handled. Instead, start fetching messages for blocks created after the devnet was launched.

Do not include L2->L1 messages from pre-confirmed block

7bd0f33

fix: only collect L2->L1 messages if there are L2 accepted blocks to …

81d5162

…process

Kelvyne force-pushed the fix-fork-messaging branch from 5c03c20 to 81d5162 Compare November 6, 2025 13:47

3alpha approved these changes Nov 6, 2025

View reviewed changes

3alpha merged commit 09d1909 into 0xSpaceShard:main Nov 6, 2025
1 check passed

coderabbitai bot mentioned this pull request Nov 7, 2025

Migrate from Ethers to Alloy #887

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix messaging bridge when using forked network#882

Fix messaging bridge when using forked network#882
3alpha merged 3 commits into0xSpaceShard:mainfrom
Kelvyne:fix-fork-messaging

Kelvyne commented Oct 30, 2025 •

edited by 3alpha

Loading

Uh oh!

coderabbitai bot commented Oct 30, 2025 •

edited

Loading

Uh oh!

3alpha commented Nov 3, 2025

Uh oh!

coderabbitai bot commented Nov 3, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

3alpha commented Nov 6, 2025

Uh oh!

Kelvyne commented Nov 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Kelvyne commented Oct 30, 2025 • edited by 3alpha Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage related changes

Development related changes

Checklist:

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

3alpha commented Nov 3, 2025

Uh oh!

coderabbitai bot commented Nov 3, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

3alpha commented Nov 6, 2025

Uh oh!

Kelvyne commented Nov 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Kelvyne commented Oct 30, 2025 •

edited by 3alpha

Loading

coderabbitai bot commented Oct 30, 2025 •

edited

Loading