From 1750edce4fd76ce14a4ca7ff112153fdeb676b3a Mon Sep 17 00:00:00 2001 From: protolambda Date: Fri, 6 Dec 2024 00:46:12 +0100 Subject: [PATCH] all: minor OP-Stack Isthmus withdrawals-root fixes --- beacon/engine/types.go | 9 +++++++-- consensus/beacon/consensus.go | 1 + consensus/clique/clique.go | 2 +- consensus/ethash/consensus.go | 2 +- core/block_validator.go | 2 +- core/chain_makers.go | 5 +++-- core/genesis.go | 13 ++++++------- core/rlp_test.go | 9 +++++---- core/types/block.go | 2 +- eth/api_debug.go | 3 +++ eth/downloader/queue.go | 2 +- 11 files changed, 30 insertions(+), 20 deletions(-) diff --git a/beacon/engine/types.go b/beacon/engine/types.go index 0bc1404a53..c50b1bcdd0 100644 --- a/beacon/engine/types.go +++ b/beacon/engine/types.go @@ -275,8 +275,13 @@ func ExecutableDataToBlockNoHash(data ExecutableData, versionedHashes []common.H // ExecutableData before withdrawals are enabled by marshaling // Withdrawals as the json null value. var withdrawalsRoot *common.Hash - if config.IsOptimismIsthmus(data.Timestamp) && data.WithdrawalsRoot == nil { - return nil, fmt.Errorf("attribute WithdrawalsRoot is required for Isthmus blocks") + if config.IsOptimismIsthmus(data.Timestamp) { + if data.WithdrawalsRoot == nil { + return nil, fmt.Errorf("attribute WithdrawalsRoot is required for Isthmus blocks") + } + if data.Withdrawals == nil || len(data.Withdrawals) > 0 { + return nil, fmt.Errorf("expected non-nil empty withdrawals operation list in Isthmus, but got: %v", data.Withdrawals) + } } if data.WithdrawalsRoot != nil { h := *data.WithdrawalsRoot // copy, avoid any sharing of memory diff --git a/consensus/beacon/consensus.go b/consensus/beacon/consensus.go index 8b39cdd4c1..5200289a5c 100644 --- a/consensus/beacon/consensus.go +++ b/consensus/beacon/consensus.go @@ -410,6 +410,7 @@ func (beacon *Beacon) FinalizeAndAssemble(chain consensus.ChainHeaderReader, hea // State-root has just been computed, we can get an accurate storage-root now. h := state.GetStorageRoot(params.OptimismL2ToL1MessagePasser) header.WithdrawalsHash = &h + state.AccessEvents().AddAccount(params.OptimismL2ToL1MessagePasser, false) // include in execution witness } // Assemble the final block. diff --git a/consensus/clique/clique.go b/consensus/clique/clique.go index 89d9fb7abb..e19feaac60 100644 --- a/consensus/clique/clique.go +++ b/consensus/clique/clique.go @@ -304,7 +304,7 @@ func (c *Clique) verifyHeader(chain consensus.ChainHeaderReader, header *types.H } // Verify the non-existence of withdrawalsHash. if header.WithdrawalsHash != nil { - return fmt.Errorf("invalid withdrawalsHash: have %x, expected nil", header.WithdrawalsHash) + return fmt.Errorf("invalid withdrawalsHash: have %s, expected nil", header.WithdrawalsHash) } if chain.Config().IsCancun(header.Number, header.Time) { return errors.New("clique does not support cancun fork") diff --git a/consensus/ethash/consensus.go b/consensus/ethash/consensus.go index 475be5476f..9ee6b6c102 100644 --- a/consensus/ethash/consensus.go +++ b/consensus/ethash/consensus.go @@ -270,7 +270,7 @@ func (ethash *Ethash) verifyHeader(chain consensus.ChainHeaderReader, header, pa } // Verify the non-existence of withdrawalsHash. if header.WithdrawalsHash != nil { - return fmt.Errorf("invalid withdrawalsHash: have %x, expected nil", header.WithdrawalsHash) + return fmt.Errorf("invalid withdrawalsHash: have %s, expected nil", header.WithdrawalsHash) } if chain.Config().IsCancun(header.Number, header.Time) { return errors.New("ethash does not support cancun fork") diff --git a/core/block_validator.go b/core/block_validator.go index c0e7349727..bc3076ed08 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -79,7 +79,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { } // The withdrawalsHash is verified in ValidateState, like the state root, as verification requires state merkleization. } else if hash := types.DeriveSha(block.Withdrawals(), trie.NewStackTrie(nil)); hash != *header.WithdrawalsHash { - return fmt.Errorf("withdrawals root hash mismatch (header value %x, calculated %x)", *header.WithdrawalsHash, hash) + return fmt.Errorf("withdrawals root hash mismatch (header value %s, calculated %s)", *header.WithdrawalsHash, hash) } } else if block.Withdrawals() != nil { // Withdrawals are not allowed prior to Shanghai fork diff --git a/core/chain_makers.go b/core/chain_makers.go index aca4feccec..46e78b58f3 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -329,9 +329,10 @@ func GenerateChain(config *params.ChainConfig, parent *types.Block, engine conse b.header.Difficulty = big.NewInt(0) } } - if config.IsIsthmus(b.header.Time) { + if config.IsOptimismIsthmus(b.header.Time) { b.withdrawals = make([]*types.Withdrawal, 0) - b.header.WithdrawalsHash = &types.EmptyWithdrawalsHash + h := types.EmptyWithdrawalsHash + b.header.WithdrawalsHash = &h } // Mutate the state and block according to any hard-fork specs if daoBlock := config.DAOForkBlock; daoBlock != nil { diff --git a/core/genesis.go b/core/genesis.go index 2c324e6bb0..ae376cd1b7 100644 --- a/core/genesis.go +++ b/core/genesis.go @@ -504,13 +504,13 @@ func (g *Genesis) ToBlock() *types.Block { panic(fmt.Errorf("cannot both have genesis hash %s "+ "and non-empty state-allocation", *g.StateHash)) } - // stateHash is only relevant for pre-bedrock (and hence pre-isthmus) chains. + // g.StateHash is only relevant for pre-bedrock (and hence pre-isthmus) chains. // we bail here since this is not a valid usage of StateHash - if g.Config.IsIsthmus(g.Timestamp) { + if g.Config.IsOptimismIsthmus(g.Timestamp) { panic(fmt.Errorf("stateHash usage disallowed in chain with isthmus active at genesis")) } stateRoot = *g.StateHash - } else if stateRoot, storageRootMessagePasser, err = hashAlloc(&g.Alloc, g.IsVerkle(), g.Config.IsIsthmus(g.Timestamp)); err != nil { + } else if stateRoot, storageRootMessagePasser, err = hashAlloc(&g.Alloc, g.IsVerkle(), g.Config.IsOptimismIsthmus(g.Timestamp)); err != nil { panic(err) } return g.toBlockWithRoot(stateRoot, storageRootMessagePasser) @@ -575,13 +575,12 @@ func (g *Genesis) toBlockWithRoot(stateRoot, storageRootMessagePasser common.Has requests = make(types.Requests, 0) } // If Isthmus is active at genesis, set the WithdrawalRoot to the storage root of the L2ToL1MessagePasser contract. - if g.Config.IsIsthmus(g.Timestamp) { + if g.Config.IsOptimismIsthmus(g.Timestamp) { if storageRootMessagePasser == (common.Hash{}) { // if there was no MessagePasser contract storage, set the WithdrawalsHash to the empty hash - head.WithdrawalsHash = &types.EmptyWithdrawalsHash - } else { - head.WithdrawalsHash = &storageRootMessagePasser + storageRootMessagePasser = types.EmptyWithdrawalsHash } + head.WithdrawalsHash = &storageRootMessagePasser } } return types.NewBlock(head, &types.Body{Withdrawals: withdrawals, Requests: requests}, nil, trie.NewStackTrie(nil), g.Config) diff --git a/core/rlp_test.go b/core/rlp_test.go index 3e338d14a2..c2db2b8a91 100644 --- a/core/rlp_test.go +++ b/core/rlp_test.go @@ -27,7 +27,7 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rlp" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/crypto/sha3" ) @@ -207,15 +207,16 @@ func TestBlockRlpEncodeDecode(t *testing.T) { config := *params.OptimismTestConfig config.ShanghaiTime = &zeroTime config.IsthmusTime = &zeroTime + require.True(t, config.IsOptimismIsthmus(zeroTime)) block := getBlock(&config, 10, 2, 50) blockRlp, err := rlp.EncodeToBytes(block) - assert.Nil(t, err) + require.NoError(t, err) var decoded types.Block err = rlp.DecodeBytes(blockRlp, &decoded) - assert.Nil(t, err) + require.NoError(t, err) - assert.Equal(t, decoded.Hash(), block.Hash()) + require.Equal(t, decoded.Hash(), block.Hash()) } diff --git a/core/types/block.go b/core/types/block.go index 0eb5095c98..372332520d 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -295,7 +295,7 @@ func NewBlock(header *Header, body *Body, receipts []*Receipt, hasher TrieHasher } } - if config.IsIsthmus(header.Time) { + if config.IsOptimismIsthmus(header.Time) { if withdrawals == nil || len(withdrawals) > 0 { panic(fmt.Sprintf("expected non-nil empty withdrawals operation list in Isthmus, but got: %v", body.Withdrawals)) } diff --git a/eth/api_debug.go b/eth/api_debug.go index ab8dc7420f..203764a00e 100644 --- a/eth/api_debug.go +++ b/eth/api_debug.go @@ -482,6 +482,9 @@ func generateWitness(blockchain *core.BlockChain, block *types.Block) (*stateles return nil, fmt.Errorf("failed to process block %d: %w", block.Number(), err) } + // OP-Stack warning: below has the side-effect of including the withdrawals storage-root + // into the execution witness through the storage lookup by ValidateState, triggering the pre-fetcher. + // The Process function only runs through Finalize steps, not through FinalizeAndAssemble, missing merkleization. if err := blockchain.Validator().ValidateState(block, statedb, res, false); err != nil { return nil, fmt.Errorf("failed to validate block %d: %w", block.Number(), err) } diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index 582a9f87b2..1e3f1a8a97 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -167,7 +167,7 @@ type queue struct { } // newQueue creates a new download queue for scheduling block retrieval. -// The +// The opConfig argument may be nil, if not an OP-Stack chain. func newQueue(opConfig OPStackChainConfig, blockCacheLimit int, thresholdInitialSize int) *queue { lock := new(sync.RWMutex) q := &queue{