Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Fix for bug in the validation of Round-Change-Certificates #984

Conversation

saltiniroberto
Copy link
Contributor

PR description

The Round-Change-Certificate must contain at least Quorum(n) Round-Change messages sent by different validators
The current code does not verify this condition, but only that the Round-Change-Certificate contains at least Quorum(n) Round-Change messages sent by validators.
This PR fixes the issue.
Note: Tests must be added

Fixed Issue(s)

@saltiniroberto saltiniroberto force-pushed the fix_Round_Change_Message_Verification branch from 1ab32ff to 0eb7c19 Compare February 26, 2019 14:59
@saltiniroberto saltiniroberto changed the title Fix for bug in the validation of the Round-Change-Certificate Fix for bug in the validation of the Round-Change-Certificates Feb 26, 2019
@saltiniroberto saltiniroberto changed the title Fix for bug in the validation of the Round-Change-Certificates Fix for bug in the validation of Round-Change-Certificates Feb 26, 2019
@saltiniroberto saltiniroberto force-pushed the fix_Round_Change_Message_Verification branch from 0eb7c19 to 6f9f74c Compare February 26, 2019 15:28
@@ -58,7 +59,8 @@ public boolean validateRoundChangeMessagesAndEnsureTargetRoundMatchesRoot(
final Collection<SignedData<RoundChangePayload>> roundChangeMsgs =
roundChangeCert.getRoundChangePayloads();

if (roundChangeMsgs.size() < quorum) {
if (roundChangeMsgs.stream().map(SignedData::getAuthor).collect(Collectors.toSet()).size()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be written a bit more simply: roundChangeMsgs.stream().map(SignedData::getAuthor).distinct().count()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice...I didn't know about distinct()

@@ -58,7 +59,8 @@ public boolean validateRoundChangeMessagesAndEnsureTargetRoundMatchesRoot(
final Collection<SignedData<RoundChangePayload>> roundChangeMsgs =
roundChangeCert.getRoundChangePayloads();

if (roundChangeMsgs.size() < quorum) {
if (roundChangeMsgs.stream().map(SignedData::getAuthor).collect(Collectors.toSet()).size()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should create a test which validates the fix (which appears necessary!)

@saltiniroberto
Copy link
Contributor Author

Replaced by #989

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants