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

Commit

Permalink
Rename functions in Ibft MessageValidator (#800)
Browse files Browse the repository at this point in the history
  • Loading branch information
rain-on authored Feb 6, 2019
1 parent 53cddb6 commit 17b6847
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 99 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ public ConsensusRoundIdentifier getRoundIdentifier() {
public boolean setProposedBlock(final Proposal msg) {

if (!proposalMessage.isPresent()) {
if (validator.addSignedProposalPayload(msg)) {
if (validator.validateProposal(msg)) {
proposalMessage = Optional.of(msg);
prepareMessages.removeIf(p -> !validator.validatePrepareMessage(p));
commitMessages.removeIf(p -> !validator.validateCommitMessage(p));
prepareMessages.removeIf(p -> !validator.validatePrepare(p));
commitMessages.removeIf(p -> !validator.validateCommit(p));
updateState();
return true;
}
Expand All @@ -78,15 +78,15 @@ public boolean setProposedBlock(final Proposal msg) {
}

public void addPrepareMessage(final Prepare msg) {
if (!proposalMessage.isPresent() || validator.validatePrepareMessage(msg)) {
if (!proposalMessage.isPresent() || validator.validatePrepare(msg)) {
prepareMessages.add(msg);
LOG.debug("Round state added prepare message prepare={}", msg);
}
updateState();
}

public void addCommitMessage(final Commit msg) {
if (!proposalMessage.isPresent() || validator.validateCommitMessage(msg)) {
if (!proposalMessage.isPresent() || validator.validateCommit(msg)) {
commitMessages.add(msg);
LOG.debug("Round state added commit message commit={}", msg);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ public MessageValidator(final SignedDataValidator signedDataValidator) {
this.signedDataValidator = signedDataValidator;
}

public boolean addSignedProposalPayload(final Proposal msg) {
return signedDataValidator.addSignedProposalPayload(msg.getSignedPayload());
public boolean validateProposal(final Proposal msg) {
return signedDataValidator.validateProposal(msg.getSignedPayload());
}

public boolean validatePrepareMessage(final Prepare msg) {
return signedDataValidator.validatePrepareMessage(msg.getSignedPayload());
public boolean validatePrepare(final Prepare msg) {
return signedDataValidator.validatePrepare(msg.getSignedPayload());
}

public boolean validateCommitMessage(final Commit msg) {
return signedDataValidator.validateCommmitMessage(msg.getSignedPayload());
public boolean validateCommit(final Commit msg) {
return signedDataValidator.validateCommit(msg.getSignedPayload());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public boolean validateNewRoundMessage(final SignedData<NewRoundPayload> msg) {
final SignedData<ProposalPayload> proposalPayload = payload.getProposalPayload();
final SignedDataValidator proposalValidator =
messageValidatorFactory.createAt(rootRoundIdentifier);
if (!proposalValidator.addSignedProposalPayload(proposalPayload)) {
if (!proposalValidator.validateProposal(proposalPayload)) {
LOG.info("Invalid NewRound message, embedded proposal failed validation");
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private boolean validatePrepareCertificate(
private boolean validateConsistencyOfPrepareCertificateMessages(
final PreparedCertificate certificate, final SignedDataValidator signedDataValidator) {

if (!signedDataValidator.addSignedProposalPayload(certificate.getProposalPayload())) {
if (!signedDataValidator.validateProposal(certificate.getProposalPayload())) {
LOG.info("Invalid RoundChange message, embedded Proposal message failed validation.");
return false;
}
Expand All @@ -102,7 +102,7 @@ private boolean validateConsistencyOfPrepareCertificateMessages(
}

for (final SignedData<PreparePayload> prepareMsg : certificate.getPreparePayloads()) {
if (!signedDataValidator.validatePrepareMessage(prepareMsg)) {
if (!signedDataValidator.validatePrepare(prepareMsg)) {
LOG.info("Invalid RoundChange message, embedded Prepare message failed validation.");
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ public SignedDataValidator(
this.protocolContext = protocolContext;
}

public boolean addSignedProposalPayload(final SignedData<ProposalPayload> msg) {
public boolean validateProposal(final SignedData<ProposalPayload> msg) {

if (proposal.isPresent()) {
return handleSubsequentProposal(proposal.get(), msg);
}

if (!validateSignedProposalPayload(msg)) {
if (!validateProposalSignedDataPayload(msg)) {
return false;
}

Expand All @@ -78,7 +78,7 @@ public boolean addSignedProposalPayload(final SignedData<ProposalPayload> msg) {
return true;
}

private boolean validateSignedProposalPayload(final SignedData<ProposalPayload> msg) {
private boolean validateProposalSignedDataPayload(final SignedData<ProposalPayload> msg) {

if (!msg.getPayload().getRoundIdentifier().equals(roundIdentifier)) {
LOG.info("Invalid Proposal message, does not match current round.");
Expand Down Expand Up @@ -124,7 +124,7 @@ private boolean handleSubsequentProposal(
return true;
}

public boolean validatePrepareMessage(final SignedData<PreparePayload> msg) {
public boolean validatePrepare(final SignedData<PreparePayload> msg) {
final String msgType = "Prepare";

if (!isMessageForCurrentRoundFromValidatorAndProposalAvailable(msg, msgType)) {
Expand All @@ -139,7 +139,7 @@ public boolean validatePrepareMessage(final SignedData<PreparePayload> msg) {
return validateDigestMatchesProposal(msg.getPayload().getDigest(), msgType);
}

public boolean validateCommmitMessage(final SignedData<CommitPayload> msg) {
public boolean validateCommit(final SignedData<CommitPayload> msg) {
final String msgType = "Commit";

if (!isMessageForCurrentRoundFromValidatorAndProposalAvailable(msg, msgType)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ public void setup() {
buildCreatedBlock();

final MessageValidator messageValidator = mock(MessageValidator.class);
when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validateCommitMessage(any())).thenReturn(true);
when(messageValidator.validatePrepareMessage(any())).thenReturn(true);
when(messageValidator.validateProposal(any())).thenReturn(true);
when(messageValidator.validateCommit(any())).thenReturn(true);
when(messageValidator.validatePrepare(any())).thenReturn(true);
when(finalState.getTransmitter()).thenReturn(messageTransmitter);
when(finalState.getBlockTimer()).thenReturn(blockTimer);
when(finalState.getRoundTimer()).thenReturn(roundTimer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ public void setup() {
worldStateArchive,
new IbftContext(new VoteTally(emptyList()), new VoteProposer()));

when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validatePrepareMessage(any())).thenReturn(true);
when(messageValidator.validateCommitMessage(any())).thenReturn(true);
when(messageValidator.validateProposal(any())).thenReturn(true);
when(messageValidator.validatePrepare(any())).thenReturn(true);
when(messageValidator.validateCommit(any())).thenReturn(true);

proposedExtraData =
new IbftExtraData(BytesValue.wrap(new byte[32]), emptyList(), empty(), 0, emptyList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void defaultRoundIsNotPreparedOrCommittedAndHasNoPreparedCertificate() {

@Test
public void ifProposalMessageFailsValidationMethodReturnsFalse() {
when(messageValidator.addSignedProposalPayload(any())).thenReturn(false);
when(messageValidator.validateProposal(any())).thenReturn(false);
final RoundState roundState = new RoundState(roundIdentifier, 1, messageValidator);

final Proposal proposal =
Expand All @@ -90,7 +90,7 @@ public void ifProposalMessageFailsValidationMethodReturnsFalse() {

@Test
public void singleValidatorIsPreparedWithJustProposal() {
when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validateProposal(any())).thenReturn(true);
final RoundState roundState = new RoundState(roundIdentifier, 1, messageValidator);

final Proposal proposal =
Expand All @@ -111,8 +111,8 @@ public void singleValidatorIsPreparedWithJustProposal() {

@Test
public void singleValidatorRequiresCommitMessageToBeCommitted() {
when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validateCommitMessage(any())).thenReturn(true);
when(messageValidator.validateProposal(any())).thenReturn(true);
when(messageValidator.validateCommit(any())).thenReturn(true);

final RoundState roundState = new RoundState(roundIdentifier, 1, messageValidator);

Expand All @@ -139,8 +139,8 @@ public void singleValidatorRequiresCommitMessageToBeCommitted() {

@Test
public void prepareMessagesCanBeReceivedPriorToProposal() {
when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validatePrepareMessage(any())).thenReturn(true);
when(messageValidator.validateProposal(any())).thenReturn(true);
when(messageValidator.validatePrepare(any())).thenReturn(true);

final RoundState roundState = new RoundState(roundIdentifier, 3, messageValidator);

Expand Down Expand Up @@ -180,13 +180,13 @@ public void invalidPriorPrepareMessagesAreDiscardedUponSubsequentProposal() {
// 'prepared'.
final RoundState roundState = new RoundState(roundIdentifier, 3, messageValidator);

when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validatePrepareMessage(firstPrepare)).thenReturn(true);
when(messageValidator.validatePrepareMessage(secondPrepare)).thenReturn(false);
when(messageValidator.validateProposal(any())).thenReturn(true);
when(messageValidator.validatePrepare(firstPrepare)).thenReturn(true);
when(messageValidator.validatePrepare(secondPrepare)).thenReturn(false);

roundState.addPrepareMessage(firstPrepare);
roundState.addPrepareMessage(secondPrepare);
verify(messageValidator, never()).validatePrepareMessage(any());
verify(messageValidator, never()).validatePrepare(any());

final Proposal proposal =
validatorMessageFactories.get(0).createProposal(roundIdentifier, block);
Expand All @@ -210,9 +210,9 @@ public void prepareMessageIsValidatedAgainstExitingProposal() {
final Proposal proposal =
validatorMessageFactories.get(0).createProposal(roundIdentifier, block);

when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validatePrepareMessage(firstPrepare)).thenReturn(false);
when(messageValidator.validatePrepareMessage(secondPrepare)).thenReturn(true);
when(messageValidator.validateProposal(any())).thenReturn(true);
when(messageValidator.validatePrepare(firstPrepare)).thenReturn(false);
when(messageValidator.validatePrepare(secondPrepare)).thenReturn(true);

roundState.setProposedBlock(proposal);
assertThat(roundState.isPrepared()).isFalse();
Expand All @@ -226,8 +226,8 @@ public void prepareMessageIsValidatedAgainstExitingProposal() {

@Test
public void commitSealsAreExtractedFromReceivedMessages() {
when(messageValidator.addSignedProposalPayload(any())).thenReturn(true);
when(messageValidator.validateCommitMessage(any())).thenReturn(true);
when(messageValidator.validateProposal(any())).thenReturn(true);
when(messageValidator.validateCommit(any())).thenReturn(true);

final RoundState roundState = new RoundState(roundIdentifier, 2, messageValidator);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ public class MessageValidatorTest {

@Before
public void setup() {
when(signedDataValidator.addSignedProposalPayload(any())).thenReturn(true);
when(signedDataValidator.validatePrepareMessage(any())).thenReturn(true);
when(signedDataValidator.validateCommmitMessage(any())).thenReturn(true);
when(signedDataValidator.validateProposal(any())).thenReturn(true);
when(signedDataValidator.validatePrepare(any())).thenReturn(true);
when(signedDataValidator.validateCommit(any())).thenReturn(true);
}

@Test
Expand All @@ -73,13 +73,13 @@ public void messageValidatorDefersToUnderlyingSignedDataValidator() {
messageFactory.createCommit(
roundIdentifier, block.getHash(), SECP256K1.sign(block.getHash(), keyPair));

messageValidator.addSignedProposalPayload(proposal);
verify(signedDataValidator, times(1)).addSignedProposalPayload(proposal.getSignedPayload());
messageValidator.validateProposal(proposal);
verify(signedDataValidator, times(1)).validateProposal(proposal.getSignedPayload());

messageValidator.validatePrepareMessage(prepare);
verify(signedDataValidator, times(1)).validatePrepareMessage(prepare.getSignedPayload());
messageValidator.validatePrepare(prepare);
verify(signedDataValidator, times(1)).validatePrepare(prepare.getSignedPayload());

messageValidator.validateCommitMessage(commit);
verify(signedDataValidator, times(1)).validateCommmitMessage(commit.getSignedPayload());
messageValidator.validateCommit(commit);
verify(signedDataValidator, times(1)).validateCommit(commit.getSignedPayload());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ public void setup() {
when(proposerSelector.selectProposerForRound(any())).thenReturn(proposerAddress);

when(validatorFactory.createAt(any())).thenReturn(signedDataValidator);
when(signedDataValidator.addSignedProposalPayload(any())).thenReturn(true);
when(signedDataValidator.validatePrepareMessage(any())).thenReturn(true);
when(signedDataValidator.validateProposal(any())).thenReturn(true);
when(signedDataValidator.validatePrepare(any())).thenReturn(true);

validator =
new NewRoundPayloadValidator(
Expand Down Expand Up @@ -216,7 +216,7 @@ public void invalidEmbeddedRoundChangeMessageFails() {
msgBuilder.setRoundChangeCertificate(roundChangeBuilder.buildCertificate());

// The prepare Message in the RoundChange Cert will be deemed illegal.
when(signedDataValidator.validatePrepareMessage(any())).thenReturn(false);
when(signedDataValidator.validatePrepare(any())).thenReturn(false);

final NewRound msg = signPayload(msgBuilder.build(), proposerKey);

Expand Down Expand Up @@ -281,7 +281,7 @@ public void lastestPreparedCertificateMatchesNewRoundProposalIsSuccessful() {

@Test
public void embeddedProposalFailsValidation() {
when(signedDataValidator.addSignedProposalPayload(any())).thenReturn(false, true);
when(signedDataValidator.validateProposal(any())).thenReturn(false, true);

final Proposal proposal = proposerMessageFactory.createProposal(roundIdentifier, proposedBlock);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ public void setup() {

// By default, have all basic messages being valid thus any failures are attributed to logic
// in the RoundChangePayloadValidator
when(basicValidator.addSignedProposalPayload(any())).thenReturn(true);
when(basicValidator.validatePrepareMessage(any())).thenReturn(true);
when(basicValidator.validateProposal(any())).thenReturn(true);
when(basicValidator.validatePrepare(any())).thenReturn(true);
}

@Test
Expand Down Expand Up @@ -104,15 +104,14 @@ public void roundChangeContainingInvalidProposalFails() {
final RoundChange msg =
proposerMessageFactory.createRoundChange(targetRound, Optional.of(preparedRoundArtifacts));

when(basicValidator.addSignedProposalPayload(any())).thenReturn(false);
when(basicValidator.validateProposal(any())).thenReturn(false);

assertThat(validator.validateRoundChange(msg.getSignedPayload())).isFalse();
verify(validatorFactory, times(1))
.createAt(prepareCertificate.getProposalPayload().getPayload().getRoundIdentifier());
verify(basicValidator, times(1))
.addSignedProposalPayload(prepareCertificate.getProposalPayload());
verify(basicValidator, never()).validatePrepareMessage(any());
verify(basicValidator, never()).validateCommmitMessage(any());
verify(basicValidator, times(1)).validateProposal(prepareCertificate.getProposalPayload());
verify(basicValidator, never()).validatePrepare(any());
verify(basicValidator, never()).validateCommit(any());
}

@Test
Expand All @@ -124,7 +123,7 @@ public void roundChangeContainingValidProposalButNoPrepareMessagesFails() {
final RoundChange msg =
proposerMessageFactory.createRoundChange(targetRound, Optional.of(preparedRoundArtifacts));

when(basicValidator.addSignedProposalPayload(any())).thenReturn(true);
when(basicValidator.validateProposal(any())).thenReturn(true);
assertThat(validator.validateRoundChange(msg.getSignedPayload())).isFalse();
}

Expand All @@ -136,16 +135,16 @@ public void roundChangeInvalidPrepareMessageFromProposerFails() {
proposerMessageFactory.createProposal(currentRound, block),
Lists.newArrayList(prepareMsg));

when(basicValidator.addSignedProposalPayload(any())).thenReturn(true);
when(basicValidator.validatePrepareMessage(any())).thenReturn(false);
when(basicValidator.validateProposal(any())).thenReturn(true);
when(basicValidator.validatePrepare(any())).thenReturn(false);

final RoundChange msg =
proposerMessageFactory.createRoundChange(targetRound, Optional.of(preparedRoundArtifacts));

assertThat(validator.validateRoundChange(msg.getSignedPayload())).isFalse();

verify(basicValidator, times(1)).validatePrepareMessage(prepareMsg.getSignedPayload());
verify(basicValidator, never()).validateCommmitMessage(any());
verify(basicValidator, times(1)).validatePrepare(prepareMsg.getSignedPayload());
verify(basicValidator, never()).validateCommit(any());
}

@Test
Expand All @@ -157,7 +156,7 @@ public void roundChangeWithDifferentSequenceNumberFails() {
proposerMessageFactory.createRoundChange(latterRoundIdentifier, Optional.empty());

assertThat(validator.validateRoundChange(msg.getSignedPayload())).isFalse();
verify(basicValidator, never()).validatePrepareMessage(any());
verify(basicValidator, never()).validatePrepare(any());
}

@Test
Expand All @@ -177,8 +176,8 @@ public void roundChangeWithProposalFromARoundAheadOfRoundChangeTargetFails() {

assertThat(validator.validateRoundChange(msg.getSignedPayload())).isFalse();
verify(validatorFactory, never()).createAt(any());
verify(basicValidator, never()).validatePrepareMessage(prepareMsg.getSignedPayload());
verify(basicValidator, never()).validateCommmitMessage(any());
verify(basicValidator, never()).validatePrepare(prepareMsg.getSignedPayload());
verify(basicValidator, never()).validateCommit(any());
}

@Test
Expand All @@ -194,15 +193,13 @@ public void roundChangeWithPastProposalForCurrentHeightIsSuccessful() {
final RoundChange msg =
proposerMessageFactory.createRoundChange(targetRound, Optional.of(preparedRoundArtifacts));

when(basicValidator.addSignedProposalPayload(prepareCertificate.getProposalPayload()))
.thenReturn(true);
when(basicValidator.validatePrepareMessage(prepareMsg.getSignedPayload())).thenReturn(true);
when(basicValidator.validateProposal(prepareCertificate.getProposalPayload())).thenReturn(true);
when(basicValidator.validatePrepare(prepareMsg.getSignedPayload())).thenReturn(true);

assertThat(validator.validateRoundChange(msg.getSignedPayload())).isTrue();
verify(validatorFactory, times(1))
.createAt(prepareCertificate.getProposalPayload().getPayload().getRoundIdentifier());
verify(basicValidator, times(1))
.addSignedProposalPayload(prepareCertificate.getProposalPayload());
verify(basicValidator, times(1)).validatePrepareMessage(prepareMsg.getSignedPayload());
verify(basicValidator, times(1)).validateProposal(prepareCertificate.getProposalPayload());
verify(basicValidator, times(1)).validatePrepare(prepareMsg.getSignedPayload());
}
}
Loading

0 comments on commit 17b6847

Please sign in to comment.