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

QBFT/Shanghai blocks failing withdrawal validation on import #6760

Closed
matthew1001 opened this issue Mar 19, 2024 · 2 comments · Fixed by #6765
Closed

QBFT/Shanghai blocks failing withdrawal validation on import #6760

matthew1001 opened this issue Mar 19, 2024 · 2 comments · Fixed by #6765
Assignees
Labels
bug Something isn't working non mainnet (private networks) not related to mainnet features - covers privacy, permissioning, IBFT2, QBFT

Comments

@matthew1001
Copy link
Contributor

Description

Running 4 QBFT validators with shanghaiTime: 0 the nodes mine blocks correctly and accept transactions correctly.

However, when a new validator joins and syncs with the chain it fails to validate the imported blocks because the block body withdrawals element is null (represented as Optional.empty() in the code)

In the QBFT/Shanghai PR (#6353) I disabled withdrawal validation for all BFT protocol schedules (see https://github.com/matthew1001/besu/blob/97c50417787f21bca8acf0a8b7d4e74d65f9f19e/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java#L120) since withdrawals aren't a thing for BFT chains.

In hindsight I think the correct way to treat them would have been to always include an empty withdrawals list, rather than no list at all. This would pass withdrawal validation by shanghai and onwards protocol schedule validators.

Removing the BFT protocol schedule override:

.withdrawalsValidator(
            new WithdrawalsValidator
                .ProhibitedWithdrawals()) // QBFT/IBFT doesn't support withdrawals

means that BFT nodes will use whichever withdrawal validator is used by the given protocol schedule.

Versions (Add all that apply)

  • Software version: 24.3.0
@matthew1001 matthew1001 self-assigned this Mar 19, 2024
@matthew1001 matthew1001 added bug Something isn't working non mainnet (private networks) not related to mainnet features - covers privacy, permissioning, IBFT2, QBFT labels Mar 19, 2024
@matthew1001
Copy link
Contributor Author

Removing the withdrawal validator override does indeed resolve the import issue, but I see an interesting QBFT issue if QBFT validators set an empty withdrawals list in the blocks they propose. The recipients of the proposed blocks seem to receive a block containing a null withdrawals field, instead of the empty list set by the proposer. I've drilled all the way down to the RLP encoding layer and back up, and can't identify where the withdrawals field is being nulled out during the multicast to other nodes. It's possible the RLP decode is removing it, rather than the encode not including it (scratches head)

@matthew1001
Copy link
Contributor Author

It turns out there are two code BFT code-paths to create a new block and they both needed updating to create a correct block with withdrawals enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non mainnet (private networks) not related to mainnet features - covers privacy, permissioning, IBFT2, QBFT
Projects
None yet
1 participant