From a75e82367d97e3a17b538e147029a98939e5fd48 Mon Sep 17 00:00:00 2001 From: buddho Date: Wed, 10 Apr 2024 14:42:16 +0800 Subject: [PATCH] fix: ensure empty withdrawals after cancun (#2384) --- consensus/parlia/parlia.go | 4 +--- core/block_validator.go | 4 ++-- core/chain_makers.go | 3 +++ core/types/block.go | 4 ++-- eth/downloader/queue.go | 6 +++--- eth/fetcher/block_fetcher.go | 5 ++++- eth/protocols/eth/handlers.go | 2 +- graphql/graphql.go | 2 +- miner/worker.go | 4 ++++ 9 files changed, 21 insertions(+), 13 deletions(-) diff --git a/consensus/parlia/parlia.go b/consensus/parlia/parlia.go index 9b07fd9bb4..911a04359c 100644 --- a/consensus/parlia/parlia.go +++ b/consensus/parlia/parlia.go @@ -599,9 +599,7 @@ func (p *Parlia) verifyHeader(chain consensus.ChainHeaderReader, header *types.H switch { case header.ParentBeaconRoot != nil: return fmt.Errorf("invalid parentBeaconRoot, have %#x, expected nil", header.ParentBeaconRoot) - // types.EmptyWithdrawalsHash represents a empty value when EIP-4895 enabled, - // here, EIP-4895 still be disabled, value expected to be `types.EmptyWithdrawalsHash` is only to feet the demand of rlp encode/decode - case header.WithdrawalsHash == nil || *header.WithdrawalsHash != types.EmptyWithdrawalsHash: + case !header.EmptyWithdrawalsHash(): return errors.New("header has wrong WithdrawalsHash") } if err := eip4844.VerifyEIP4844Header(parent, header); err != nil { diff --git a/core/block_validator.go b/core/block_validator.go index 8bd5071f12..d15e2cd786 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -96,7 +96,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { }, func() error { // Withdrawals are present after the Shanghai fork. - if !header.EmptyWithdrawalsHash() { + if header.WithdrawalsHash != nil { // Withdrawals list must be present in body after Shanghai. if block.Withdrawals() == nil { return errors.New("missing withdrawals in block body") @@ -104,7 +104,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { 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) } - } else if len(block.Withdrawals()) != 0 { // Withdrawals turn into empty from nil when BlockBody has Sidecars + } else if block.Withdrawals() != nil { // Withdrawals turn into empty from nil when BlockBody has Sidecars // Withdrawals are not allowed prior to shanghai fork return errors.New("withdrawals present in block body") } diff --git a/core/chain_makers.go b/core/chain_makers.go index e3cf61dc63..0592210dba 100644 --- a/core/chain_makers.go +++ b/core/chain_makers.go @@ -372,6 +372,9 @@ func GenerateChain(config *params.ChainConfig, parent *types.Block, engine conse if err != nil { panic(err) } + if block.Header().EmptyWithdrawalsHash() { + block = block.WithWithdrawals(make([]*types.Withdrawal, 0)) + } if config.IsCancun(block.Number(), block.Time()) { for _, s := range b.sidecars { s.BlockNumber = block.Number() diff --git a/core/types/block.go b/core/types/block.go index e947792c1e..422363b32f 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -193,9 +193,9 @@ func (h *Header) EmptyReceipts() bool { return h.ReceiptHash == EmptyReceiptsHash } -// EmptyWithdrawalsHash returns true if there are no WithdrawalsHash for this header/block. +// EmptyWithdrawalsHash returns true if the WithdrawalsHash is EmptyWithdrawalsHash. func (h *Header) EmptyWithdrawalsHash() bool { - return h.WithdrawalsHash == nil || *h.WithdrawalsHash == EmptyWithdrawalsHash + return h.WithdrawalsHash != nil && *h.WithdrawalsHash == EmptyWithdrawalsHash } // Body is a simple (mutable, non-safe) data container for storing and moving diff --git a/eth/downloader/queue.go b/eth/downloader/queue.go index b97cfdec6d..72d21c329e 100644 --- a/eth/downloader/queue.go +++ b/eth/downloader/queue.go @@ -81,7 +81,7 @@ func newFetchResult(header *types.Header, fastSync bool, pid string) *fetchResul } if !header.EmptyBody() { item.pending.Store(item.pending.Load() | (1 << bodyType)) - } else if !header.EmptyWithdrawalsHash() { + } else if header.WithdrawalsHash != nil { item.Withdrawals = make(types.Withdrawals, 0) } if fastSync && !header.EmptyReceipts() { @@ -788,9 +788,9 @@ func (q *queue) DeliverBodies(id string, txLists [][]*types.Transaction, txListH if uncleListHashes[index] != header.UncleHash { return errInvalidBody } - if header.EmptyWithdrawalsHash() { + if header.WithdrawalsHash == nil { // nil hash means that withdrawals should not be present in body - if len(withdrawalLists[index]) != 0 { + if withdrawalLists[index] != nil { return errInvalidBody } } else { // non-nil hash: body must have withdrawals diff --git a/eth/fetcher/block_fetcher.go b/eth/fetcher/block_fetcher.go index e013e3b29d..4cd6d5f37e 100644 --- a/eth/fetcher/block_fetcher.go +++ b/eth/fetcher/block_fetcher.go @@ -576,7 +576,7 @@ func (f *BlockFetcher) loop() { select { case res := <-resCh: res.Done <- nil - // Ignoring withdrawals here, since the block fetcher is not used post-merge. + // Ignoring withdrawals here, will set it to empty later if EmptyWithdrawalsHash in header. txs, uncles, _, sidecars := res.Res.(*eth.BlockBodiesResponse).Unpack() f.FilterBodies(peer, txs, uncles, sidecars, time.Now()) @@ -723,6 +723,9 @@ func (f *BlockFetcher) loop() { matched = true if f.getBlock(hash) == nil { block := types.NewBlockWithHeader(announce.header).WithBody(task.transactions[i], task.uncles[i]) + if block.Header().EmptyWithdrawalsHash() { + block = block.WithWithdrawals(make([]*types.Withdrawal, 0)) + } block = block.WithSidecars(task.sidecars[i]) block.ReceivedAt = task.time blocks = append(blocks, block) diff --git a/eth/protocols/eth/handlers.go b/eth/protocols/eth/handlers.go index 992449be69..b028f50637 100644 --- a/eth/protocols/eth/handlers.go +++ b/eth/protocols/eth/handlers.go @@ -367,7 +367,7 @@ func handleBlockBodies(backend Backend, msg Decoder, peer *Peer) error { txsHashes[i] = types.DeriveSha(types.Transactions(body.Transactions), hasher) uncleHashes[i] = types.CalcUncleHash(body.Uncles) // Withdrawals may be not nil, but a empty value when Sidecars not empty - if len(body.Withdrawals) > 0 { + if body.Withdrawals != nil { withdrawalHashes[i] = types.DeriveSha(types.Withdrawals(body.Withdrawals), hasher) } } diff --git a/graphql/graphql.go b/graphql/graphql.go index cf4d7a34ca..fd782206fb 100644 --- a/graphql/graphql.go +++ b/graphql/graphql.go @@ -1053,7 +1053,7 @@ func (b *Block) Withdrawals(ctx context.Context) (*[]*Withdrawal, error) { return nil, err } // Pre-shanghai blocks - if block.Header().EmptyWithdrawalsHash() { + if block.Header().WithdrawalsHash == nil { return nil, nil } ret := make([]*Withdrawal, 0, len(block.Withdrawals())) diff --git a/miner/worker.go b/miner/worker.go index cdd47e2bc6..09d7fedf03 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -1367,6 +1367,10 @@ func (w *worker) commit(env *environment, interval func(), update bool, start ti // env.receipts = receipts finalizeBlockTimer.UpdateSince(finalizeStart) + if block.Header().EmptyWithdrawalsHash() { + block = block.WithWithdrawals(make([]*types.Withdrawal, 0)) + } + // If Cancun enabled, sidecars can't be nil then. if w.chainConfig.IsCancun(env.header.Number, env.header.Time) && env.sidecars == nil { env.sidecars = make(types.BlobSidecars, 0)