Repository navigation
Conversation
5d4f84d to
a7f2d69
Compare
a91aa2d to
11278eb
Compare
a7f2d69 to
fd8225c
Compare
8c879a6 to
867bbec
Compare
fd8225c to
467a874
Compare
467a874 to
aaba193
Compare
867bbec to
4caa4eb
Compare
aaba193 to
bbc1036
Compare
454b09c to
d9c9caa
Compare
d9c9caa to
c493e0a
Compare
8551b43 to
9536157
Compare
61dfd9c to
a29b6d6
Compare
ea43662 to
3b79dd8
Compare
0e2daec to
9bf378b
Compare
94b2274 to
a385b90
Compare
9bf378b to
fa35088
Compare
a385b90 to
a97ce76
Compare
fa35088 to
694558a
Compare
a97ce76 to
e0a1476
Compare
694558a to
3c63ab0
Compare
|
@rain-marvin review |
|
🔎 Reviewing |
|
@CodeRabbit review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
This PR pins every receipt balanceOf read in the backfill to an explicit block: the pass head, or a fresh eth_blockNumber taken after the log scan when that is later. Before, the reads used latest. A lagging read node could answer zero for a receipt that had just arrived, and the checkpoint then moved past its block for good. Now a node that does not have the block fails the pass. The checkpoint stays where it was, and the next pass retries. It also answers both earlier threads: the fresh head keeps the reads on recent state after a long scan, and it shrinks the window in which a settled burn can outrun a reading.
Overall the change is correct and does what it says. The checkpoint moves only after every pinned read succeeds, and both new Anvil tests check real chain behavior. No reviewer found a blocker. Three minor points remain. Two are about how often the stricter reads fail: reads at the newest block on a load-balanced RPC, and a single read block that gets old during a long balance phase. Both can make startup restart more often, but no funds are at risk. The third is that the new SPEC sentence promises more than the code does for the log scan.
claude-opus-5-5 · high · 11 min
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. 6 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 8 reviews per hour. WalkthroughReceipt backfill now selects an explicit read block: the scanned block when no logs were found, or the later of that block and a fetched chain head when logs were found. Discovery and reconciliation balance reads use this block. A failed read aborts the pass before checkpoint advancement. The specification describes this behavior, and tests cover an unavailable block and burns occurring before and after the pass head. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to A large backfill may repeatedly fail and prevent startup if its RPC node stops serving the selected block during reconciliation. Resolve that deployment risk before merging unless the RPC’s historical-state support is confirmed. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
3c63ab0 to
8f8799d
Compare
|
@rain-marvin review |
|
🔎 Reviewing |
There was a problem hiding this comment.
This PR pins every balanceOf read in the receipt backfill (discovery and reconciliation) to an explicit block: the larger of the block the logs reach and a fresh head less two blocks, fetched after the log scan. Before, the reads went to latest, so an RPC node behind the node that served the logs could read zero for a receipt that just arrived, skip it, and move the checkpoint past it for good. Now such a node fails the pass, the checkpoint stays, and the next pass retries. SPEC.md and two Anvil tests cover the new behavior, and the mock tests get the extra eth_blockNumber call.
Overall read: the change does what it says. The floor at the log head holds on every path, errors surface before the checkpoint moves, and the earlier threads (archive-state window, reconciler at latest, log-scan clamping, settled-burn window) are resolved as the code shows. One minor point: the two-block margin rarely takes effect, because the floor is a head fetched only moments earlier, so a one-block RPC skew can still fail a short pass. This costs availability, not funds. Nothing blocks the merge.
Panel: 9 lanes. The two flash 3.7 lanes ran on composer 2.5 because flash 3.7 was not logged in. Eight lanes found nothing new.
claude-opus-5-5 · high · 10 min
8f8799d to
425af02
Compare
|
@rain-marvin approve |
|
🔎 Verifying that my findings from |
There was a problem hiding this comment.
This PR (RAI-2834) pins the receipt backfill's balanceOf reads to an explicit block that is never below the block the log scan reaches. A lagging RPC node then fails the pass and keeps the checkpoint, instead of reading zero and moving the checkpoint past a receipt for good. After a long scan, the reads move to a fresh head less READ_BLOCK_MARGIN.
The head is still 425af02, the commit of my last review, so no code changed since then and there is no new diff to check.
Earlier findings
READ_BLOCK_MARGINhas no effect after a short scan (src/receipt_inventory/backfill.rs:257): resolved. The const doc, the inline comment and SPEC.md now say the margin applies only after a long scan, and that a backend behind the pass head fails the pass and the next pass or the restart retries. The checkpoint never moves past an unread receipt.
Threads resolved by hand
- Pinned read can restore shares that a settled burn consumed (comment 4178143745): addressed.
read_blockis now fetched after the log scan (backfill.rs:245-255), which was the fix offered, so the window is the margin plus the time to the reads, not the whole scan.reconcile_receiptdocuments the remaining window, and the next pass reconciles the receipt again. - Pinned reads need archive state after a long scan (comment 4178143752): addressed. The reads use a fresh head (
get_block_number()after the scan, floored at the pass head), so they stay inside a full node's recent-state window.
The other threads (reconciler reads at latest, one read block per pass, SPEC scope for the log scan, pooled-RPC skew) were resolved earlier with sound reasons that still hold at this head. Nothing open remains; the PR is good to merge.
claude-opus-5-5 · high · 35 s
|
@CodeRabbit approve |
✅ Action performedComments resolved and changes approved. |
Merge activity
|
425af02 to
ced9d23
Compare
e0a1476 to
359b367
Compare

Receipt inventory backfill now pins each
balanceOfread to the block its logs reach, notlatest. The old behavior let a lagging RPC node read zero for a receipt that just arrived, skip it, and advance the checkpoint past its block permanently. The new behavior fails the pass if the node does not have that block yet, so the checkpoint stays and the next pass retries. At startup, a failed pass stops startup and the service restarts.Closes RAI-2834
Contributes to RAI-2601
Contributes to RAI-1215
Live effect: prevents permanent receipt loss when RPC nodes lag · Risk: high (receipt tracking in money path; startup fails on slow RPC nodes instead of skipping data) · Ships: on merge · Blocks: —
Decisions
latest, so a node without that block fails the call instead of returning stale zero. The pass does not move the checkpoint, and the next pass retries.Risks
Proof
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.