Implementation ideas for IBFT2 block validation support

[VladLupashevskyi]

QBFT algorithm seems the preferred one to be used among IBFT, IBFT2 and QBFT on both Besu and GoQuorum clients. However there are no plans to add support for IBFT2 in GoQuorum as mentioned in this issue #1377. So I'd thought it would be nice idea to add support to GoQuorum for only block verification of IBFT2 chains that have switched to QBFT. I did some analysis and found out that BFT specific extra data which is stored per block has the same structure for IBFT2 and QBFT consensuses, however RLP encoding differs for some cases, therefore block hash would differ and validation during the sync would fail. As a conclusion by just introducing an IBFT2 block extra data encoder/decoder will make it possible to sync and validate IBFT2 blocks on GoQuorum.

Here are the differences that I found out:

• In case there are no votes, IBFT2 represents the value as a null value, QBFT represents it as empty list.
• Encoding for drop vote on IBFT2 is represented as zero byte 0x00, QBFT represents it as null value (according to Ethereum conventions).
• Round is represented on IBFT2 always as 4 bytes value (0 as 0x00000000, 1 as 0x00000001), QBFT represents it as scalar according to Ethereum conventions (0 as 0x, 1 as 0x01).
• For block hash calculations and for block signing when round number or/and block seals have to be excluded, IBFT2 drops excluded arguments completely from encoding (so <Vanity>, <Validators>, <Vote>, <Round> or <Vanity>, <Validators>, <Vote> structures are used), whereas QBFT puts zero as default for round number and empty list for seals in such cases. However on IBFT2 the full extra data is used for genesis block hash calculation.

I have made some changes to client to make QBFT extra data encoder/decoder behave as IBFT2 one and I have run it for IBFT2 chain which was successfully synced. The changes I've done can be found in this commit. It has to be mentioned that I'm a complete beginner in Go and this commit is just a proof of concept, however any comments are welcome :)

The proper solution would be to have some param in chain spec like useIBFT2ExtraDataValidationUntilBlock, that will enable this logic until some block which would be the block when IBFT2 chain have switched consensus to QBFT. But before starting to implement this, I wanted to gather opinions of more experienced devs here whether it makes sense and that I haven't missed anything. Also already implementation related question: What is the better way to submit chain params to QBFTFilteredHeader function that is used in the Hash function? (which is used in many places and adding extra argument for chain params to it would not be that convenient)

This approach of course does not enable mining on IBFT2 chain however I think it's a good solution for chains that have switched from IBFT2 to QBFT (where mining could be possible already) and requires much less effort to implement.

Thank you very much and I'm looking forward to your feedback!

[antonydenyer]

It's certainly feasible the concerns I have are around long term maintainability. The more things we add the harder it is to keep up to date with upstream. I personally think that long term migrating/re-genesis-ing networks to be qbft only is more sustainable. That said if we're going to add support for ibft2 we would need a bunch of tests added into https://github.com/ConsenSys/quorum-acceptance-tests to ensure that we don't break it going forward.

[VladLupashevskyi]

@antonydenyer I agree re-genesising networks to be qbft would be easier solution, however I think it will not work for some cases, where the private key of a validator in the past is not known anymore (e.g. when this validator does not participate in the chain anymore or private key was lost, etc.). Such chains will not have possibility to migrate to qbft and will be stuck with ibft2. What do you think about that?

I very much agree that such a case if implemented must have tests written, I have no experience with Gauge yet, but I could start with pull request and implementation first and as a next point try to create some tests for that, maybe with somebody's assistance. I would appreciate if I could get an answer for my question about submitting chain params to QBFTFilteredHeader that I've mentioned in the first message.

[antonydenyer]

(From a colleague) The main changes were in how to represent empty data and numbers. You can compare the extra data encoding by looking at

https://github.com/hyperledger/besu/blob/main/consensus/ibft/src/main/java/org/hyperledger/besu/consensus/ibft/IbftExtraDataCodec.java
https://github.com/hyperledger/besu/blob/main/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/QbftExtraDataCodec.java

[VladLupashevskyi]

@antonydenyer Thank you for reply. That's exactly what I've checked and created afterwards a list of differences in the first message :)

The main issue I faced was how to pass chain params with potentially new useIBFT2ExtraDataValidationUntilBlock param to QBFTFilteredHeader (where all the extra data logic takes place), which is called in Hash function.

[baptiste-b-pegasys]

hello, can you review and sign the cla?
#1531