Include receipt store earliest height in watermark bounds to prevent silent undercount on pruned blocks#3216
Open
amir-deris wants to merge 6 commits intomainfrom
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3216 +/- ##
==========================================
- Coverage 59.02% 58.96% -0.06%
==========================================
Files 2065 2062 -3
Lines 169414 168992 -422
==========================================
- Hits 99994 99650 -344
+ Misses 60673 60627 -46
+ Partials 8747 8715 -32
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
bdchatham
reviewed
Apr 9, 2026
Comment on lines
+231
to
+233
| if earliest <= 0 { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Trying to wrap my head around this. Is there something special about this? Is it not generalized in the if-statement below it?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
There was a fix implemented in PR 3125 in order to make
eth_getBlockTransactionCountByNumber == len(eth_getBlockByNumber.transactions)by using same filtering mechanism for evm transactions to consider availability of receipts. That fix raised the following concern:This change makes GetBlockTransactionCountByNumber and GetBlockTransactionCountByHash results dependent on the receipt store due to the receipt query leading to inconsistencies in historical results.
Those functions do ensure that the block is available based on watermarks. However, the receipt store does not contribute to the earliest height in the watermark manager, only the latest. So even if EnsureBlockHeightAvailable returns that the block is available that does not guarantee all receipts for that height are available. This will occur in the cases when ReceiptStore's KeepRecent is less than the StateStore's KeepRecent and MinRetainBlocks. In such cases the returned transaction count may diverge from the true count due to pruned receipts.
Analysis
eth_getBlockTransactionCountByNumberandeth_getBlockTransactionCountByHashsilently return an incorrect (lower) transaction count for historical blocks when the receipt store has been pruned further back than the block or state stores.The watermark check in
EnsureBlockHeightAvailableusesblockEarliest(derived from Tendermint) to gate requests, but the receipt store's earliest version never contributed to that bound. So a block could pass the availability check while its receipts were already pruned — causingcountBlockTxsLikeEncodeTmBlockto silently skip EVM transactions whose receipts are missing rather than returning an error.This scenario occurs when
ReceiptStoreConfig.KeepRecentis smaller thanStateStoreConfig.KeepRecent/MinRetainBlocks.Fix
EarliestVersion() int64to theReceiptStoreinterface and implemented it across all backends (receiptStore/pebble,parquetReceiptStore,cachedReceiptStore) and the underlyingparquet.Store.WatermarkManager.EnsureReceiptHeightAvailable— a targeted check that returns an explicit "receipts have been pruned" error when the requested height is below the receipt store's earliest retained version. Returnsnilwhen no receipt store is configured or no pruning has occurred.EnsureReceiptHeightAvailablein bothGetBlockTransactionCountByNumberandGetBlockTransactionCountByHashafter block resolution, so callers receive a clear error instead of a silently wrong count.Result
eth_getBlockByNumber)Fixes: PLT-256