Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/ethapi: support for beacon root and withdrawals in simulate api #31304

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rezbera
Copy link

@rezbera rezbera commented Mar 1, 2025

Addresses #31264

@rezbera rezbera requested review from fjl, s1na and lightclient as code owners March 1, 2025 20:59
@@ -267,20 +276,22 @@ func (sim *simulator) processBlock(ctx context.Context, block *simBlock, header,
// EIP-7251
core.ProcessConsolidationQueue(&requests, evm)
}
header.Root = sim.state.IntermediateRoot(true)
Copy link
Author

Choose a reason for hiding this comment

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

Root calculation moved to the end, i.e. after withdrawals since they change state.

Comment on lines 271 to 274
header.GasUsed = gasUsed
if sim.chainConfig.IsCancun(header.Number, header.Time) {
header.BlobGasUsed = &blobGasUsed
}
Copy link
Author

Choose a reason for hiding this comment

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

gas and blob setting moved up, i.e. closer to where gasUsed and blobGasUsed are last used

@rezbera rezbera marked this pull request as draft March 1, 2025 21:17
Comment on lines +292 to +295
if block.BlockOverrides.Withdrawals != nil {
// We assume that if the user provides Withdrawals, we're operating on Beacon consensus
// which can have nil ChainHeadReader
sim.b.Engine().Finalize(nil, header, sim.state, blockBody)
Copy link
Author

Choose a reason for hiding this comment

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

Is there a better way to do this? e.g. create a ChainHeadReader (seems non-trivial)

@rezbera rezbera marked this pull request as ready for review March 1, 2025 21:39
Update simulate.go

Update simulate.go

Update simulate.go

Update consensus.go

passed withdrawals - now state root mismartch

working but withdrawals issue

Added support
@rezbera rezbera force-pushed the enhance-simulation-api branch from fb1dd96 to 2407255 Compare March 1, 2025 21:40
}
if requests != nil {
Copy link
Author

Choose a reason for hiding this comment

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

This was moved up, closer where requests is calculated

@phearnot
Copy link

phearnot commented Mar 3, 2025

Hey @rezbera, we also need withdrawal support in simulateV1 for our project. I've tweaked it like so. My idea was to handle withdrawals in the same way as they are handled in the "real" engine. State root hash is adjusted when address balance changes, so there's no need to manually change it.
I've run some smoke tests against my patch. I was able to pass the block with withdrawals generated by simulateV1 to newPayload/forkchoiceUpdated, and everything seemed to work fine.

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.

2 participants