Previously, IBFT only validated the header of the block on reception
of a proposal. However in order to minimise the risk of a "bad block"
being detected after a PreparedCertificate being created, it has been
decided to validate the whole block (i.e. exercise the transactions,
process the block and ensure hashes etc line up with the header).
There is still some (minor) risk of a failed block import, however
it is substantially reduced.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
Testing indicated that if quorum-1 commit messages is received prior
to the proposal, the proposed block will be imported twice.
Theoretically this is handled safely by the BlockImporter, however
is clearly an error in the IbftRound code.
This commit updates IbftRound such that the import can only occur
once.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
* Removing code identified by the 'unused declaration' analysis in IntelliJ
* Undoing removal of injectNewRound as that's actually used
* Removing wrapper class from Clique UT
* Spotless
* Adding back code
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
In order to validate a block separate to its commit seals, the commit
seals must be validated separately to the validator content of the
block.
At this stage, the commit rule is _not_ light, however this change
is to be conducted in subsequent PRs.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
Constructor of the RoundChangeManager now accepts a
RoundChangeMessageValidator rather than creating it from inputs.
This removes complexity from this class, pushes creation back to the
validator factory, and simplifies the IbftBlockHeightManagerFactory.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
Ibft is required to validate a block upon reception, but not import
it until a later time.
As such, IBFT will require validation and importing to be separated.
The validator has been exposed as part of the ProtocolSpecification.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
Integration tests have been added to determine the IBFT components
response to the reception of RoundChange messages.
Through the course of creating these tests the following changes were
made to the RoundChangeManager:
* Stores RoundChangeMessages in the order of arrival
* Discards subsequent RoundChange messages from the same node,
targeting a specific round.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
* NewRoundMessageValidator ignores Round Number when comparing blocks
NewRoundMessageValidator previously ensured the block in the newest
PreparedCertificate was the same as that in the Proposal by comparing
block hashes.
However, the hash-function for received IBFT blocks includes round
number - thus the blocks will never be deemed 'equal'.
As such, the blocks must be compared using a hash function which
ignores the round number - i.e. the OnChain function.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
To test the IBFT implementation at an integration level, an amount of "setup" is required.
This commit creates a framework of "n" 'external' nodes which are able to inject messages to the IbftController, while also exposing the messages received from the local node.
Thus integration tests are able to be written in terms of input events (including messages), expected (network) responses and changes in the blockchain state.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
* Initial implementation of ibft_getValidatorsByBlockHash
* Added in missing method to blockchain queries and cleaned up code.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
The RoundChangeManager was incorrectly creating its internal
RoundChangeMessageValidator with "quorum" number of Prepare messages
required - however, given the ProposalMessage in the Prepared-
Certificate represents a valid node.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
The IbftHeightManager is responsible for all things 'meta-round'
related, such things as:
* Handling RoundTimeout and starting a new round
* Handling NewRound messages and starting a new round
* Ensuring RoundChange messages are sent at the correct time with
appropriate content.
* Collating RoundChange messages and starting a new round using the
best prepared certificate in the collection.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
When the local node is to be the proposer for a new round
(due to round change), the new Ibft Round is to be created using the
latest received PreparedCertificate, and a NewRound message multicast
to all validators.
It should be noted, the block in the NewRound Proposal must be
modified from that received in the PreparedCertificate such that it
contains the executing round number.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
This has required the MessageValidatorFactory used as part of the
RoundChangeMessageValidator to be renamed, which in turn has had
renaming knock on effects through the code.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
An abstraction existed which allowed for the differentiation of IBFT
messages which were used 'in-round' (Proposal, Prepare, Commit) and
those that were inter-round (RoundChange, NewRound).
This abstraction has not proven useful now that actual capability has
been developed, and as such is being removed.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
IbftBlockCreator is required to correctly set the MixHash, and also ensure the IbftBlockHashing function used to hash the block includes the round number, but not the commit seals.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
The IbftRound is responsible for sequencing the transmission of
network packets based on received data.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
* Upgrade errorprone
* Upgrade errorprone from 2.3.1 to 2.3.2
* Upgrade Jenkinsfile so that CI will use Java 11
* Suppress these new rules:
* EqualsGetClass
* ImmutableEnumChecker
* UnnecessaryParentheses
* Change code to conform to these new rules:
* BadImport
* BadInstanceof
* InconsistentHashCode
* LockNotBeforeTry
* MathAbsoluteRandom
* ModifiedButNotUsed
* UndefinedEquals
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
The IBFT RoundState is responsible for determining if sufficient
valid messages have been received to deem the node "Prepared" or
"Committed".
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
Received New Round messages require significant validation to ensure their content is consistent and valid with regard the current state of the system.
This commit includes a class (and tests) which ensures illegally constructed NewRound messages are identified.
This does not include the action to be taken upon detection of an illegal New Round message.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
The classes used around the IBFT Messaging classes have all been renamed to make their use less cumbersome throughout the codebase.
Other issues are yet to be found (not all variable names and function names have been captured in this sweep).
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
A problem was identified whereby the vote in a child block was applied to the parent, this was resolved by
duplicating the parent VoteTally before modifying it, and injecting to the child block.
Some refactoring was also conducted to make VoteTally logic simpler.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
Adrian Sutton
tmohay
CJ Hare
Jason Frame
Rob Dawson
Chris Mckay
Danno Ferrin