fix(l1): flush frame receipts with the storage codec - #7082
fix(l1): flush frame receipts with the storage codec#7082AnkushinDaniil wants to merge 1 commit into
Conversation
Greptile SummaryThe PR corrects deferred receipt persistence by using the storage codec expected by RECEIPTS_V2 readers.
Confidence Score: 5/5The PR appears safe to merge and fixes the frame-receipt persistence mismatch without introducing a conflicting storage path. The changed writer now uses the codec consumed by every live RECEIPTS_V2 read path, while the regression test synchronously evicts the buffered receipt and verifies the persisted frame receipt round-trips from backend storage.
|
| Filename | Overview |
|---|---|
| crates/storage/store.rs | Aligns deferred receipt writes with the storage decoder and the existing direct receipt writers. |
| test/tests/storage/deferred_persistence_tests.rs | Adds a valid disk-path regression test that verifies a frame receipt survives flush, eviction, canonical lookup, and storage decoding. |
Sequence Diagram
sequenceDiagram
participant Worker as Persist worker
participant Buffer as BlockDataBuffer
participant DB as RECEIPTS_V2
participant Reader as Receipt reader
Worker->>Buffer: Select flushable block
Worker->>DB: Write receipt.encode_storage()
Worker->>Buffer: Evict after commit
Reader->>Buffer: Lookup receipt
Buffer-->>Reader: Miss after eviction
Reader->>DB: Read persisted bytes
DB-->>Reader: Receipt::decode_storage()
Reviews (1): Last reviewed commit: "fix(l1): flush frame receipts with the s..." | Re-trigger Greptile
There was a problem hiding this comment.
Pull request overview
This PR fixes a receipt persistence bug in the storage flush path where receipts were written using the consensus/wire codec, while the read path (and other writers) expect the storage codec—causing EIP-8141 frame receipts to become undecodable after flush.
Changes:
- Persist flushed receipts using
Receipt::encode_storage()to match the read/other write paths. - Add a regression test covering a flushed frame receipt roundtrip through
store_block_updates+ forced flush +get_receipt.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/tests/storage/deferred_persistence_tests.rs | Adds a regression test ensuring frame receipts remain readable after the buffer flushes to disk. |
| crates/storage/store.rs | Switches receipt encoding during block-data flush from consensus (encode_to_vec) to storage (encode_storage). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ElFantasma
left a comment
There was a problem hiding this comment.
Correct and now consistent with all four other receipt codec sites; one non-blocking note about receipts already written by the old path.
| RECEIPTS_V2, | ||
| &receipt_key(&hash, index as u64), | ||
| &receipt.encode_to_vec(), | ||
| &receipt.encode_storage(), |
There was a problem hiding this comment.
Verified this was the only outlier — add_receipt (:836), add_receipts (:851) and both read paths (:883, :1385) already use the storage codec, so this line makes all five agree.
One thing the fix can't do by itself: frame receipts already flushed by the old path stay unreadable. They're on disk in the consensus encoding, and after this change the reader still calls decode_storage, so they keep failing with the same MalformedBoolean. Anyone running a Hegota devnet node from before this lands will still see eth_getTransactionReceipt return null for those blocks.
I don't think that warrants a migration or a STORE_SCHEMA_VERSION bump. Non-frame receipts encode identically under both codecs, so nothing pre-Hegota is affected, and frame receipts only exist on an unreleased fork — a resync is the proportionate remedy for the handful of devnet databases involved. But it's worth a line in the PR description so whoever runs those nodes knows the fix is forward-only, since the symptom (null receipts) is identical before and after and looks like the bug simply wasn't fixed.
Worth double-checking one thing I couldn't confirm from the diff: whether any devnet node is expected to keep its database across this change. If the answer is yes for a long-lived one, the calculus shifts toward a migration that re-encodes RECEIPTS_V2 entries whose tx_type is Frame.
There was a problem hiding this comment.
No node keeps a database across this. The frame devnets are rebuilt from genesis on every run, so every affected database is throwaway and a resync costs nothing. The description now says the fix is forward-only. If a long-lived node appears later, re-encoding RECEIPTS_V2 entries whose tx_type is Frame is the migration, and it can land separately without touching this.
7a4ff51 to
bea20c4
Compare
The block-data buffer flush wrote receipts with the consensus codec while every other writer and the read path use the storage codec. For non-frame receipts the two agree, so this was invisible. A frame receipt's consensus layout omits `succeeded` and leads with `cumulative_gas_used`, so `decode_storage` reads a gas value as a bool and fails with `MalformedBoolean`: once a block leaves the buffer, every frame transaction in it loses its receipt. Found on a two-client devnet, where `eth_getTransactionReceipt` returned null on ethrex for a frame transaction both clients had in the same block.
bea20c4 to
785bfe0
Compare
Motivation
The block-data buffer flush writes receipts with the consensus codec (
encode_to_vec), whileadd_receipt,add_receiptsand the read path (get_receipt_by_block_hash) all use the storage codec. For non-frame receipts the two encodings coincide, so the mismatch never showed.An EIP-8141 frame receipt is different: its consensus layout omits the top-level
succeededand leads withcumulative_gas_used.decode_storagetherefore reads a gas value where it expects a bool and fails withMalformedBoolean. The receipt is served correctly from the buffer and becomes permanently unreadable the moment the block is flushed.Description
One line: the flush path now uses
encode_storage(), matching every other writer and the reader.How was it found
On a Nethermind + ethrex devnet. Both clients agreed on block 69 byte for byte — same block hash, state root, receipts root and block access list hash — and both served
eth_getTransactionByHashfor the frame transaction in it, buteth_getTransactionReceiptreturnednullon ethrex.eth_getBlockReceiptssurfaced the real cause:Tests
flushed_frame_receipt_survives_the_storage_codecstores a frame receipt throughstore_block_updates, forces a flush so the buffer can no longer answer, and reads it back. Verified in both directions: it fails withMalformedBooleanon the previous line and passes with the fix.The fix is forward-only. Frame receipts already written by the old path stay on disk in the consensus encoding and keep failing to decode, so
eth_getTransactionReceiptstill returns null for those blocks until the node resyncs. Non-frame receipts encode identically under both codecs, so nothing before Hegota is affected.