Skip to content

fix(rpc): align eth_getBlockReceipts response with execution-apis spec#965

Merged
songgaoye merged 4 commits into
developfrom
fix/rpc/eth_getBlockReceipts
Jun 3, 2026
Merged

fix(rpc): align eth_getBlockReceipts response with execution-apis spec#965
songgaoye merged 4 commits into
developfrom
fix/rpc/eth_getBlockReceipts

Conversation

@XinyuCRO
Copy link
Copy Markdown

@XinyuCRO XinyuCRO commented May 26, 2026

  1. Accept a block number, hash, or tag. The method signature changes from BlockNumber to BlockNumberOrHash across the EthereumAPI/EVMBackend interfaces and PublicAPI. A new tendermintBlockByNumberOrHash helper resolves a hash (via endermintBlockByHash), a number (via TendermintBlockByNumber), or an empty argument (defaults to latest). Previously only a bare block number was accepted, so spec-compliant clients passing a block hash or tag failed.

  2. Support the safe block tag. BlockNumberOrHash.decodeFromString now maps safe (alongside latest/finalized) to the latest height; previously safe errored.

  3. Propagate block-lookup errors. GetBlockReceipts no longer swallows a failed block lookup with return nil, nil — it
    now returns the underlying error, so RPC callers see a real failure instead of an empty result.

  4. Correct effectiveGasPrice for EIP-1559-style txs. For dynamic-fee, blob, and set-code (EIP-1559/4844/7702)
    transactions, effectiveGasPrice is now omitted when baseFee is unavailable (pruned node or disabled fee market) rather than emitting GasFeeCap, which is the max fee and not the effective price. Legacy txs continue to report
    effectiveGasPrice as before (the previously commented-out integration-test expectation is re-enabled).

  5. Populate blob fields for blob txs. Blob (EIP-4844) transactions now include blobGasUsed and blobGasPrice in the
    receipt (currently 0, as Ethermint does not yet execute blob-gas accounting).

Comment thread CHANGELOG.md Outdated
@XinyuCRO
Copy link
Copy Markdown
Author

@claude review PR

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 26, 2026

Claude finished @XinyuCRO's task in 3m 24s —— View job


PR Review: fix(rpc): align eth_getBlockReceipts response with execution-apis spec

Tasks:

  • Read CLAUDE.md for repo guidelines
  • Analyze PR diff and changed files
  • Review code quality, correctness, and best practices
  • Check for breaking changes
  • Post inline comments for specific issues
  • Finalize review summary

Summary

This PR aligns eth_getBlockReceipts with the execution-apis spec by:

  1. Accepting BlockNumberOrHash instead of just BlockNumber (supports querying by block hash or tag like "safe")
  2. Adding effectiveGasPrice for all transaction types (previously only for DynamicFeeTxType)
  3. Adding blobGasUsed/blobGasPrice fields for BlobTxType transactions
  4. Fixing "safe" tag handling in BlockNumberOrHash.decodeFromString

No consensus/state-breaking changes detected. This is a JSON-RPC layer change only — it does not affect on-chain state, message signing, Amino JSON encoding, or module logic.


Assessment: Mostly Good ✅

The core approach is correct and well-structured. The interface change is clean and the new helper tendermintBlockByNumberOrHash is a sensible factoring. Tests are updated appropriately.


Issues Found

Minor / Nits

  • CHANGELOG.md — The author's own suggestion in the review correctly adds a missing space before fix(rpc):; that's the only formatting issue.
  • rpc/backend/blocks.go:138 — The silent fallback to EthLatestBlockNumber when both BlockHash and BlockNumber are nil is defensively fine, but undocumented. See inline comment.
  • rpc/backend/tx_info.go:477 — Blob gas fields are hardcoded to 0. Ethermint doesn't support EIP-4844, so this is safe today, but no comment explains why. See inline comment.
  • rpc/backend/tx_info.go:488 — Pre-existing dereference of GetEffectiveGasPrice return value without a nil check. Safe as long as go-ethereum's GasPrice() is non-nil, but worth noting. See inline comment.
  • rpc/backend/tx_info_test.go:935TestGetBlockReceipts_ByHash uses a zero hash that matches the hardcoded mock. Fragile test but works. See inline comment.

Spec compliance note: The execution-apis spec also requires effectiveGasPrice for EIP-2930 AccessList transactions (type: 0x1). The PR handles DynamicFeeTxType and BlobTxType but silently falls into the else branch (GetEffectiveGasPrice(nil), i.e. just gasPrice) for AccessListTxType. This is actually correct behavior — for EIP-2930 txs, there is no baseFee adjustment and gasPrice == effectiveGasPrice. Just worth confirming this is intentional. Fix or clarify →


No Breaking Changes

  • RPC interface signature changes (BlockNumberBlockNumberOrHash) are backward-compatible from a caller perspective: string block tags and hex block numbers continue to work; block-hash lookups are newly supported.
  • No changes to consensus, module state, Amino serialization, or tx structure.

| View PR branch

Comment thread CHANGELOG.md Outdated
Comment thread rpc/backend/tx_info.go
Comment thread rpc/backend/tx_info.go Outdated
Comment thread rpc/backend/blocks.go
Comment thread rpc/backend/tx_info_test.go Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 71.87500% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.38%. Comparing base (a947b22) to head (82aec0c).

Files with missing lines Patch % Lines
rpc/backend/tx_info.go 68.75% 3 Missing and 2 partials ⚠️
rpc/namespaces/ethereum/eth/api.go 0.00% 3 Missing ⚠️
rpc/backend/blocks.go 91.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #965      +/-   ##
===========================================
+ Coverage    45.30%   45.38%   +0.08%     
===========================================
  Files          197      197              
  Lines        17052    17072      +20     
===========================================
+ Hits          7725     7748      +23     
+ Misses        8511     8508       -3     
  Partials       816      816              
Files with missing lines Coverage Δ
rpc/backend/backend.go 77.77% <ø> (ø)
rpc/types/block.go 52.74% <100.00%> (ø)
rpc/backend/blocks.go 79.82% <91.66%> (+1.06%) ⬆️
rpc/namespaces/ethereum/eth/api.go 0.00% <0.00%> (ø)
rpc/backend/tx_info.go 61.76% <68.75%> (+1.41%) ⬆️
🚀 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.

@XinyuCRO XinyuCRO changed the title fix(rpc): align eth_getBlockReceipts response with execution-apis s… fix(rpc): align eth_getBlockReceipts response with execution-apis spec May 28, 2026
@XinyuCRO XinyuCRO marked this pull request as ready for review May 28, 2026 08:42
@XinyuCRO XinyuCRO requested a review from a team as a code owner May 28, 2026 08:42
@XinyuCRO XinyuCRO requested review from ApacCronos and randy-cro and removed request for a team May 28, 2026 08:42
Comment thread rpc/backend/tx_info.go
@songgaoye
Copy link
Copy Markdown

LGTM, need to fix When BaseFee() returns nil for a fee-market tx (whether due to pruning or fee market being disabled), the receipt should not contain effectiveGasPrice

@songgaoye
Copy link
Copy Markdown

@claude

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

Claude finished @songgaoye's task in 2m 42s —— View job


Tasks

  • Analyze the request
  • Read rpc/backend/tx_info.go to understand the issue
  • Implement fix: omit effectiveGasPrice when baseFee is nil for fee-market txs
  • Add test: TestBuildReceiptDirect_FeeMarketTxOmitsEffectiveGasPriceWhenBaseFeeUnavailable
  • Commit changes
  • Push to remote — blocked (403: github-actions[bot] lacks write access to this fork)

Fix: Omit effectiveGasPrice for fee-market txs when baseFee unavailable

Files changed:

  • rpc/backend/tx_info.go — conditional effectiveGasPrice assignment
  • rpc/backend/tx_info_test.go — new test for pruned-node scenario

rpc/backend/tx_info.go (lines ~482-501)

Before:

var baseFee *big.Int
if txData.Type() == ethtypes.DynamicFeeTxType || ... {
    baseFee, err = b.BaseFee(blockResults)
    if err != nil {
        b.logger.Error(...)
        baseFee = nil
    }
}
effectiveGasPrice := ethMsg.GetEffectiveGasPrice(baseFee)
if effectiveGasPrice == nil {
    return nil, errorsmod.Wrap(...)
}
receipt["effectiveGasPrice"] = hexutil.Big(*effectiveGasPrice)

After:

isFeeMarketTx := txData.Type() == ethtypes.DynamicFeeTxType || txData.Type() == ethtypes.BlobTxType || txData.Type() == ethtypes.SetCodeTxType
var baseFee *big.Int
if isFeeMarketTx {
    baseFee, err = b.BaseFee(blockResults)
    if err != nil {
        b.logger.Error("fetch basefee failed, node is pruned?", ...)
    }
}

// For fee-market txs, omit effectiveGasPrice when baseFee is unavailable (pruned node).
// For legacy/access-list txs, gasPrice == effectiveGasPrice so no baseFee is needed.
if !isFeeMarketTx || baseFee != nil {
    effectiveGasPrice := ethMsg.GetEffectiveGasPrice(baseFee)
    if effectiveGasPrice == nil {
        return nil, errorsmod.Wrap(errortypes.ErrLogic, "effective gas price is nil")
    }
    receipt["effectiveGasPrice"] = hexutil.Big(*effectiveGasPrice)
}

Behavior summary:

  • Legacy txs (type 0) and EIP-2930 access-list txs (type 1): effectiveGasPrice always included (gasPrice == effectiveGasPrice, no baseFee dependency)
  • Fee-market txs (DynamicFee/Blob/SetCode): effectiveGasPrice included only when baseFee is available; omitted when BaseFee() errors (pruned node / fee market disabled)

Note: The commit is local but push is blocked — github-actions[bot] does not have write permission to this fork repository. The change needs to be pushed manually or the bot needs write access granted.
| View PR branch

@songgaoye
Copy link
Copy Markdown

@claude

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

Claude finished @songgaoye's task in 1m 25s —— View job


Tasks

  • Analyze the request
  • Check current state of the PR branch
  • Review changed files
  • Post final status

Status Review

The fix requested by @songgaoye (omit effectiveGasPrice from EIP-1559 receipts when baseFee is unavailable) has been implemented in commit ce1ed17 and is now on the branch.

rpc/backend/tx_info.go — effectiveGasPrice fix

The implementation is correct:

isEIP1559Tx := txData.Type() == ethtypes.DynamicFeeTxType ||
    txData.Type() == ethtypes.BlobTxType ||
    txData.Type() == ethtypes.SetCodeTxType
if isEIP1559Tx {
    baseFee, err = b.BaseFee(blockResults)
    if err != nil {
        b.logger.Error("fetch basefee failed, node is pruned?", ...)
        baseFee = nil
    }
}
// Omit effectiveGasPrice for EIP-1559 txs when baseFee is unavailable
if !isEIP1559Tx || baseFee != nil {
    effectiveGasPrice := ethMsg.GetEffectiveGasPrice(baseFee)
    ...
    receipt["effectiveGasPrice"] = hexutil.Big(*effectiveGasPrice)
}

Behavior:

  • Legacy/EIP-2930 txs (type 0/1): effectiveGasPrice always included (gasPrice == effectiveGasPrice, no baseFee needed)
  • EIP-1559/Blob/SetCode txs (type 2/3/4): effectiveGasPrice included only when baseFee is available; omitted otherwise

All Original Issues Addressed

Issue Status
BlockNumberOrHash instead of BlockNumber ✅ Done
effectiveGasPrice for all tx types ✅ Done
blobGasUsed/blobGasPrice for BlobTx ✅ Done (with explanatory comment)
"safe" tag handling in decodeFromString ✅ Done
Block lookup error propagated (not swallowed) ✅ Done
Omit effectiveGasPrice when baseFee unavailable ✅ Done (latest commit)

Tests Added

  • TestGetBlockReceipts_ByHash — validates hash-based lookup path
  • TestGetBlockReceipts_EmptyBlockNumberOrHashDefaultsToLatest — validates empty input fallback
  • TestBuildReceiptDirect_SetCodeTxEffectiveGasPrice — validates SetCode tx effectiveGasPrice
  • TestBuildReceiptDirect_EIP1559_NilBaseFee — validates omission when baseFee unavailable
  • TestGetBlockReceipts_BlockLookupError — validates error propagation

No Breaking Changes

This is a JSON-RPC layer change only — no consensus, state, Amino, or module changes.


The PR looks ready to merge. All review feedback has been addressed.
| View PR branch

@songgaoye songgaoye requested a review from thomas-nguy June 3, 2026 02:25
@songgaoye songgaoye enabled auto-merge (squash) June 3, 2026 03:34
@thomas-nguy thomas-nguy force-pushed the fix/rpc/eth_getBlockReceipts branch from 9883477 to 82aec0c Compare June 3, 2026 03:39
@songgaoye songgaoye merged commit eecd6b9 into develop Jun 3, 2026
41 of 42 checks passed
@songgaoye songgaoye deleted the fix/rpc/eth_getBlockReceipts branch June 3, 2026 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants