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

Isthmus: withdrawals root in block header #383

Merged
merged 6 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
all: minor OP-Stack Isthmus withdrawals-root fixes
  • Loading branch information
protolambda authored and vdamle committed Dec 10, 2024
commit 1750edce4fd76ce14a4ca7ff112153fdeb676b3a
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
vdamle marked this conversation as resolved.
Show resolved Hide resolved
}

// 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
vdamle marked this conversation as resolved.
Show resolved Hide resolved
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
Loading