Skip to content

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
amir/plt-256-include-receipt-store-earliest-height-for-watermark-bounds
Open

Include receipt store earliest height in watermark bounds to prevent silent undercount on pruned blocks#3216
amir-deris wants to merge 6 commits intomainfrom
amir/plt-256-include-receipt-store-earliest-height-for-watermark-bounds

Conversation

@amir-deris
Copy link
Copy Markdown
Contributor

@amir-deris amir-deris commented Apr 8, 2026

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_getBlockTransactionCountByNumber and eth_getBlockTransactionCountByHash silently 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 EnsureBlockHeightAvailable uses blockEarliest (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 — causing countBlockTxsLikeEncodeTmBlock to silently skip EVM transactions whose receipts are missing rather than returning an error.

This scenario occurs when ReceiptStoreConfig.KeepRecent is smaller than StateStoreConfig.KeepRecent / MinRetainBlocks.

Fix

  • Added EarliestVersion() int64 to the ReceiptStore interface and implemented it across all backends (receiptStore/pebble, parquetReceiptStore, cachedReceiptStore) and the underlying parquet.Store.
  • Added 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. Returns nil when no receipt store is configured or no pruning has occurred.
  • Called EnsureReceiptHeightAvailable in both GetBlockTransactionCountByNumber and GetBlockTransactionCountByHash after block resolution, so callers receive a clear error instead of a silently wrong count.

Result

  • Within receipt retention window: correct count (consistent with eth_getBlockByNumber)
  • Outside receipt retention window: explicit pruned error (instead of silent undercount)

Fixes: PLT-256

@amir-deris amir-deris self-assigned this Apr 8, 2026
@amir-deris amir-deris changed the title added earliest version to receipt store Include receipt store earliest height in watermark bounds to prevent silent undercount on pruned blocks Apr 8, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 9, 2026, 4:04 PM

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.96%. Comparing base (8d7e329) to head (20438fe).

Additional details and impacted files

Impacted file tree graph

@@            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     
Flag Coverage Δ
sei-chain-pr 66.63% <100.00%> (?)
sei-db 70.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
evmrpc/block.go 82.37% <100.00%> (+0.21%) ⬆️
evmrpc/watermark_manager.go 76.00% <100.00%> (+1.53%) ⬆️
sei-db/ledger_db/parquet/store.go 69.67% <100.00%> (+1.35%) ⬆️
sei-db/ledger_db/receipt/cached_receipt_store.go 86.14% <100.00%> (+0.16%) ⬆️
sei-db/ledger_db/receipt/parquet_store.go 69.51% <100.00%> (+2.85%) ⬆️
sei-db/ledger_db/receipt/receipt_store.go 75.00% <100.00%> (+0.25%) ⬆️

... and 40 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@amir-deris amir-deris requested review from bdchatham and masih April 9, 2026 16:16
Comment on lines +231 to +233
if earliest <= 0 {
return nil
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to wrap my head around this. Is there something special about this? Is it not generalized in the if-statement below it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants