From 95c7a76fce11fbc478366ff4d294cc08844d5f15 Mon Sep 17 00:00:00 2001 From: tmohay Date: Mon, 4 Feb 2019 13:10:26 +1100 Subject: [PATCH 1/3] Separate round change reception from RoundChangeCertificate IBFT2.1 requires that message content be separated, thus concepts at the message level must not leak into the business logic - eg RoundChangeCertificate should not be created by the RoundChangeManager - rather an intermediate type is to be inserted. --- .../consensus/ibft/IbftBlockInterface.java | 32 ++++++++++ .../statemachine/IbftBlockHeightManager.java | 9 ++- .../ibft/statemachine/IbftRound.java | 50 ++++----------- .../statemachine/RoundChangeArtefacts.java | 62 ++++++++++++++++++ .../ibft/statemachine/RoundChangeManager.java | 14 ++-- .../IbftBlockHeightManagerTest.java | 5 +- .../ibft/statemachine/IbftRoundTest.java | 64 +++++++++---------- 7 files changed, 150 insertions(+), 86 deletions(-) create mode 100644 consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java index e8e8807e3a..53cb4460a9 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java @@ -16,7 +16,10 @@ import tech.pegasys.pantheon.consensus.common.ValidatorVote; import tech.pegasys.pantheon.consensus.common.VoteType; import tech.pegasys.pantheon.ethereum.core.Address; +import tech.pegasys.pantheon.ethereum.core.Block; +import tech.pegasys.pantheon.ethereum.core.BlockHashFunction; import tech.pegasys.pantheon.ethereum.core.BlockHeader; +import tech.pegasys.pantheon.ethereum.core.BlockHeaderBuilder; import java.util.Collection; import java.util.Optional; @@ -49,4 +52,33 @@ public Collection
validatorsInBlock(final BlockHeader header) { final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData()); return ibftExtraData.getValidators(); } + + public int roundOfBlock(final BlockHeader header) { + final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData()); + return ibftExtraData.getRound(); + } + + public static Block replaceRoundInBlock( + final Block block, final int round, final BlockHashFunction blockHashFunction) { + final IbftExtraData prevExtraData = IbftExtraData.decode(block.getHeader().getExtraData()); + final IbftExtraData substituteExtraData = + new IbftExtraData( + prevExtraData.getVanityData(), + prevExtraData.getSeals(), + prevExtraData.getVote(), + round, + prevExtraData.getValidators()); + + final BlockHeaderBuilder headerBuilder = BlockHeaderBuilder.fromHeader(block.getHeader()); + headerBuilder + .extraData(substituteExtraData.encode()) + .blockHashFunction( + blockHeader -> + IbftBlockHashing.calculateDataHashForCommittedSeal( + blockHeader, substituteExtraData)); + + final BlockHeader newHeader = headerBuilder.buildBlockHeader(); + + return new Block(newHeader, block.getBody()); + } } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java index 280e8c5371..e888f3a469 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java @@ -30,12 +30,12 @@ import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.payload.Payload; import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate; -import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate; import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidatorFactory; import tech.pegasys.pantheon.consensus.ibft.validation.NewRoundMessageValidator; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import java.time.Clock; +import java.util.Collection; import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -202,15 +202,18 @@ public void handleRoundChangePayload(final RoundChange message) { return; } - final Optional result = + final Optional> result = roundChangeManager.appendRoundChangeMessage(message); if (result.isPresent()) { if (messageAge == FUTURE_ROUND) { startNewRound(targetRound.getRoundNumber()); } + final RoundChangeArtefacts roundChangeArtefacts = RoundChangeArtefacts.create(result.get()); + if (finalState.isLocalNodeProposerForRound(targetRound)) { - currentRound.startRoundWith(result.get(), TimeUnit.MILLISECONDS.toSeconds(clock.millis())); + currentRound.startRoundWith( + roundChangeArtefacts, TimeUnit.MILLISECONDS.toSeconds(clock.millis())); } } } 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 c6a288dbe3..6d1eeac078 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,10 +12,9 @@ */ 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.IbftBlockInterface; import tech.pegasys.pantheon.consensus.ibft.IbftContext; import tech.pegasys.pantheon.consensus.ibft.IbftExtraData; import tech.pegasys.pantheon.consensus.ibft.IbftHelpers; @@ -27,7 +26,6 @@ import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter; import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate; -import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; @@ -35,7 +33,6 @@ 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; @@ -97,12 +94,11 @@ public void createAndSendProposalMessage(final long headerTimeStampSeconds) { } public void startRoundWith( - final RoundChangeCertificate roundChangeCertificate, final long headerTimestamp) { - final Optional latestCertificate = - findLatestPreparedCertificate(roundChangeCertificate.getRoundChangePayloads()); + final RoundChangeArtefacts roundChangeArtefacts, final long headerTimestamp) { + final Optional bestBlockFromRoundChange = roundChangeArtefacts.getBlock(); Proposal proposal; - if (!latestCertificate.isPresent()) { + if (!bestBlockFromRoundChange.isPresent()) { LOG.trace("Multicasting NewRound with new block. round={}", roundState.getRoundIdentifier()); final Block block = blockCreator.createBlock(headerTimestamp); proposal = messageFactory.createSignedProposalPayload(getRoundIdentifier(), block); @@ -110,40 +106,22 @@ public void startRoundWith( LOG.trace( "Multicasting NewRound from PreparedCertificate. round={}", roundState.getRoundIdentifier()); - proposal = createProposalFromPreparedCertificate(latestCertificate.get()); + proposal = createProposalAroundBlock(bestBlockFromRoundChange.get()); } transmitter.multicastNewRound( - getRoundIdentifier(), roundChangeCertificate, proposal.getSignedPayload()); + getRoundIdentifier(), + roundChangeArtefacts.getRoundChangeCertificate(), + proposal.getSignedPayload()); updateStateWithProposedBlock(proposal); } - private Proposal 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(), + private Proposal createProposalAroundBlock(final Block block) { + final Block blockToPublish = + IbftBlockInterface.replaceRoundInBlock( + block, 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()); - LOG.debug( - "Created proposal from prepared certificate blockHeader={} extraData={}", - block.getHeader(), - extraDataToPublish); - return messageFactory.createSignedProposalPayload(getRoundIdentifier(), newBlock); + IbftBlockHashing::calculateDataHashForCommittedSeal); + return messageFactory.createSignedProposalPayload(getRoundIdentifier(), blockToPublish); } public void handleProposalMessage(final Proposal msg) { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java new file mode 100644 index 0000000000..30ea3d7061 --- /dev/null +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java @@ -0,0 +1,62 @@ +/* + * Copyright 2019 ConsenSys AG. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on + * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the + * specific language governing permissions and limitations under the License. + */ +package tech.pegasys.pantheon.consensus.ibft.statemachine; + +import tech.pegasys.pantheon.consensus.ibft.IbftHelpers; +import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange; +import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate; +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.ethereum.core.Block; + +import java.util.Collection; +import java.util.Optional; +import java.util.stream.Collectors; + +public class RoundChangeArtefacts { + + private final Optional block; + private final Collection> roundChangePayloads; + + public RoundChangeArtefacts( + final Optional block, + final Collection> roundChangePayloads) { + this.block = block; + this.roundChangePayloads = roundChangePayloads; + } + + public Optional getBlock() { + return block; + } + + public RoundChangeCertificate getRoundChangeCertificate() { + return new RoundChangeCertificate(roundChangePayloads); + } + + public static RoundChangeArtefacts create(final Collection roundChanges) { + + final Collection> payloads = + roundChanges + .stream() + .map(roundChange -> roundChange.getSignedPayload()) + .collect(Collectors.toList()); + + final Optional latestdPreparedCertificate = + IbftHelpers.findLatestPreparedCertificate(payloads); + + return new RoundChangeArtefacts( + latestdPreparedCertificate.map(cert -> cert.getProposalPayload().getPayload().getBlock()), + payloads); + } +} diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java index 2d1d3d8a41..a260a7bb47 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java @@ -14,13 +14,12 @@ 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.validation.RoundChangeMessageValidator; import tech.pegasys.pantheon.ethereum.core.Address; +import java.util.Collection; 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; @@ -61,15 +60,10 @@ public boolean roundChangeReady() { return receivedMessages.size() >= quorum && !actioned; } - public RoundChangeCertificate createRoundChangeCertificate() { + public Collection createRoundChangeCertificate() { if (roundChangeReady()) { actioned = true; - return new RoundChangeCertificate( - receivedMessages - .values() - .stream() - .map(RoundChange::getSignedPayload) - .collect(Collectors.toList())); + return receivedMessages.values(); } else { throw new IllegalStateException("Unable to create RoundChangeCertificate at this time."); } @@ -97,7 +91,7 @@ public RoundChangeManager( * @return Empty if the round change threshold hasn't been hit, otherwise a round change * certificate */ - public Optional appendRoundChangeMessage(final RoundChange msg) { + public Optional> appendRoundChangeMessage(final RoundChange msg) { if (!isMessageValid(msg)) { LOG.info("RoundChange message was invalid."); diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java index c22388c455..3ef7dde59a 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java @@ -219,8 +219,7 @@ public void onRoundChangeReceptionRoundChangeManagerIsInvokedAndNewRoundStarted( final RoundChange roundChange = messageFactory.createSignedRoundChangePayload(futureRoundIdentifier, Optional.empty()); when(roundChangeManager.appendRoundChangeMessage(any())) - .thenReturn( - Optional.of(new RoundChangeCertificate(singletonList(roundChange.getSignedPayload())))); + .thenReturn(Optional.of(singletonList(roundChange))); when(finalState.isLocalNodeProposerForRound(any())).thenReturn(false); final IbftBlockHeightManager manager = @@ -267,7 +266,7 @@ public void whenSufficientRoundChangesAreReceivedANewRoundMessageIsTransmitted() new RoundChangeCertificate(singletonList(roundChange.getSignedPayload())); when(roundChangeManager.appendRoundChangeMessage(any())) - .thenReturn(Optional.of(roundChangCert)); + .thenReturn(Optional.of(singletonList(roundChange))); when(finalState.isLocalNodeProposerForRound(any())).thenReturn(true); final IbftBlockHeightManager manager = 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 5b17ef8069..5448cdaf61 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 @@ -12,6 +12,8 @@ */ package tech.pegasys.pantheon.consensus.ibft.statemachine; +import static java.util.Collections.emptyList; +import static java.util.Optional.empty; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -94,26 +96,20 @@ public void setup() { new ProtocolContext<>( blockChain, worldStateArchive, - new IbftContext(new VoteTally(Collections.emptyList()), new VoteProposer())); + new IbftContext(new VoteTally(emptyList()), new VoteProposer())); when(messageValidator.addSignedProposalPayload(any())).thenReturn(true); when(messageValidator.validatePrepareMessage(any())).thenReturn(true); when(messageValidator.validateCommmitMessage(any())).thenReturn(true); proposedExtraData = - new IbftExtraData( - BytesValue.wrap(new byte[32]), - Collections.emptyList(), - Optional.empty(), - 0, - Collections.emptyList()); + new IbftExtraData(BytesValue.wrap(new byte[32]), emptyList(), empty(), 0, emptyList()); final BlockHeaderTestFixture headerTestFixture = new BlockHeaderTestFixture(); headerTestFixture.extraData(proposedExtraData.encode()); headerTestFixture.number(1); final BlockHeader header = headerTestFixture.buildHeader(); - proposedBlock = - new Block(header, new BlockBody(Collections.emptyList(), Collections.emptyList())); + proposedBlock = new Block(header, new BlockBody(emptyList(), emptyList())); when(blockCreator.createBlock(anyLong())).thenReturn(proposedBlock); @@ -262,7 +258,7 @@ public void localNodeProposesToNetworkOfTwoValidatorsImportsOnReceptionOfCommitF } @Test - public void aNewRoundMessageWithAnewBlockIsSentUponReceptionOfARoundChangeWithNoCertificate() { + public void aNewRoundMessageWithANewBlockIsSentUponReceptionOfARoundChangeWithNoCertificate() { final RoundState roundState = new RoundState(roundIdentifier, 2, messageValidator); final IbftRound round = new IbftRound( @@ -275,10 +271,9 @@ public void aNewRoundMessageWithAnewBlockIsSentUponReceptionOfARoundChangeWithNo messageFactory, transmitter); - final RoundChangeCertificate roundChangeCertificate = - new RoundChangeCertificate(Collections.emptyList()); + final RoundChangeCertificate roundChangeCertificate = new RoundChangeCertificate(emptyList()); - round.startRoundWith(roundChangeCertificate, 15); + round.startRoundWith(new RoundChangeArtefacts(empty(), emptyList()), 15); verify(transmitter, times(1)) .multicastNewRound(eq(roundIdentifier), eq(roundChangeCertificate), any()); } @@ -298,25 +293,26 @@ public void aNewRoundMessageWithTheSameBlockIsSentUponReceptionOfARoundChangeWit messageFactory, transmitter); - final RoundChangeCertificate roundChangeCertificate = - new RoundChangeCertificate( + final RoundChangeArtefacts roundChangeArtefacts = + RoundChangeArtefacts.create( Collections.singletonList( - messageFactory - .createSignedRoundChangePayload( - roundIdentifier, - Optional.of( - new PreparedCertificate( - messageFactory - .createSignedProposalPayload(priorRoundChange, proposedBlock) - .getSignedPayload(), - Collections.emptyList()))) - .getSignedPayload())); + messageFactory.createSignedRoundChangePayload( + roundIdentifier, + Optional.of( + new PreparedCertificate( + messageFactory + .createSignedProposalPayload(priorRoundChange, proposedBlock) + .getSignedPayload(), + emptyList()))))); + // NOTE: IbftRound assumes the prepare's are valid - round.startRoundWith(roundChangeCertificate, 15); + round.startRoundWith(roundChangeArtefacts, 15); verify(transmitter, times(1)) .multicastNewRound( - eq(roundIdentifier), eq(roundChangeCertificate), payloadArgCaptor.capture()); + eq(roundIdentifier), + eq(roundChangeArtefacts.getRoundChangeCertificate()), + payloadArgCaptor.capture()); final IbftExtraData proposedExtraData = IbftExtraData.decode( @@ -344,17 +340,17 @@ public void creatingNewBlockFromEmptyPreparedCertificateUpdatesInternalState() { messageFactory, transmitter); - final RoundChangeCertificate roundChangeCertificate = - new RoundChangeCertificate( + final RoundChangeArtefacts roundChangeArtefacts = + RoundChangeArtefacts.create( Collections.singletonList( - messageFactory - .createSignedRoundChangePayload(roundIdentifier, Optional.empty()) - .getSignedPayload())); + messageFactory.createSignedRoundChangePayload(roundIdentifier, empty()))); - round.startRoundWith(roundChangeCertificate, 15); + round.startRoundWith(roundChangeArtefacts, 15); verify(transmitter, times(1)) .multicastNewRound( - eq(roundIdentifier), eq(roundChangeCertificate), payloadArgCaptor.capture()); + eq(roundIdentifier), + eq(roundChangeArtefacts.getRoundChangeCertificate()), + payloadArgCaptor.capture()); // Inject a single Prepare message, and confirm the roundState has gone to Prepared (which // indicates the block has entered the roundState (note: all msgs are deemed valid due to mocks) From b440eb9e23a0849e413ad3983df5d4cf35422f24 Mon Sep 17 00:00:00 2001 From: tmohay Date: Mon, 4 Feb 2019 16:40:18 +1100 Subject: [PATCH 2/3] rebased and repaired following review --- .../pantheon/consensus/ibft/IbftBlockInterface.java | 12 +----------- .../ibft/statemachine/IbftBlockHeightManager.java | 3 +-- .../consensus/ibft/statemachine/IbftRound.java | 1 - .../consensus/ibft/statemachine/IbftRoundTest.java | 13 ++++++------- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java index 53cb4460a9..2bf0f296db 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java @@ -53,11 +53,6 @@ public Collection
validatorsInBlock(final BlockHeader header) { return ibftExtraData.getValidators(); } - public int roundOfBlock(final BlockHeader header) { - final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData()); - return ibftExtraData.getRound(); - } - public static Block replaceRoundInBlock( final Block block, final int round, final BlockHashFunction blockHashFunction) { final IbftExtraData prevExtraData = IbftExtraData.decode(block.getHeader().getExtraData()); @@ -70,12 +65,7 @@ public static Block replaceRoundInBlock( prevExtraData.getValidators()); final BlockHeaderBuilder headerBuilder = BlockHeaderBuilder.fromHeader(block.getHeader()); - headerBuilder - .extraData(substituteExtraData.encode()) - .blockHashFunction( - blockHeader -> - IbftBlockHashing.calculateDataHashForCommittedSeal( - blockHeader, substituteExtraData)); + headerBuilder.extraData(substituteExtraData.encode()).blockHashFunction(blockHashFunction); final BlockHeader newHeader = headerBuilder.buildBlockHeader(); diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java index e49950fb42..4d531c6fb6 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java @@ -29,13 +29,12 @@ import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter; import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; import tech.pegasys.pantheon.consensus.ibft.payload.Payload; -import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate; -import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate; import tech.pegasys.pantheon.consensus.ibft.validation.MessageValidatorFactory; import tech.pegasys.pantheon.consensus.ibft.validation.NewRoundMessageValidator; import tech.pegasys.pantheon.ethereum.core.BlockHeader; import java.time.Clock; +import java.util.Collection; import java.util.Map; import java.util.Optional; import java.util.concurrent.TimeUnit; 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 b5ab86acc4..d16accb5b6 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 @@ -25,7 +25,6 @@ import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Proposal; import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter; import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; -import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate; import tech.pegasys.pantheon.crypto.SECP256K1; import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair; import tech.pegasys.pantheon.crypto.SECP256K1.Signature; 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 8e21d981a7..01a2a0a62a 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 @@ -12,6 +12,8 @@ */ package tech.pegasys.pantheon.consensus.ibft.statemachine; +import static java.util.Collections.emptyList; +import static java.util.Optional.empty; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -30,7 +32,6 @@ import tech.pegasys.pantheon.consensus.ibft.blockcreation.IbftBlockCreator; import tech.pegasys.pantheon.consensus.ibft.network.IbftMessageTransmitter; import tech.pegasys.pantheon.consensus.ibft.payload.MessageFactory; -import tech.pegasys.pantheon.consensus.ibft.payload.PreparedCertificate; import tech.pegasys.pantheon.consensus.ibft.payload.ProposalPayload; import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate; import tech.pegasys.pantheon.consensus.ibft.payload.SignedData; @@ -107,8 +108,7 @@ public void setup() { headerTestFixture.number(1); final BlockHeader header = headerTestFixture.buildHeader(); - proposedBlock = - new Block(header, new BlockBody(Collections.emptyList(), Collections.emptyList())); + proposedBlock = new Block(header, new BlockBody(emptyList(), emptyList())); when(blockCreator.createBlock(anyLong())).thenReturn(proposedBlock); @@ -298,10 +298,9 @@ public void aNewRoundMessageWithTheSameBlockIsSentUponReceptionOfARoundChangeWit messageFactory.createSignedRoundChangePayload( roundIdentifier, Optional.of( - new PreparedCertificate( - messageFactory - .createSignedProposalPayload(priorRoundChange, proposedBlock) - .getSignedPayload(), + new TerminatedRoundArtefacts( + messageFactory.createSignedProposalPayload( + priorRoundChange, proposedBlock), emptyList()))))); // NOTE: IbftRound assumes the prepare's are valid From 03e55745add98e8ad1233869d8d7061de4d7ceaf Mon Sep 17 00:00:00 2001 From: tmohay Date: Mon, 4 Feb 2019 16:44:48 +1100 Subject: [PATCH 3/3] minor fix --- .../consensus/ibft/statemachine/RoundChangeArtefacts.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java index 30ea3d7061..ab3dad16d7 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java @@ -52,11 +52,11 @@ public static RoundChangeArtefacts create(final Collection roundCha .map(roundChange -> roundChange.getSignedPayload()) .collect(Collectors.toList()); - final Optional latestdPreparedCertificate = + final Optional latestPreparedCertificate = IbftHelpers.findLatestPreparedCertificate(payloads); return new RoundChangeArtefacts( - latestdPreparedCertificate.map(cert -> cert.getProposalPayload().getPayload().getBlock()), + latestPreparedCertificate.map(cert -> cert.getProposalPayload().getPayload().getBlock()), payloads); } }