Conversation
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
…rgument Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
… QBFT is active Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
3d91f05 to
9fe66cf
Compare
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
9fe66cf to
4872e93
Compare
fc60c5e to
cec133c
Compare
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
cec133c to
0e2dd4a
Compare
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
.../src/main/java/org/hyperledger/besu/consensus/common/bft/BaseBftProtocolScheduleBuilder.java
Show resolved
Hide resolved
.../main/java/org/hyperledger/besu/ethereum/referencetests/BlockchainReferenceTestCaseSpec.java
Outdated
Show resolved
Hide resolved
...reum/core/src/main/java/org/hyperledger/besu/ethereum/transaction/BlockSimulationResult.java
Show resolved
Hide resolved
...qbft-core/src/main/java/org/hyperledger/besu/consensus/qbft/core/statemachine/QbftRound.java
Outdated
Show resolved
Hide resolved
consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/messagewrappers/Proposal.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
|
Started 2 nodes on mainnet to check if there is performance or stability issue with this PR |
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
| public void multicastProposal( | ||
| final ConsensusRoundIdentifier roundIdentifier, | ||
| final QbftBlock block, | ||
| final Optional<BlockAccessList> blockAccessList, |
There was a problem hiding this comment.
I could do with a little more context/background to the PR that would help me understand the changes here.
So my understanding of BALs is they are a new field in the block header? But your PR separates them out from the block.
In your PR you've said stored and handled separately for both PoS and PoA networks. I think I understand storing them separately on disk - to have different retention policies etc. But I'm not sure why the code needs to pass them around separately to the block? We currently pass around QbftBlock - is there a reason this can't just contain an Optional BAL field rather than changing all of the internal function signatures?
There was a problem hiding this comment.
So my understanding of BALs is they are a new field in the block header? But your PR separates them out from the block.
There is a new BAL-related field in the block header called balHash. BALs used to be part of the block body (and hence stored together). This PR mainly separates BALs from the block body.
Passing BALs in the block was actually an intermediate stage when working on this PR, but then I invested into passing it around as a separate argument, because, conceptually, a block consists of only body + header, not body + header + BAL. BAL is a data structure separate from the block.
I don't fully understand the intended purpose of QbftBlock, but currently its only implementation simply wraps a Besu block (body + header) and wrapped Besu header (again). If that is the case, not coupling QbftBlock with BALs makes it potentially easier to remove (and thus simplify the code).
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com> # Conflicts: # consensus/qbft-core/src/test/java/org/hyperledger/besu/consensus/qbft/core/messagewrappers/ProposalTest.java # ethereum/core/src/main/java/org/hyperledger/besu/ethereum/MainnetBlockValidator.java
|
Tested last version via kurtosis again, seems running fine. However currently there does seem to be a regression in QBFT. EDIT: Fixed in fe37907 |
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com>
matkt
left a comment
There was a problem hiding this comment.
LGTM,
Tested on mainnet, hoodi and also tried to produce block on hoodi with a validator
Signed-off-by: Miroslav Kovar <miroslavkovar@protonmail.com> Co-authored-by: Karim Taam <karim.t2am@gmail.com>
…nodes PR besu-eth#9629 added blockAccessList to QBFT/IBFT consensus messages but broke backward compatibility with older nodes that do not include this field. PR besu-eth#9977 partially fixed this for QBFT ProposalPayload (where blockAccessList is the last field, so isEndOfCurrentList() suffices), but missed QBFT RoundChange where blockAccessList sits *before* the Prepares list. In that position isEndOfCurrentList() returns false (because Prepares is still there), causing the decoder to consume the Prepares list as a BlockAccessList and then fail with: RLPException: Cannot enter a lists, input is fully consumed Fix: use the enterList() item count to detect old (3-item) vs new (4-item) format in QBFT RoundChange.decode(). Also add the missing isEndOfCurrentList() guard to IBFT RoundChange and IBFT Proposal (where blockAccessList is the last field). Signed-off-by: qinglin <qinglin@example.com> 🤖 Generated with [Qoder][https://qoder.com]
…nodes PR besu-eth#9629 added blockAccessList to QBFT/IBFT consensus messages but broke backward compatibility with older nodes that do not include this field. PR besu-eth#9977 partially fixed this for QBFT ProposalPayload (where blockAccessList is the last field, so isEndOfCurrentList() suffices), but missed QBFT RoundChange where blockAccessList sits *before* the Prepares list. In that position isEndOfCurrentList() returns false (because Prepares is still there), causing the decoder to consume the Prepares list as a BlockAccessList and then fail with: RLPException: Cannot enter a lists, input is fully consumed Fix: use the enterList() item count to detect old (3-item) vs new (4-item) format in QBFT RoundChange.decode(). Also add the missing isEndOfCurrentList() guard to IBFT RoundChange and IBFT Proposal (where blockAccessList is the last field). Signed-off-by: qinglin <qinglin@example.com> 🤖 Generated with [Qoder][https://qoder.com]
With these changes, BALs are removed from the block body, stored and handled separately for both PoS and PoA networks.
The follow up PRs will:
Testing was done by running pandoras-box against QBFT network with and without BALs and by running a local devnet across Amsterdam fork boundary using Ethereum Kurtosis package with
tx_fuzzandspamoor.Reference tests
bal@v2.0.0are passing,bal@v3.0.1are not (due to incorrect state roots - maybe a problem with BPO config?).Closes: #9589