From 5425a92c0d557dc42b1162615dddc8fa44d73fcd Mon Sep 17 00:00:00 2001 From: tmohay <37158202+rain-on@users.noreply.github.com> Date: Mon, 17 Dec 2018 21:05:27 +1100 Subject: [PATCH] Allow IBFT Round to be created using PreparedCert (#429) When the local node is to be the proposer for a new round (due to round change), the new Ibft Round is to be created using the latest received PreparedCertificate, and a NewRound message multicast to all validators. It should be noted, the block in the NewRound Proposal must be modified from that received in the PreparedCertificate such that it contains the executing round number. --- .../pantheon/consensus/ibft/IbftHelpers.java | 28 +++++++ .../ibft/statemachine/IbftRound.java | 47 +++++++++++ .../ibft/statemachine/IbftRoundFactory.java | 15 +++- .../validation/NewRoundMessageValidator.java | 26 +----- .../consensus/ibft/IbftHelpersTest.java | 83 +++++++++++++++++++ .../ibft/statemachine/IbftRoundTest.java | 72 ++++++++++++++++ 6 files changed, 244 insertions(+), 27 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpers.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpers.java index 9e402c2284..24b0e1834c 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpers.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpers.java @@ -12,6 +12,9 @@ */ package tech.pegasys.pantheon.consensus.ibft; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparedCertificate; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangePayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; import tech.pegasys.pantheon.ethereum.core.Block; import tech.pegasys.pantheon.ethereum.core.BlockHeader; @@ -20,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.core.Util; import java.util.Collection; +import java.util.Optional; public class IbftHelpers { @@ -51,4 +55,28 @@ public class IbftHelpers { return new Block(sealedHeader, block.getBody()); } + + public static Optional findLatestPreparedCertificate( + final Collection> msgs) { + + Optional result = Optional.empty(); + + for (SignedData roundChangeMsg : msgs) { + final RoundChangePayload payload = roundChangeMsg.getPayload(); + if (payload.getPreparedCertificate().isPresent()) { + if (!result.isPresent()) { + result = payload.getPreparedCertificate(); + } else { + final PreparedCertificate currentLatest = result.get(); + final PreparedCertificate nextCert = payload.getPreparedCertificate().get(); + + if (currentLatest.getProposalPayload().getPayload().getRoundIdentifier().getRoundNumber() + < nextCert.getProposalPayload().getPayload().getRoundIdentifier().getRoundNumber()) { + result = Optional.of(nextCert); + } + } + } + } + return result; + } } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java index b18f862935..35287ddb51 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java @@ -12,6 +12,8 @@ */ package tech.pegasys.pantheon.consensus.ibft.statemachine; +import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.findLatestPreparedCertificate; + import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing; import tech.pegasys.pantheon.consensus.ibft.IbftContext; @@ -23,6 +25,7 @@ import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparePayload; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparedCertificate; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangeCertificate; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; @@ -31,6 +34,7 @@ import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.chain.MinedBlockObserver; import tech.pegasys.pantheon.ethereum.core.Block; import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.core.BlockHeaderBuilder; import tech.pegasys.pantheon.ethereum.core.BlockImporter; import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.mainnet.HeaderValidationMode; @@ -86,6 +90,49 @@ public class IbftRound { messageFactory.createSignedProposalPayload(roundState.getRoundIdentifier(), block)); } + public void startRoundWith( + final RoundChangeCertificate roundChangeCertificate, final long headerTimestamp) { + final Optional latestCertificate = + findLatestPreparedCertificate(roundChangeCertificate.getRoundChangePayloads()); + if (!latestCertificate.isPresent()) { + final Block block = blockCreator.createBlock(headerTimestamp); + transmitter.multicastNewRound( + getRoundIdentifier(), + roundChangeCertificate, + messageFactory.createSignedProposalPayload(getRoundIdentifier(), block)); + } else { + final SignedData proposal = + createProposalFromPreparedCertificate(latestCertificate.get()); + transmitter.multicastNewRound(getRoundIdentifier(), roundChangeCertificate, proposal); + updateStateWithProposedBlock(proposal); + } + } + + private SignedData createProposalFromPreparedCertificate( + final PreparedCertificate preparedCertificate) { + final Block block = preparedCertificate.getProposalPayload().getPayload().getBlock(); + + final IbftExtraData prevExtraData = IbftExtraData.decode(block.getHeader().getExtraData()); + final IbftExtraData extraDataToPublish = + new IbftExtraData( + prevExtraData.getVanityData(), + prevExtraData.getSeals(), + prevExtraData.getVote(), + getRoundIdentifier().getRoundNumber(), + prevExtraData.getValidators()); + + final BlockHeaderBuilder headerBuilder = BlockHeaderBuilder.fromHeader(block.getHeader()); + headerBuilder + .extraData(extraDataToPublish.encode()) + .blockHashFunction( + blockHeader -> + IbftBlockHashing.calculateDataHashForCommittedSeal( + blockHeader, extraDataToPublish)); + final BlockHeader newHeader = headerBuilder.buildBlockHeader(); + final Block newBlock = new Block(newHeader, block.getBody()); + return messageFactory.createSignedProposalPayload(getRoundIdentifier(), newBlock); + } + public void handleProposalMessage(final SignedData msg) { LOG.info("Received a Proposal message."); final Block block = msg.getPayload().getBlock(); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java index 342d73698e..8d31f7ceca 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundFactory.java @@ -48,7 +48,7 @@ public class IbftRoundFactory { new ConsensusRoundIdentifier(nextBlockHeight, round); final IbftBlockCreator blockCreator = blockCreatorFactory.create(parentHeader, round); - final RoundState roundContext = + final RoundState roundState = new RoundState( roundIdentifier, finalState.getQuorumSize(), @@ -60,11 +60,20 @@ public class IbftRoundFactory { protocolContext, parentHeader)); + return createNewRoundWithState(parentHeader, roundState); + } + + public IbftRound createNewRoundWithState( + final BlockHeader parentHeader, final RoundState roundState) { + final ConsensusRoundIdentifier roundIdentifier = roundState.getRoundIdentifier(); + final IbftBlockCreator blockCreator = + blockCreatorFactory.create(parentHeader, roundIdentifier.getRoundNumber()); + return new IbftRound( - roundContext, + roundState, blockCreator, protocolContext, - protocolSchedule.getByBlockNumber(nextBlockHeight).getBlockImporter(), + protocolSchedule.getByBlockNumber(roundIdentifier.getSequenceNumber()).getBlockImporter(), minedBlockObservers, finalState.getNodeKeys(), finalState.getMessageFactory(), diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java index ff35f2135b..5135bcc028 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundMessageValidator.java @@ -12,6 +12,8 @@ */ package tech.pegasys.pantheon.consensus.ibft.validation; +import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.findLatestPreparedCertificate; + import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.blockcreation.ProposerSelector; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.NewRoundPayload; @@ -160,28 +162,4 @@ public class NewRoundMessageValidator { return true; } - - private Optional findLatestPreparedCertificate( - final Collection> msgs) { - - Optional result = Optional.empty(); - - for (SignedData roundChangeMsg : msgs) { - final RoundChangePayload payload = roundChangeMsg.getPayload(); - if (payload.getPreparedCertificate().isPresent()) { - if (!result.isPresent()) { - result = Optional.of(payload.getPreparedCertificate().get()); - } else { - final PreparedCertificate currentLatest = result.get(); - final PreparedCertificate nextCert = payload.getPreparedCertificate().get(); - - if (currentLatest.getProposalPayload().getPayload().getRoundIdentifier().getRoundNumber() - < nextCert.getProposalPayload().getPayload().getRoundIdentifier().getRoundNumber()) { - result = Optional.of(nextCert); - } - } - } - } - return result; - } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpersTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpersTest.java index 7a29047274..1c06ff83c6 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpersTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/IbftHelpersTest.java @@ -13,8 +13,21 @@ package tech.pegasys.pantheon.consensus.ibft; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.calculateRequiredValidatorQuorum; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparedCertificate; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; +import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; +import tech.pegasys.pantheon.ethereum.core.Block; +import tech.pegasys.pantheon.ethereum.core.Hash; + +import java.util.Optional; + +import com.google.common.collect.Lists; import org.junit.Test; public class IbftHelpersTest { @@ -63,4 +76,74 @@ public class IbftHelpersTest { public void calculateRequiredValidatorQuorum20Validator() { assertThat(calculateRequiredValidatorQuorum(20)).isEqualTo(14); } + + @Test + public void latestPreparedCertificateIsExtractedFromRoundChangeCertificate() { + // NOTE: This function does not validate that all RoundCHanges/Prepares etc. come from valid + // sources, it is only responsible for determine which of the list or RoundChange messages + // contains the newest + // NOTE: This capability is tested as part of the NewRoundMessageValidationTests. + final KeyPair proposerKey = KeyPair.generate(); + final MessageFactory proposerMessageFactory = new MessageFactory(proposerKey); + final Block proposedBlock = mock(Block.class); + when(proposedBlock.getHash()).thenReturn(Hash.fromHexStringLenient("1")); + final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(1, 4); + + final ConsensusRoundIdentifier preparedRound = TestHelpers.createFrom(roundIdentifier, 0, -1); + final SignedData differentProposal = + proposerMessageFactory.createSignedProposalPayload(preparedRound, proposedBlock); + + final Optional latterPreparedCert = + Optional.of( + new PreparedCertificate( + differentProposal, + Lists.newArrayList( + proposerMessageFactory.createSignedPreparePayload( + roundIdentifier, proposedBlock.getHash()), + proposerMessageFactory.createSignedPreparePayload( + roundIdentifier, proposedBlock.getHash())))); + + // An earlier PrepareCert is added to ensure the path to find the latest PrepareCert + // is correctly followed. + final ConsensusRoundIdentifier earlierPreparedRound = + TestHelpers.createFrom(roundIdentifier, 0, -2); + final SignedData earlierProposal = + proposerMessageFactory.createSignedProposalPayload(earlierPreparedRound, proposedBlock); + final Optional earlierPreparedCert = + Optional.of( + new PreparedCertificate( + earlierProposal, + Lists.newArrayList( + proposerMessageFactory.createSignedPreparePayload( + earlierPreparedRound, proposedBlock.getHash()), + proposerMessageFactory.createSignedPreparePayload( + earlierPreparedRound, proposedBlock.getHash())))); + + final Optional newestCert = + IbftHelpers.findLatestPreparedCertificate( + Lists.newArrayList( + proposerMessageFactory.createSignedRoundChangePayload( + roundIdentifier, earlierPreparedCert), + proposerMessageFactory.createSignedRoundChangePayload( + roundIdentifier, latterPreparedCert))); + + assertThat(newestCert).isEqualTo(latterPreparedCert); + } + + @Test + public void allRoundChangeHaveNoPreparedReturnsEmptyOptional() { + final KeyPair proposerKey = KeyPair.generate(); + final MessageFactory proposerMessageFactory = new MessageFactory(proposerKey); + final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(1, 4); + + final Optional newestCert = + IbftHelpers.findLatestPreparedCertificate( + Lists.newArrayList( + proposerMessageFactory.createSignedRoundChangePayload( + roundIdentifier, Optional.empty()), + proposerMessageFactory.createSignedRoundChangePayload( + roundIdentifier, Optional.empty()))); + + assertThat(newestCert).isEmpty(); + } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java index 38f5786b08..9715954e9d 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java @@ -15,6 +15,7 @@ package tech.pegasys.pantheon.consensus.ibft.statemachine; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -28,6 +29,10 @@ import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreator; import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.MessageFactory; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.PreparedCertificate; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.ProposalPayload; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.RoundChangeCertificate; +import tech.pegasys.pantheon.consensus.ibft.ibftmessagedata.SignedData; import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidator; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; @@ -53,6 +58,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.Captor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -73,6 +79,8 @@ public class IbftRoundTest { @Mock private IbftBlockCreator blockCreator; @Mock private MessageValidator messageValidator; + @Captor private ArgumentCaptor> payloadArgCaptor; + private Block proposedBlock; private IbftExtraData proposedExtraData; @@ -248,4 +256,68 @@ public class IbftRoundTest { roundIdentifier, proposedBlock.getHash(), remoteCommitSeal)); verify(blockImporter, times(1)).importBlock(any(), any(), any()); } + + @Test + public void aNewRoundMessageWithAnewBlockIsSentUponReceptionOfARoundChangeWithNoCertificate() { + final RoundState roundState = new RoundState(roundIdentifier, 2, messageValidator); + final IbftRound round = + new IbftRound( + roundState, + blockCreator, + protocolContext, + blockImporter, + subscribers, + localNodeKeys, + messageFactory, + transmitter); + + final RoundChangeCertificate roundChangeCertificate = + new RoundChangeCertificate(Collections.emptyList()); + + round.startRoundWith(roundChangeCertificate, 15); + verify(transmitter, times(1)) + .multicastNewRound(eq(roundIdentifier), eq(roundChangeCertificate), any()); + } + + @Test + public void aNewRoundMessageWithTheSameBlockIsSentUponReceptionOfARoundChangeWithCertificate() { + SignedData mockedSentMessage = + messageFactory.createSignedProposalPayload(roundIdentifier, proposedBlock); + + final ConsensusRoundIdentifier priorRoundChange = new ConsensusRoundIdentifier(1, 0); + final RoundState roundState = new RoundState(roundIdentifier, 2, messageValidator); + final IbftRound round = + new IbftRound( + roundState, + blockCreator, + protocolContext, + blockImporter, + subscribers, + localNodeKeys, + messageFactory, + transmitter); + + final RoundChangeCertificate roundChangeCertificate = + new RoundChangeCertificate( + Collections.singletonList( + messageFactory.createSignedRoundChangePayload( + roundIdentifier, + Optional.of( + new PreparedCertificate( + messageFactory.createSignedProposalPayload( + priorRoundChange, proposedBlock), + Collections + .emptyList()))))); // NOTE: IbftRound assumes the prepare's are + // valid + + round.startRoundWith(roundChangeCertificate, 15); + verify(transmitter, times(1)) + .multicastNewRound( + eq(roundIdentifier), eq(roundChangeCertificate), payloadArgCaptor.capture()); + + final IbftExtraData proposedExtraData = + IbftExtraData.decode( + payloadArgCaptor.getValue().getPayload().getBlock().getHeader().getExtraData()); + assertThat(proposedExtraData.getRound()).isEqualTo(roundIdentifier.getRoundNumber()); + } }