Conversation
reth marks the network as syncing on every start and ignores transaction gossip until the engine commits its first canonical block. That includes the pool announcement a peer sends once when a session opens; the peer records those hashes as known and never announces them again. Morph blocks only ever arrive from the consensus client, so there is nothing to catch up on over p2p and the window is just the gap between start-up and the first imported block. On a sequencer it swallows every transaction RPC nodes were holding while it restarted: those never reach it, and each affected sender's later transactions wait behind the missing nonce. morph-geth accepts transactions from process start. Enable reth's --debug.startup-sync-state-idle for `morph-reth node` by default. The network is marked idle once the engine starts (after an interrupted pipeline backfill, if one is needed), so gossip and session announcements are accepted from start-up. eth_syncing now reports false from start-up as well; it already did after the first imported block however far behind the node was, so it was never a caught-up signal here.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedNext included review available in 59 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe Morph node now uses ChangesStartup transaction gossip
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant RPCNode
participant Sequencer
participant TransactionPool
RPCNode->>RPCNode: Hold pending transaction
RPCNode->>Sequencer: Connect peer networks
Sequencer->>TransactionPool: Receive transaction gossip
TransactionPool-->>Sequencer: Confirm transaction is present
Merge Risk: ⚪ Minimal · up to The change is mergeable on the supplied evidence: it targets the startup transaction gap and includes a test for propagation before the first block. Confirm the pinned dependency’s sync-state behavior during normal validation. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 5 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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. Comment |
The previous commit enabled reth's --debug.startup-sync-state-idle, which switches the network to idle only when the consensus engine task starts. The launcher starts the network, and with it the dials to trusted peers, before it sets the Syncing state, so a session that opens between the two still has its pool announcement dropped. A node restarting against a peer that holds a pending transaction received it in 5 of 20 restarts. reth gates transaction gossip on is_initially_syncing, which stays false for good after the first Syncing -> Idle switch. Build the network through a MorphNetworkBuilder that makes that switch as soon as the network is up, and drop the CLI default. The same restarts now deliver the transaction every time, and is_syncing, and with it eth_syncing, behaves exactly as before.
panos-xyz
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| let mut sequencer = sequencers.pop().unwrap(); | ||
| let mut rpc = rpcs.pop().unwrap(); | ||
|
|
||
| let tx = make_transfer_tx(wallet.chain_id, wallet_at_index(1, wallet.chain_id), 0).await; |
| } | ||
| let tip = tip.expect("five blocks were imported"); | ||
|
|
||
| let tx = make_transfer_tx(wallet.chain_id, wallet_at_index(1, wallet.chain_id), 0).await; |
2 participants
Summary
reth marks the network as syncing on every start and ignores transaction gossip until the node has switched from syncing to idle once, which otherwise happens with the first block the consensus client imports. That includes the pool announcement each peer sends once when a session opens; the peer records those hashes as known and never announces them again.
On Morph, blocks only ever come from the consensus client through the engine API, so there is nothing to catch up on over p2p. On a sequencer the gap is a real problem: after every restart, the transactions RPC nodes were holding, including everything users sent during the downtime, never reach the sequencer, and each affected sender's later transactions wait behind the missing nonce indefinitely. The gas-price and token-price oracles stall the same way when they submit through an RPC node. morph-geth accepts transactions from process start, so this only shows up with the switch to reth.
This builds the network through a new
MorphNetworkBuilder, which delegates toEthereumNetworkBuilderand switches the network's sync stateSyncing→Idleonce as soon as it is up. reth gates transaction gossip onis_initially_syncing(), which staysfalsefor good after that first switch, so theSyncingstate the launcher sets afterwards no longer drops peer transactions.The first revision of this PR enabled
--debug.startup-sync-state-idleby default instead. That flag switches to idle only when the consensus engine task starts, but the launcher starts the network, and its dials to trusted peers, before it setsSyncing, so a session opening in between still lost its pool announcement. The flag is not needed with this change, and it is not sufficient on its own.Behaviour change
is_syncing()is unchanged:eth_syncingstill reports syncing until the first imported block, and bad-transaction peer penalties stay suppressed until then, as before.Testing
network::peer_pool_reaches_node_before_first_block: a peer holding a pending transaction connects to a node that has not imported a block, and the transaction must reach the node. It times out without the network builder change and passes with it.4955f7c13): 0 of 3;--debug.startup-sync-state-idle(first revision): 5 of 20;make test-e2e, workspace unit tests,make clippy-e2e,cargo clippy -p morph-reth --all-targets -- -D warnings,cargo fmt --all -- --check.Found in the pre-migration sequencer/validator audit (SEQP-01).
Summary by CodeRabbit