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

Update RoundChangeManager to use flattened message #742

Merged
merged 3 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ public void send(final Message message) {
decodedMessage = RoundChangeMessageData.fromMessageData(messageData).decode();
break;
case IbftV2.NEW_ROUND:
decodedMessage =
NewRoundMessageData.fromMessageData(messageData).decode().getSignedPayload();
decodedMessage = NewRoundMessageData.fromMessageData(messageData).decode();
break;
default:
throw new IllegalArgumentException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public void handleRoundChangePayload(final RoundChange message) {
}

final Optional<RoundChangeCertificate> result =
roundChangeManager.appendRoundChangeMessage(message.getSignedPayload());
roundChangeManager.appendRoundChangeMessage(message);
if (result.isPresent()) {
if (messageAge == FUTURE_ROUND) {
startNewRound(targetRound.getRoundNumber());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@
package tech.pegasys.pantheon.consensus.ibft.statemachine;

import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangePayload;
import tech.pegasys.pantheon.consensus.ibft.payload.SignedData;
import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangeMessageValidator;
import tech.pegasys.pantheon.ethereum.core.Address;

import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.Maps;
Expand All @@ -43,16 +43,15 @@ public static class RoundChangeStatus {
private final long quorum;

// Store only 1 round change per round per validator
@VisibleForTesting
final Map<Address, SignedData<RoundChangePayload>> receivedMessages = Maps.newLinkedHashMap();
@VisibleForTesting final Map<Address, RoundChange> receivedMessages = Maps.newLinkedHashMap();

private boolean actioned = false;

public RoundChangeStatus(final long quorum) {
this.quorum = quorum;
}

public void addMessage(final SignedData<RoundChangePayload> msg) {
public void addMessage(final RoundChange msg) {
if (!actioned) {
receivedMessages.putIfAbsent(msg.getAuthor(), msg);
}
Expand All @@ -65,7 +64,12 @@ public boolean roundChangeReady() {
public RoundChangeCertificate createRoundChangeCertificate() {
if (roundChangeReady()) {
actioned = true;
return new RoundChangeCertificate(receivedMessages.values());
return new RoundChangeCertificate(
receivedMessages
.values()
.stream()
.map(RoundChange::getSignedPayload)
.collect(Collectors.toList()));
} else {
throw new IllegalStateException("Unable to create RoundChangeCertificate at this time.");
}
Expand Down Expand Up @@ -93,8 +97,7 @@ public RoundChangeManager(
* @return Empty if the round change threshold hasn't been hit, otherwise a round change
* certificate
*/
public Optional<RoundChangeCertificate> appendRoundChangeMessage(
final SignedData<RoundChangePayload> msg) {
public Optional<RoundChangeCertificate> appendRoundChangeMessage(final RoundChange msg) {

if (!isMessageValid(msg)) {
LOG.info("RoundChange message was invalid.");
Expand All @@ -110,12 +113,12 @@ public Optional<RoundChangeCertificate> appendRoundChangeMessage(
return Optional.empty();
}

private boolean isMessageValid(final SignedData<RoundChangePayload> msg) {
return roundChangeMessageValidator.validateMessage(msg);
private boolean isMessageValid(final RoundChange msg) {
return roundChangeMessageValidator.validateMessage(msg.getSignedPayload());
}

private RoundChangeStatus storeRoundChangeMessage(final SignedData<RoundChangePayload> msg) {
final ConsensusRoundIdentifier msgTargetRound = msg.getPayload().getRoundIdentifier();
private RoundChangeStatus storeRoundChangeMessage(final RoundChange msg) {
final ConsensusRoundIdentifier msgTargetRound = msg.getRoundIdentifier();

final RoundChangeStatus roundChangeStatus =
roundChangeCache.computeIfAbsent(msgTargetRound, ignored -> new RoundChangeStatus(quorum));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public void onRoundChangeReceptionRoundChangeManagerIsInvokedAndNewRoundStarted(

manager.handleRoundChangePayload(roundChange);

verify(roundChangeManager, times(1)).appendRoundChangeMessage(roundChange.getSignedPayload());
verify(roundChangeManager, times(1)).appendRoundChangeMessage(roundChange);
verify(roundFactory, times(1))
.createNewRound(any(), eq(futureRoundIdentifier.getRoundNumber()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
import tech.pegasys.pantheon.consensus.ibft.IbftContext;
import tech.pegasys.pantheon.consensus.ibft.IbftHelpers;
import tech.pegasys.pantheon.consensus.ibft.TestHelpers;
import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory;
import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload;
import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate;
import tech.pegasys.pantheon.consensus.ibft.payload.ProposalPayload;
import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangePayload;
import tech.pegasys.pantheon.consensus.ibft.payload.SignedData;
import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidator;
import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangeMessageValidator;
Expand Down Expand Up @@ -126,13 +126,13 @@ public void setup() {
manager = new RoundChangeManager(2, roundChangeMessageValidator);
}

private SignedData<RoundChangePayload> makeRoundChangeMessage(
private RoundChange makeRoundChangeMessage(
final KeyPair key, final ConsensusRoundIdentifier round) {
MessageFactory messageFactory = new MessageFactory(key);
return messageFactory.createSignedRoundChangePayload(round, Optional.empty());
return new RoundChange(messageFactory.createSignedRoundChangePayload(round, Optional.empty()));
}

private SignedData<RoundChangePayload> makeRoundChangeMessageWithPreparedCert(
private RoundChange makeRoundChangeMessageWithPreparedCert(
final KeyPair key,
final ConsensusRoundIdentifier round,
final List<KeyPair> prepareProviders) {
Expand All @@ -158,48 +158,44 @@ private SignedData<RoundChangePayload> makeRoundChangeMessageWithPreparedCert(

final PreparedCertificate cert = new PreparedCertificate(proposal, preparePayloads);

return messageFactory.createSignedRoundChangePayload(round, Optional.of(cert));
return new RoundChange(messageFactory.createSignedRoundChangePayload(round, Optional.of(cert)));
}

@Test
public void rejectsInvalidRoundChangeMessage() {
SignedData<RoundChangePayload> roundChangeData = makeRoundChangeMessage(nonValidatorKey, ri1);
final RoundChange roundChangeData = makeRoundChangeMessage(nonValidatorKey, ri1);
assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
assertThat(manager.roundChangeCache.get(ri1)).isNull();
}

@Test
public void acceptsValidRoundChangeMessage() {
SignedData<RoundChangePayload> roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
final RoundChange roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
assertThat(manager.roundChangeCache.get(ri2).receivedMessages.size()).isEqualTo(1);
}

@Test
public void doesntAcceptDuplicateValidRoundChangeMessage() {
SignedData<RoundChangePayload> roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
final RoundChange roundChangeData = makeRoundChangeMessage(proposerKey, ri2);
assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
assertThat(manager.roundChangeCache.get(ri2).receivedMessages.size()).isEqualTo(1);
}

@Test
public void becomesReadyAtThreshold() {
SignedData<RoundChangePayload> roundChangeDataProposer =
makeRoundChangeMessage(proposerKey, ri2);
SignedData<RoundChangePayload> roundChangeDataValidator1 =
makeRoundChangeMessage(validator1Key, ri2);
final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri2);
final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri2);
assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
.isEqualTo(Optional.empty());
assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1).isPresent()).isTrue();
}

@Test
public void doesntReachReadyWhenSuppliedWithDifferentRounds() {
SignedData<RoundChangePayload> roundChangeDataProposer =
makeRoundChangeMessage(proposerKey, ri2);
SignedData<RoundChangePayload> roundChangeDataValidator1 =
makeRoundChangeMessage(validator1Key, ri3);
final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri2);
final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri3);
assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
.isEqualTo(Optional.empty());
assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1))
Expand All @@ -210,12 +206,9 @@ public void doesntReachReadyWhenSuppliedWithDifferentRounds() {

@Test
public void discardsRoundPreviousToThatRequested() {
SignedData<RoundChangePayload> roundChangeDataProposer =
makeRoundChangeMessage(proposerKey, ri1);
SignedData<RoundChangePayload> roundChangeDataValidator1 =
makeRoundChangeMessage(validator1Key, ri2);
SignedData<RoundChangePayload> roundChangeDataValidator2 =
makeRoundChangeMessage(validator2Key, ri3);
final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri1);
final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri2);
final RoundChange roundChangeDataValidator2 = makeRoundChangeMessage(validator2Key, ri3);
assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
.isEqualTo(Optional.empty());
assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1))
Expand All @@ -230,12 +223,9 @@ public void discardsRoundPreviousToThatRequested() {

@Test
public void stopsAcceptingMessagesAfterReady() {
SignedData<RoundChangePayload> roundChangeDataProposer =
makeRoundChangeMessage(proposerKey, ri2);
SignedData<RoundChangePayload> roundChangeDataValidator1 =
makeRoundChangeMessage(validator1Key, ri2);
SignedData<RoundChangePayload> roundChangeDataValidator2 =
makeRoundChangeMessage(validator2Key, ri2);
final RoundChange roundChangeDataProposer = makeRoundChangeMessage(proposerKey, ri2);
final RoundChange roundChangeDataValidator1 = makeRoundChangeMessage(validator1Key, ri2);
final RoundChange roundChangeDataValidator2 = makeRoundChangeMessage(validator2Key, ri2);
assertThat(manager.appendRoundChangeMessage(roundChangeDataProposer))
.isEqualTo(Optional.empty());
assertThat(manager.appendRoundChangeMessage(roundChangeDataValidator1).isPresent()).isTrue();
Expand All @@ -252,7 +242,7 @@ public void roundChangeMessagesWithPreparedCertificateMustHaveSufficientPrepareM
// There are 3 validators, therefore, should only need 2 prepare message to be acceptable.

// These tests are run at ri2, such that validators can be found for past round at ri1.
SignedData<RoundChangePayload> roundChangeData =
RoundChange roundChangeData =
makeRoundChangeMessageWithPreparedCert(proposerKey, ri2, Collections.emptyList());
assertThat(manager.appendRoundChangeMessage(roundChangeData)).isEmpty();
assertThat(manager.roundChangeCache.get(ri2)).isNull();
Expand Down