Skip to content

evmrpc: refactor getHeader to always require tmBlock, fix CurrentHeader block fetch#3274

Open
amir-deris wants to merge 2 commits intomainfrom
amir/plt-275-refactor-simulate-currentHeader
Open

evmrpc: refactor getHeader to always require tmBlock, fix CurrentHeader block fetch#3274
amir-deris wants to merge 2 commits intomainfrom
amir/plt-275-refactor-simulate-currentHeader

Conversation

@amir-deris
Copy link
Copy Markdown
Contributor

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

Summary

  • getHeader previously accepted an optional blockNumber *big.Int and a nullable tmBlock, doing its own block fetch when tmBlock was nil. This PR removes that dual path: getHeader now always receives a non-nil tmBlock and derives the height from it directly.
  • CurrentHeader is updated to accept a context.Context, fetch the Tendermint block itself, and call getHeader with it. A new fallbackEthereumHeaderWithoutTendermintBlock helper handles the error path (block RPC unavailable).
  • header.Time now uses the actual block timestamp (tmBlock.Block.Time) instead of time.Now(), and header.ParentHash is always populated since tmBlock is never nil.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 20, 2026

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 20, 2026, 7:02 PM

@amir-deris amir-deris changed the title Removed block number from simulate/getHeader evmrpc: refactor getHeader to always require tmBlock, fix CurrentHeader block fetch Apr 20, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.30%. Comparing base (9225538) to head (102f1d6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3274   +/-   ##
=======================================
  Coverage   59.30%   59.30%           
=======================================
  Files        2071     2071           
  Lines      169814   169816    +2     
=======================================
+ Hits       100707   100709    +2     
  Misses      60333    60333           
  Partials     8774     8774           
Flag Coverage Δ
sei-chain-pr 64.41% <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/simulate.go 74.06% <100.00%> (+0.12%) ⬆️
🚀 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 20, 2026 19:58
Comment thread evmrpc/simulate.go
func (b *Backend) CurrentHeader() *ethtypes.Header {
header := b.getHeader(context.Background(), big.NewInt(b.ctxProvider(LatestCtxHeight).BlockHeight()), nil)
height := b.ctxProvider(LatestCtxHeight).BlockHeight()
ctx := context.Background()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Out of scope for this PR, but a first glance this seems off but something to look into later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is a good point. I actually tried passing in context as an argument for CurrentHeader, but the issue is the Backend is supposed to implement the interface in go ethereum:
https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/backend.go#L64
and passing an extra argument breaks that contract.

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