-
Notifications
You must be signed in to change notification settings - Fork 130
Conversation
It was identified during a demonstration that Pantheon, when running in IBFT would show a "Bad Block Import" when a validator was added or removed from the validator pool. It was determined this was due to IBFT maintaining a single, 'global' copy of the curent list of validators, which was updated when a block was imported - thus when a block which had been imported vi IBFT was then received via Eth block propogation, the validator list would not align with the global list (as it had been updated in the IBFT import). The solution has been to utilise the VoteTallyCache as used in the Clique implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nowhere near as daunting as I expected for a 53 file change set!
@@ -81,7 +81,7 @@ | |||
if (header.getNumber() == BlockHeader.GENESIS_BLOCK_NUMBER) { | |||
continue; | |||
} | |||
if (header.getNumber() % 100 == 0) { | |||
if (header.getNumber() % 1 == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: why not just get rid of the if statement to log every block import
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was not meant to change, was used for debug, reverted.
@@ -162,7 +162,7 @@ private void extractSignatures(final Block block) { | |||
final BlockHeaderValidator<C> blockHeaderValidator = protocolSpec.getBlockHeaderValidator(); | |||
final boolean validHeader = | |||
blockHeaderValidator.validateHeader( | |||
header, previousHeader, context, HeaderValidationMode.FULL); | |||
header, previousHeader, context, HeaderValidationMode.DETACHED_ONLY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did the validation mode need to be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a bug in Pantheon which was hidden up until this change - basically they do async block validation (thus block 2 may be validated before block 1) - this is illegal in IBFT, as we must validate a block fully with respect to its parent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked and this was changed to fix a bug "the validation rule was a hidden bug in Pantheon - the BlockImporter validates blocks ASYNC - meaning, out of order 😞 Which IBFT can no longer do!
So when doing ASYNC, you MUST do only detached rules as the initial validation, then do the rest at the end"
…it's completely routine. (PegaSysEng#919)
It was identified during a demonstration that Pantheon, when running
in IBFT would show a "Bad Block Import" when a validator was added
removed from the validator pool.
It was determined this was due to IBFT maintaining a single, 'global'
copy of the curent list of validators, which was updated when a block
was imported - thus when a block which had been imported via IBFT
was then received via Eth block propogation, the validator list would
not align with the global list (as it had been updated in the IBFT
import).
The solution has been to utilise the VoteTallyCache as used in the
Clique implementation.