Skip to content

Commit

Permalink
all: minor OP-Stack Isthmus withdrawals-root fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
protolambda committed Dec 5, 2024
1 parent c5f905b commit 77467f4
Show file tree
Hide file tree
Showing 11 changed files with 30 additions and 20 deletions.
9 changes: 7 additions & 2 deletions beacon/engine/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions consensus/beacon/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion consensus/clique/clique.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion consensus/ethash/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion core/block_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions core/chain_makers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
13 changes: 6 additions & 7 deletions core/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions core/rlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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())
}
2 changes: 1 addition & 1 deletion core/types/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
3 changes: 3 additions & 0 deletions eth/api_debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion eth/downloader/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit 77467f4

Please sign in to comment.