Skip to content

fix: ensure empty withdrawals after cancun#2384

Merged
zzzckck merged 2 commits intodevelopfrom
empty_withdraws_after_cancun
Apr 10, 2024
Merged

fix: ensure empty withdrawals after cancun#2384
zzzckck merged 2 commits intodevelopfrom
empty_withdraws_after_cancun

Conversation

@buddh0
Copy link
Copy Markdown
Contributor

@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
  • ...

},
func() error {
// Withdrawals are present after the Shanghai fork.
if !header.EmptyWithdrawalsHash() {
Copy link
Copy Markdown
Contributor 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

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
Copy Markdown
Contributor 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


// 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
Copy Markdown
Contributor 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

}
if !header.EmptyBody() {
item.pending.Store(item.pending.Load() | (1 << bodyType))
} else if !header.EmptyWithdrawalsHash() {
Copy link
Copy Markdown
Contributor 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

if uncleListHashes[index] != header.UncleHash {
return errInvalidBody
}
if header.EmptyWithdrawalsHash() {
Copy link
Copy Markdown
Contributor 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

if header.EmptyWithdrawalsHash() {
if header.WithdrawalsHash == nil {
// nil hash means that withdrawals should not be present in body
if len(withdrawalLists[index]) != 0 {
Copy link
Copy Markdown
Contributor 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

return nil, err
}
// Pre-shanghai blocks
if block.Header().EmptyWithdrawalsHash() {
Copy link
Copy Markdown
Contributor 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

// env.receipts = receipts
finalizeBlockTimer.UpdateSince(finalizeStart)

if block.Header().EmptyWithdrawalsHash() {
Copy link
Copy Markdown
Contributor 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

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
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
Contributor 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor 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

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
Copy Markdown
Contributor 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
Copy Markdown
Collaborator

zzzckck commented Apr 10, 2024

@zzzckck zzzckck merged commit a75e823 into develop Apr 10, 2024
@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