-
Notifications
You must be signed in to change notification settings - Fork 130
Spilt Ibft MessageValidator into components #752
Conversation
Moving to IBFT2.1 requires that validation be conducted on the signeddata aspects of a message separately from the 'piggybacked' block. Move Validators to using Messages
@@ -1,5 +1,5 @@ | |||
/* | |||
* Copyright 2018 ConsenSys AG. | |||
* Copyright 2019 ConsenSys AG. |
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.
FYI - copyright dates are usually left as the earliest date i.e. prior art
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.
added by build magic?
if (!validateBlockMatchesProposalRound(msg.getPayload())) { | ||
return false; | ||
} | ||
|
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.
proposal = Optional.of(msg);
return true;
}
return false;
Nicer to read? (removes negation, groups together logical conditions)
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.
Somewhat - but this follows the pattern which has been used throughout - i.e. final return is always true ...
final ConsensusRoundIdentifier roundIdentifier, | ||
final BlockValidator<IbftContext> blockValidator, | ||
final ProtocolContext<IbftContext> protocolContext, | ||
final BlockHeader parentHeader) { |
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.
doesn't look this is used
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.
True, and this is part of a bigger issue - but will remove it from here (which is the start of something nice).
msgType); | ||
return false; | ||
} | ||
private final SignedDataValidator signedDataValidator; |
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.
Perhaps some tests for messageValidator to prove that it is delegating the validation of the payload to the signedDataValidator
Moving to IBFT2.1 requires that validation be conducted on the signeddata aspects of a message separately from the 'piggybacked' block. Move Validators to using Messages
Moving to IBFT2.1 requires that validation be conducted
on the signeddata aspects of a message separately from the
'piggybacked' block.