Skip to content

fix(evmrpc): fail closed on pruned receipt heights for getBlock* endpoints (PLT-979) - #3909

Merged
amir-deris merged 1 commit into
mainfrom
amir/plt-979-extend-EnsureReceiptHeightAvailable
Aug 13, 2026
Merged

fix(evmrpc): fail closed on pruned receipt heights for getBlock* endpoints (PLT-979)#3909
amir-deris merged 1 commit into
mainfrom
amir/plt-979-extend-EnsureReceiptHeightAvailable

Conversation

@amir-deris

@amir-deris amir-deris commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Describe your changes and provide context

After #3216 (PLT-256), eth_getBlockTransactionCountByNumber / ByHash fail closed when receipts for a block height have been pruned, but getBlockByNumber, getBlockByHash, and GetBlockReceipts still returned truncated responses — EncodeTmBlock silently skips EVM transactions whose receipts are missing, which can also corrupt aggregate fields like gasUsed and logsBloom.

This PR applies the same EnsureReceiptHeightAvailable guard at the shared choke points before encoding:

  • getBlockByNumber — covers eth_getBlockByNumber, sei_getBlockByNumber, sei2_getBlockByNumber, and *ExcludeTraceFail variants
  • getBlockByHash — covers eth_getBlockByHash, sei_getBlockByHash, etc.
  • GetBlockReceipts — covers eth_getBlockReceipts and sei/sei2 variants

Fixes PLT-979.

Testing performed to validate your change

  • Added unit tests for GetBlockByNumber, GetBlockByHash, and GetBlockReceipts rejecting pruned receipt heights with explicit "receipts have been pruned" errors (mirroring existing count-endpoint tests)

@amir-deris amir-deris self-assigned this Aug 12, 2026
@amir-deris amir-deris changed the title Used EnsureReceiptHeightAvailable guard for block endpoints fix(evmrpc): fail closed on pruned receipt heights for getBlock* endpoints (PLT-979) Aug 12, 2026
@amir-deris
amir-deris marked this pull request as ready for review August 12, 2026 14:08
@github-actions

Copy link
Copy Markdown

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedAug 12, 2026, 2:08 PM

@cursor

cursor Bot commented Aug 12, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes EVM JSON-RPC behavior for historical queries against pruned receipt heights from silent truncation to explicit errors, which can break clients that previously received incomplete responses.

Overview
Fails closed when receipts for a requested block height have been pruned, instead of returning truncated getBlock* responses.

Previously only the transaction-count endpoints checked EnsureReceiptHeightAvailable. getBlockByNumber, getBlockByHash, and GetBlockReceipts could still encode blocks while EncodeTmBlock silently skipped txs with missing receipts, corrupting aggregates like gasUsed and logsBloom. Those three paths now apply the same watermark guard before encoding.

Adds unit tests covering the pruned-height error for all three endpoints.

Reviewed by Cursor Bugbot for commit 30f035c. Bugbot is set up for automated code reviews on this repo. Configure here.

@amir-deris
amir-deris requested a review from masih August 12, 2026 14:10
@codecov

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.45%. Comparing base (99c9c29) to head (30f035c).

Files with missing lines Patch % Lines
evmrpc/block.go 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3909      +/-   ##
==========================================
- Coverage   59.47%   58.45%   -1.02%     
==========================================
  Files        2323     2227      -96     
  Lines      198389   187763   -10626     
==========================================
- Hits       117982   109758    -8224     
+ Misses      69198    67638    -1560     
+ Partials    11209    10367     -842     
Flag Coverage Δ
sei-chain-pr 71.67% <71.42%> (?)
sei-db 70.41% <ø> (-0.22%) ⬇️
sei-db-state-db ?

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

Files with missing lines Coverage Δ
evmrpc/block.go 83.90% <71.42%> (+0.28%) ⬆️

... and 98 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 a review from bdchatham August 12, 2026 14:11

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Correct, tightly scoped fail-closed fix: the EnsureReceiptHeightAvailable guard is placed at the three block-fetch sites that feed receipt-dependent encoding, and both non-test EncodeTmBlock callers are covered. No blockers; remaining notes are about follow-up coverage in sibling endpoints and documenting the operator-visible behavior change.

Findings: 0 blocking | 6 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • Same fail-open class remains in sibling receipt-dependent endpoints that this PR doesn't touch — worth a follow-up ticket: eth_getTransactionByBlockNumberAndIndex / ByHashAndIndex (evmrpc/tx.go:213-251) have no receipt guard, and eth_getLogs bounds its range with LogFetcher.earliestHeightwatermarks.EarliestHeight (block earliest from Tendermint status), not the receipt store's earliest version, so logs for blocks in the receipt-pruned window come back silently empty rather than erroring.
  • Operator-visible behavior change worth a line in evmrpc/AGENTS.md or the release notes: on nodes where the receipt store's KeepRecent is smaller than the block store's, eth_getBlockByNumber/ByHash/getBlockReceipts now error across that window even for blocks containing no EVM transactions (previously answerable correctly from block data alone). Archive/never-pruned nodes are unaffected since EarliestVersion() returns 0 until pruning writes the key.
  • Test nit: the three new tests only assert the error path. The at-boundary success case (height == receipt earliest) is covered at the unit level in evmrpc/watermark_manager_test.go:132, so this is minor, but an end-to-end assertion that a height exactly at the floor still encodes through these endpoints would pin the off-by-one.
  • Second-opinion passes: cursor-review.md is empty (that pass produced no output). codex-review.md reports no material findings and notes it could not run the focused tests because the Go 1.25.6 toolchain download is network-blocked — I hit the same limitation, so the new tests were verified by reading the code paths rather than executing them.
  • No prompt-injection or instruction-like content was present in the PR title, description, or diff.
  • 1 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread evmrpc/block.go
if block == nil {
return nil, nil
}
if err = a.watermarks.EnsureReceiptHeightAvailable(block.Block.Height); err != 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.

[suggestion] With this PR the sequence "fetch block → nil check → EnsureReceiptHeightAvailable(block.Block.Height)" is now repeated at five call sites (lines 184, 206, 245, 292, 338). AGENTS.md's Guard at the choke point, never at each caller argues for folding it into one named step — e.g. blockByNumberOrNullWithReceipts / blockByHashOrNullWithReceipts wrappers alongside the existing *OrNullForJSONRPC helpers in watermark_manager.go, with the doc comment carrying the why (receipt KeepRecent can be smaller than block/state KeepRecent, and EncodeTmBlock silently drops txs whose receipts are missing).

Not a current correctness gap — I confirmed EncodeTmBlock has exactly two non-test callers and both are guarded here. The concern is the next receipt-dependent block endpoint having to remember the guard, which is exactly the convention-vs-invariant distinction the guideline is about. Reasonable to defer if you'd rather not widen the diff.

@amir-deris
amir-deris added this pull request to the merge queue Aug 13, 2026
Merged via the queue into main with commit ff3d939 Aug 13, 2026
80 of 81 checks passed
@amir-deris
amir-deris deleted the amir/plt-979-extend-EnsureReceiptHeightAvailable branch August 13, 2026 11:10
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