-
Notifications
You must be signed in to change notification settings - Fork 20.8k
internal/ethapi: support for beacon root and withdrawals in simulate api #31304
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
Changes from 16 commits
2407255
d71a990
6a21744
cde1056
fa245e2
170f75c
762ad05
ea7217b
6d443ca
2209cb4
26f20b2
cb24543
bddacef
e7b76da
c0f717f
795f3ec
2a99b55
9dae1ba
0ced839
5623a35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,6 @@ import ( | |
"github.com/ethereum/go-ethereum/internal/ethapi/override" | ||
"github.com/ethereum/go-ethereum/params" | ||
"github.com/ethereum/go-ethereum/rpc" | ||
"github.com/ethereum/go-ethereum/trie" | ||
) | ||
|
||
const ( | ||
|
@@ -95,6 +94,47 @@ type simOpts struct { | |
ReturnFullTransactions bool | ||
} | ||
|
||
// simChainHeadReader implements ChainHeaderReader which is needed as input for FinalizeAndAssemble. | ||
type simChainHeadReader struct { | ||
context.Context | ||
Backend | ||
} | ||
|
||
func (m *simChainHeadReader) Config() *params.ChainConfig { | ||
return m.Backend.ChainConfig() | ||
} | ||
|
||
func (m *simChainHeadReader) CurrentHeader() *types.Header { | ||
return m.Backend.CurrentHeader() | ||
} | ||
|
||
func (m *simChainHeadReader) GetHeader(hash common.Hash, number uint64) *types.Header { | ||
header, err := m.Backend.HeaderByNumber(m.Context, rpc.BlockNumber(number)) | ||
if err != nil || header == nil { | ||
return nil | ||
} | ||
if header.Hash() != hash { | ||
return nil | ||
} | ||
return header | ||
} | ||
|
||
func (m *simChainHeadReader) GetHeaderByNumber(number uint64) *types.Header { | ||
header, err := m.Backend.HeaderByNumber(m.Context, rpc.BlockNumber(number)) | ||
if err != nil { | ||
return nil | ||
} | ||
return header | ||
} | ||
|
||
func (m *simChainHeadReader) GetHeaderByHash(hash common.Hash) *types.Header { | ||
header, err := m.Backend.HeaderByHash(m.Context, hash) | ||
if err != nil { | ||
return nil | ||
} | ||
return header | ||
} | ||
|
||
// simulator is a stateful object that simulates a series of blocks. | ||
// it is not safe for concurrent use. | ||
type simulator struct { | ||
|
@@ -209,6 +249,9 @@ func (sim *simulator) processBlock(ctx context.Context, block *simBlock, header, | |
if sim.chainConfig.IsPrague(header.Number, header.Time) || sim.chainConfig.IsVerkle(header.Number, header.Time) { | ||
core.ProcessParentBlockHash(header.ParentHash, evm) | ||
} | ||
if block.BlockOverrides.BeaconRoot != nil { | ||
core.ProcessBeaconBlockRoot(*block.BlockOverrides.BeaconRoot, evm) | ||
} | ||
var allLogs []*types.Log | ||
for i, call := range block.Calls { | ||
if err := ctx.Err(); err != nil { | ||
|
@@ -258,6 +301,10 @@ func (sim *simulator) processBlock(ctx context.Context, block *simBlock, header, | |
} | ||
callResults[i] = callRes | ||
} | ||
header.GasUsed = gasUsed | ||
if sim.chainConfig.IsCancun(header.Number, header.Time) { | ||
header.BlobGasUsed = &blobGasUsed | ||
} | ||
var requests [][]byte | ||
// Process EIP-7685 requests | ||
if sim.chainConfig.IsPrague(header.Number, header.Time) { | ||
|
@@ -271,20 +318,16 @@ func (sim *simulator) processBlock(ctx context.Context, block *simBlock, header, | |
// EIP-7251 | ||
core.ProcessConsolidationQueue(&requests, evm) | ||
} | ||
header.Root = sim.state.IntermediateRoot(true) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update: Root Calculation is now done in |
||
header.GasUsed = gasUsed | ||
if sim.chainConfig.IsCancun(header.Number, header.Time) { | ||
header.BlobGasUsed = &blobGasUsed | ||
} | ||
Comment on lines
-275
to
-278
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. gas and blob setting moved up, i.e. closer to where |
||
var withdrawals types.Withdrawals | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. withdrawals are now managed in |
||
if sim.chainConfig.IsShanghai(header.Number, header.Time) { | ||
withdrawals = make([]*types.Withdrawal, 0) | ||
} | ||
if requests != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was moved up, closer where requests is calculated |
||
reqHash := types.CalcRequestsHash(requests) | ||
header.RequestsHash = &reqHash | ||
} | ||
b := types.NewBlock(header, &types.Body{Transactions: txes, Withdrawals: withdrawals}, receipts, trie.NewStackTrie(nil)) | ||
blockBody := &types.Body{Transactions: txes, Withdrawals: *block.BlockOverrides.Withdrawals} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can panic that's why tests are failing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you - have addressed this by checking withdrawals in |
||
chainHeadReader := &simChainHeadReader{ctx, sim.b} | ||
b, err := sim.b.Engine().FinalizeAndAssemble(chainHeadReader, header, sim.state, blockBody, receipts) | ||
if err != nil { | ||
return nil, nil, err | ||
} | ||
repairLogs(callResults, b.Hash()) | ||
return b, callResults, nil | ||
} | ||
|
@@ -346,6 +389,9 @@ func (sim *simulator) sanitizeChain(blocks []simBlock) ([]simBlock, error) { | |
n := new(big.Int).Add(prevNumber, big.NewInt(1)) | ||
block.BlockOverrides.Number = (*hexutil.Big)(n) | ||
} | ||
if block.BlockOverrides.Withdrawals == nil { | ||
block.BlockOverrides.Withdrawals = &types.Withdrawals{} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Noting that we do an empty slice rather than nil, which semantically is fine as checks in consensus are done as length checks rather than nil checks. if len(body.Withdrawals) > 0 {
return nil, errors.New("clique does not support withdrawals")
} |
||
} | ||
diff := new(big.Int).Sub(block.BlockOverrides.Number.ToInt(), prevNumber) | ||
if diff.Cmp(common.Big0) <= 0 { | ||
return nil, &invalidBlockNumberError{fmt.Sprintf("block numbers must be in order: %d <= %d", block.BlockOverrides.Number.ToInt().Uint64(), prevNumber)} | ||
|
@@ -360,7 +406,13 @@ func (sim *simulator) sanitizeChain(blocks []simBlock) ([]simBlock, error) { | |
for i := uint64(0); i < gap.Uint64(); i++ { | ||
n := new(big.Int).Add(prevNumber, big.NewInt(int64(i+1))) | ||
t := prevTimestamp + timestampIncrement | ||
b := simBlock{BlockOverrides: &override.BlockOverrides{Number: (*hexutil.Big)(n), Time: (*hexutil.Uint64)(&t)}} | ||
b := simBlock{ | ||
BlockOverrides: &override.BlockOverrides{ | ||
Number: (*hexutil.Big)(n), | ||
Time: (*hexutil.Uint64)(&t), | ||
Withdrawals: &types.Withdrawals{}, | ||
}, | ||
} | ||
prevTimestamp = t | ||
res = append(res, b) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noting that there doesn't seem to be a clear standard as to whether to ignore hash or number, so going with hash instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HeaderByHash
can also return non-canonical blocks. That's the reason forGetHeader
existing. It asks for a block at a particular height with a assumed hash.So we can improve this here by doing HeaderByNumber first and double-checking the hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apppreciate the additional context. I've updated as suggested