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

fix: ensure empty withdrawals after cancun #2384

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

buddh0
Copy link
Collaborator

@buddh0 buddh0 commented Apr 10, 2024

Description

fix: ensure empty withdrawals after cancun

Rationale

tell us why we need these changes...

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

@@ -96,15 +96,15 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
},
func() error {
// Withdrawals are present after the Shanghai fork.
if !header.EmptyWithdrawalsHash() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollback and align with geth

// Withdrawals list must be present in body after Shanghai.
if block.Withdrawals() == nil {
return errors.New("missing withdrawals in block body")
}
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
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollback and align with geth

@@ -195,7 +195,7 @@ func (h *Header) EmptyReceipts() bool {

// EmptyWithdrawalsHash returns true if there are no WithdrawalsHash for this header/block.
func (h *Header) EmptyWithdrawalsHash() bool {
return h.WithdrawalsHash == nil || *h.WithdrawalsHash == EmptyWithdrawalsHash
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it equivalent to other Emptyxxx function, such as EmptyReceipts

@@ -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() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollback and align with geth

@@ -788,9 +788,9 @@ func (q *queue) DeliverBodies(id string, txLists [][]*types.Transaction, txListH
if uncleListHashes[index] != header.UncleHash {
return errInvalidBody
}
if header.EmptyWithdrawalsHash() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollback and align with geth

// nil hash means that withdrawals should not be present in body
if len(withdrawalLists[index]) != 0 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollback and align with geth

@@ -1053,7 +1053,7 @@ func (b *Block) Withdrawals(ctx context.Context) (*[]*Withdrawal, error) {
return nil, err
}
// Pre-shanghai blocks
if block.Header().EmptyWithdrawalsHash() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollback and align with geth

@@ -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() {
Copy link
Collaborator Author

@buddh0 buddh0 Apr 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set empty withdraws at miner side

@@ -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() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set empty withdraws at fetcher side

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need similar changes in downloader here?

blocks[i] = types.NewBlockWithHeader(result.Header).WithBody(result.Transactions, result.Uncles).WithWithdrawals(result.Withdrawals).WithSidecars(result.Sidecars)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need similar changes in downloader here?

blocks[i] = types.NewBlockWithHeader(result.Header).WithBody(result.Transactions, result.Uncles).WithWithdrawals(result.Withdrawals).WithSidecars(result.Sidecars)

esult.Withdrawals is already empty now

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. But under what condition the above if statement of fetcher will be true?

@@ -195,7 +195,7 @@ func (h *Header) EmptyReceipts() bool {

// EmptyWithdrawalsHash returns true if there are no WithdrawalsHash for this header/block.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should change the description of this function to make it more clear.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should change the description of this function to make it more clear.

ok

@@ -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 {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rollback and align with geth

@buddh0 buddh0 changed the title fix: ensure empty withdraws after cancun fix: ensure empty withdrawals after cancun Apr 10, 2024
@zzzckck
Copy link
Collaborator

zzzckck commented Apr 10, 2024

@zzzckck zzzckck merged commit a75e823 into develop Apr 10, 2024
6 of 7 checks passed
@zzzckck zzzckck deleted the empty_withdraws_after_cancun branch April 10, 2024 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants