NewRoundMessageValidator creates RoundChangeValidator with correct value #518
Conversation
| @@ -56,7 +56,7 @@ public RoundChangeMessageValidator createRoundChangeMessageValidator( | |||
| return new RoundChangeMessageValidator( | |||
| roundIdentifier -> createMessageValidator(roundIdentifier, parentHeader), | |||
| validators, | |||
| IbftHelpers.calculateRequiredValidatorQuorum(validators.size()), | |||
| IbftHelpers.calculateRequiredValidatorQuorum(validators.size()) - 1, | |||
There was a problem hiding this comment.
Why do you want one less than the required quorum of validators? [magic number]
There was a problem hiding this comment.
like above, comment would hep explain why this is being done
There was a problem hiding this comment.
This calc has been broken out to a helper function which is now used in many places (which is good).
Throughout IBFT, the number of prepare messages is 1 less than the actual 2/3 quorum as the Proposal is deemed to act as a prepare message (thus, there is also a rule that none of the prepare messages may come from the Proposer).
| @@ -121,9 +123,11 @@ private boolean validateRoundChangeMessagesAndEnsureTargetRoundMatchesRoot( | |||
|
|
|||
| for (final SignedData<RoundChangePayload> roundChangeMsg : | |||
| roundChangeCert.getRoundChangePayloads()) { | |||
| // RoundChangeValidator requires the "requisite Prepare Msg Count" (which is 1 less than | |||
There was a problem hiding this comment.
Nit: this comment could be incorporated into a method or constant, negating the need for its existence.
There was a problem hiding this comment.
Has been moved out to IbftHelpers.prepareMessageCountFromQuorum()
| @@ -56,7 +56,7 @@ public RoundChangeMessageValidator createRoundChangeMessageValidator( | |||
| return new RoundChangeMessageValidator( | |||
| roundIdentifier -> createMessageValidator(roundIdentifier, parentHeader), | |||
| validators, | |||
| IbftHelpers.calculateRequiredValidatorQuorum(validators.size()), | |||
| IbftHelpers.calculateRequiredValidatorQuorum(validators.size()) - 1, | |||
There was a problem hiding this comment.
like above, comment would hep explain why this is being done
A RoundChangeValidator accepts number of prepare Messages (which is 1 less than quorum).
Previously, NewRoundMessageValidator created a RoundChangeValidator with expected prepare messages set to quorum (which is 1 too large).