From 9826173ed2daaa64badfece009f81bde17c8cb73 Mon Sep 17 00:00:00 2001 From: Trent Mohay <37158202+rain-on@users.noreply.github.com> Date: Tue, 12 Jan 2021 10:32:27 +1100 Subject: [PATCH] Qbft rework artifacts (#1781) Refactor how Prepared/RoundChangeArtifacts are used in QBFT business logic. Signed-off-by: Trent Mohay --- .../qbft/support/IntegrationTestHelpers.java | 20 ++-- .../qbft/support/RoundSpecificPeers.java | 9 +- .../consensus/qbft/support/ValidatorPeer.java | 22 ++--- .../qbft/messagewrappers/Proposal.java | 10 -- .../qbft/network/QbftMessageTransmitter.java | 2 +- .../qbft/payload/MessageFactory.java | 19 +--- .../qbft/payload/RoundChangeMetadata.java | 96 ------------------- .../PreparedCertificate.java | 20 ++-- .../statemachine/PreparedRoundArtifacts.java | 43 --------- .../statemachine/QbftBlockHeightManager.java | 15 ++- .../qbft/statemachine/QbftRound.java | 27 +++--- .../statemachine/RoundChangeArtifacts.java | 87 +++++++++++++++++ .../qbft/statemachine/RoundState.java | 8 +- .../PreparedRoundArtifactsTest.java | 85 ---------------- .../QbftBlockHeightManagerTest.java | 9 +- .../qbft/statemachine/QbftRoundTest.java | 20 ++-- .../qbft/statemachine/RoundStateTest.java | 18 ++-- 17 files changed, 173 insertions(+), 337 deletions(-) delete mode 100644 consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/RoundChangeMetadata.java rename consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/{payload => statemachine}/PreparedCertificate.java (70%) delete mode 100644 consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifacts.java create mode 100644 consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeArtifacts.java delete mode 100644 consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifactsTest.java diff --git a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/IntegrationTestHelpers.java b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/IntegrationTestHelpers.java index 7a453e410a13..94e17a49b9f0 100644 --- a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/IntegrationTestHelpers.java +++ b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/IntegrationTestHelpers.java @@ -18,17 +18,13 @@ import org.hyperledger.besu.consensus.common.bft.BftExtraData; import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier; import org.hyperledger.besu.consensus.common.bft.payload.SignedData; -import org.hyperledger.besu.consensus.qbft.messagewrappers.Prepare; import org.hyperledger.besu.consensus.qbft.payload.CommitPayload; import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; -import org.hyperledger.besu.consensus.qbft.statemachine.PreparedRoundArtifacts; +import org.hyperledger.besu.consensus.qbft.statemachine.PreparedCertificate; import org.hyperledger.besu.crypto.NodeKey; import org.hyperledger.besu.crypto.SECP256K1.Signature; import org.hyperledger.besu.ethereum.core.Block; -import java.util.Collections; -import java.util.stream.Collectors; - public class IntegrationTestHelpers { public static SignedData createSignedCommitPayload( @@ -45,17 +41,13 @@ public static SignedData createSignedCommitPayload( return messageFactory.createCommit(roundId, block.getHash(), commitSeal).getSignedPayload(); } - public static PreparedRoundArtifacts createValidPreparedRoundArtifacts( + public static PreparedCertificate createValidPreparedRoundArtifacts( final TestContext context, final ConsensusRoundIdentifier preparedRound, final Block block) { final RoundSpecificPeers peers = context.roundSpecificPeers(preparedRound); - return new PreparedRoundArtifacts( - peers - .getProposer() - .getMessageFactory() - .createProposal(preparedRound, block, Collections.emptyList(), Collections.emptyList()), - peers.createSignedPreparePayloadOfNonProposing(preparedRound, block.getHash()).stream() - .map(Prepare::new) - .collect(Collectors.toList())); + return new PreparedCertificate( + block, + peers.createSignedPreparePayloadOfNonProposing(preparedRound, block.getHash()), + preparedRound.getRoundNumber()); } } diff --git a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/RoundSpecificPeers.java b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/RoundSpecificPeers.java index 118010d07027..a61c6f44c7d8 100644 --- a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/RoundSpecificPeers.java +++ b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/RoundSpecificPeers.java @@ -30,7 +30,7 @@ import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; import org.hyperledger.besu.consensus.qbft.payload.PreparePayload; import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload; -import org.hyperledger.besu.consensus.qbft.statemachine.PreparedRoundArtifacts; +import org.hyperledger.besu.consensus.qbft.statemachine.PreparedCertificate; import org.hyperledger.besu.crypto.SECP256K1.Signature; import org.hyperledger.besu.ethereum.core.Hash; import org.hyperledger.besu.ethereum.p2p.rlpx.wire.MessageData; @@ -109,13 +109,12 @@ public List> createSignedRoundChangePayload( } public List> createSignedRoundChangePayload( - final ConsensusRoundIdentifier roundId, final PreparedRoundArtifacts preparedRoundArtifacts) { + final ConsensusRoundIdentifier roundId, final PreparedCertificate preparedCertificate) { return peers.stream() .map( p -> p.getMessageFactory() - .createRoundChange( - roundId, Optional.of(preparedRoundArtifacts.getPreparedCertificate())) + .createRoundChange(roundId, Optional.of(preparedCertificate)) .getSignedPayload()) .collect(Collectors.toList()); } @@ -132,7 +131,7 @@ public void forNonProposing(final Consumer assertion) { nonProposingPeers.forEach(assertion); } - public Collection> createSignedPreparePayloadOfNonProposing( + public List> createSignedPreparePayloadOfNonProposing( final ConsensusRoundIdentifier preparedRound, final Hash hash) { return nonProposingPeers.stream() .map(role -> role.getMessageFactory().createPrepare(preparedRound, hash).getSignedPayload()) diff --git a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/ValidatorPeer.java b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/ValidatorPeer.java index 40a0949df4e1..db04985d0122 100644 --- a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/ValidatorPeer.java +++ b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/ValidatorPeer.java @@ -18,6 +18,7 @@ import org.hyperledger.besu.consensus.common.bft.EventMultiplexer; import org.hyperledger.besu.consensus.common.bft.inttest.DefaultValidatorPeer; import org.hyperledger.besu.consensus.common.bft.inttest.NodeParams; +import org.hyperledger.besu.consensus.common.bft.payload.SignedData; import org.hyperledger.besu.consensus.qbft.messagedata.CommitMessageData; import org.hyperledger.besu.consensus.qbft.messagedata.PrepareMessageData; import org.hyperledger.besu.consensus.qbft.messagedata.ProposalMessageData; @@ -27,13 +28,15 @@ import org.hyperledger.besu.consensus.qbft.messagewrappers.Proposal; import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; -import org.hyperledger.besu.consensus.qbft.payload.RoundChangeMetadata; -import org.hyperledger.besu.consensus.qbft.statemachine.PreparedRoundArtifacts; +import org.hyperledger.besu.consensus.qbft.payload.PreparePayload; +import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload; +import org.hyperledger.besu.consensus.qbft.statemachine.PreparedCertificate; import org.hyperledger.besu.crypto.SECP256K1.Signature; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.Hash; import java.util.Collections; +import java.util.List; import java.util.Optional; // Each "inject" function returns the SignedPayload representation of the transmitted message. @@ -78,25 +81,20 @@ public Commit injectCommit( public Proposal injectProposalForFutureRound( final ConsensusRoundIdentifier rId, - final RoundChangeMetadata roundChangeMetadata, + final List> roundChanges, + final List> prepares, final Block blockToPropose) { final Proposal payload = - messageFactory.createProposal( - rId, - blockToPropose, - roundChangeMetadata.getRoundChangePayloads(), - roundChangeMetadata.getPrepares()); + messageFactory.createProposal(rId, blockToPropose, roundChanges, prepares); injectMessage(ProposalMessageData.create(payload)); return payload; } public RoundChange injectRoundChange( final ConsensusRoundIdentifier rId, - final Optional preparedRoundArtifacts) { - final RoundChange payload = - messageFactory.createRoundChange( - rId, preparedRoundArtifacts.map(PreparedRoundArtifacts::getPreparedCertificate)); + final Optional preparedRoundArtifacts) { + final RoundChange payload = messageFactory.createRoundChange(rId, preparedRoundArtifacts); injectMessage(RoundChangeMessageData.create(payload)); return payload; } diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/Proposal.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/Proposal.java index 1f45bc7cc6a6..3cd74ec56143 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/Proposal.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/messagewrappers/Proposal.java @@ -19,7 +19,6 @@ import org.hyperledger.besu.consensus.qbft.payload.PayloadDeserializers; import org.hyperledger.besu.consensus.qbft.payload.PreparePayload; import org.hyperledger.besu.consensus.qbft.payload.ProposalPayload; -import org.hyperledger.besu.consensus.qbft.payload.RoundChangeMetadata; import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput; @@ -27,7 +26,6 @@ import org.hyperledger.besu.ethereum.rlp.RLPInput; import java.util.List; -import java.util.Optional; import org.apache.tuweni.bytes.Bytes; @@ -57,14 +55,6 @@ public Block getBlock() { return getPayload().getProposedBlock(); } - public Optional getRoundChangeMetadata() { - if (!roundChanges.isEmpty() && !prepares.isEmpty()) { - return Optional.of(new RoundChangeMetadata(Optional.of(getBlock()), roundChanges, prepares)); - } else { - return Optional.empty(); - } - } - @Override public Bytes encode() { final BytesValueRLPOutput rlpOut = new BytesValueRLPOutput(); diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/network/QbftMessageTransmitter.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/network/QbftMessageTransmitter.java index e0c9c54830d0..1a0ec8c16e17 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/network/QbftMessageTransmitter.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/network/QbftMessageTransmitter.java @@ -27,8 +27,8 @@ import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; import org.hyperledger.besu.consensus.qbft.payload.PreparePayload; -import org.hyperledger.besu.consensus.qbft.payload.PreparedCertificate; import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload; +import org.hyperledger.besu.consensus.qbft.statemachine.PreparedCertificate; import org.hyperledger.besu.crypto.SECP256K1.Signature; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.Hash; diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/MessageFactory.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/MessageFactory.java index 6ed14c741e9a..a681711f29f0 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/MessageFactory.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/MessageFactory.java @@ -21,6 +21,7 @@ import org.hyperledger.besu.consensus.qbft.messagewrappers.Prepare; import org.hyperledger.besu.consensus.qbft.messagewrappers.Proposal; import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; +import org.hyperledger.besu.consensus.qbft.statemachine.PreparedCertificate; import org.hyperledger.besu.crypto.NodeKey; import org.hyperledger.besu.crypto.SECP256K1.Signature; import org.hyperledger.besu.ethereum.core.Block; @@ -53,9 +54,7 @@ public Proposal createProposal( } public Prepare createPrepare(final ConsensusRoundIdentifier roundIdentifier, final Hash digest) { - final PreparePayload payload = new PreparePayload(roundIdentifier, digest); - return new Prepare(createSignedMessage(payload)); } @@ -63,9 +62,7 @@ public Commit createCommit( final ConsensusRoundIdentifier roundIdentifier, final Hash digest, final Signature commitSeal) { - final CommitPayload payload = new CommitPayload(roundIdentifier, digest, commitSeal); - return new Commit(createSignedMessage(payload)); } @@ -75,24 +72,18 @@ public RoundChange createRoundChange( final RoundChangePayload payload; if (preparedRoundData.isPresent()) { - final int round = - preparedRoundData - .get() - .getPrepares() - .get(0) - .getPayload() - .getRoundIdentifier() - .getRoundNumber(); + final Block preparedBlock = preparedRoundData.get().getBlock(); payload = new RoundChangePayload( roundIdentifier, Optional.of( new PreparedRoundMetadata( - preparedRoundData.get().getPreparedBlock().getHash(), round))); + preparedBlock.getHash(), preparedRoundData.get().getRound()))); + return new RoundChange( createSignedMessage(payload), - Optional.of(preparedRoundData.get().getPreparedBlock()), + Optional.of(preparedBlock), preparedRoundData.get().getPrepares()); } else { diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/RoundChangeMetadata.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/RoundChangeMetadata.java deleted file mode 100644 index 061ce6a74aa1..000000000000 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/RoundChangeMetadata.java +++ /dev/null @@ -1,96 +0,0 @@ -/* - * Copyright 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. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.consensus.qbft.payload; - -import org.hyperledger.besu.consensus.common.bft.payload.SignedData; -import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; -import org.hyperledger.besu.ethereum.core.Block; - -import java.util.Collection; -import java.util.Comparator; -import java.util.List; -import java.util.Optional; -import java.util.stream.Collectors; - -import com.google.common.base.MoreObjects; -import com.google.common.collect.Lists; - -public class RoundChangeMetadata { - - private final Optional block; - private final List> roundChangePayloads; - private final List> prepares; - - public RoundChangeMetadata( - final Optional block, - final List> roundChangePayloads, - final List> prepares) { - this.block = block; - this.roundChangePayloads = roundChangePayloads; - this.prepares = prepares; - } - - public List> getPrepares() { - return prepares; - } - - public List> getRoundChangePayloads() { - return roundChangePayloads; - } - - public Optional getBlock() { - return block; - } - - public static RoundChangeMetadata create(final Collection roundChanges) { - - final Comparator preparedRoundComparator = - (o1, o2) -> { - if (o1.getPreparedRound().isEmpty()) { - return -1; - } - if (o2.getPreparedRound().isEmpty()) { - return 1; - } - return o1.getPreparedRound().get().compareTo(o2.getPreparedRound().get()); - }; - - final Optional roundChangeWithNewestPrepare = - roundChanges.stream().max(preparedRoundComparator); - - final Optional proposedBlock; - final List> prepares = Lists.newArrayList(); - if (roundChangeWithNewestPrepare.isPresent()) { - proposedBlock = roundChangeWithNewestPrepare.get().getProposedBlock(); - prepares.addAll(roundChangeWithNewestPrepare.get().getPrepares()); - } else { - proposedBlock = Optional.empty(); - } - - final List> payloads = - roundChanges.stream().map(RoundChange::getSignedPayload).collect(Collectors.toList()); - - return new RoundChangeMetadata(proposedBlock, payloads, prepares); - } - - @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("block", block) - .add("roundChangePayloads", roundChangePayloads) - .add("prepares", prepares) - .toString(); - } -} diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/PreparedCertificate.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedCertificate.java similarity index 70% rename from consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/PreparedCertificate.java rename to consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedCertificate.java index 8e3058f6d4af..a4702885ed5f 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/payload/PreparedCertificate.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedCertificate.java @@ -12,25 +12,33 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.consensus.qbft.payload; +package org.hyperledger.besu.consensus.qbft.statemachine; import org.hyperledger.besu.consensus.common.bft.payload.SignedData; +import org.hyperledger.besu.consensus.qbft.payload.PreparePayload; import org.hyperledger.besu.ethereum.core.Block; import java.util.List; public class PreparedCertificate { - private final Block preparedBlock; + + private final Block block; private final List> prepares; + private final int round; public PreparedCertificate( - final Block preparedBlock, final List> prepares) { - this.preparedBlock = preparedBlock; + final Block block, final List> prepares, final int round) { + this.block = block; this.prepares = prepares; + this.round = round; + } + + public int getRound() { + return round; } - public Block getPreparedBlock() { - return preparedBlock; + public Block getBlock() { + return block; } public List> getPrepares() { diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifacts.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifacts.java deleted file mode 100644 index e70a62e6d916..000000000000 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifacts.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 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. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.consensus.qbft.statemachine; - -import org.hyperledger.besu.consensus.qbft.messagewrappers.Prepare; -import org.hyperledger.besu.consensus.qbft.messagewrappers.Proposal; -import org.hyperledger.besu.consensus.qbft.payload.PreparedCertificate; -import org.hyperledger.besu.ethereum.core.Block; - -import java.util.Collection; -import java.util.stream.Collectors; - -public class PreparedRoundArtifacts { - - private final Proposal proposal; - private final Collection prepares; - - public PreparedRoundArtifacts(final Proposal proposal, final Collection prepares) { - this.proposal = proposal; - this.prepares = prepares; - } - - public Block getBlock() { - return proposal.getBlock(); - } - - public PreparedCertificate getPreparedCertificate() { - return new PreparedCertificate( - getBlock(), prepares.stream().map(Prepare::getSignedPayload).collect(Collectors.toList())); - } -} diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java index 22322bd0baef..53d99da5fbb7 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManager.java @@ -26,8 +26,6 @@ import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; import org.hyperledger.besu.consensus.qbft.network.QbftMessageTransmitter; import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; -import org.hyperledger.besu.consensus.qbft.payload.PreparedCertificate; -import org.hyperledger.besu.consensus.qbft.payload.RoundChangeMetadata; import org.hyperledger.besu.consensus.qbft.validation.FutureRoundProposalMessageValidator; import org.hyperledger.besu.consensus.qbft.validation.MessageValidatorFactory; import org.hyperledger.besu.ethereum.core.BlockHeader; @@ -68,7 +66,7 @@ public class QbftBlockHeightManager implements BaseQbftBlockHeightManager { private final Function roundStateCreator; private final BftFinalState finalState; - private Optional latestPreparedRoundArtifacts = Optional.empty(); + private Optional latestPreparedCertificate = Optional.empty(); private QbftRound currentRound; @@ -133,10 +131,10 @@ public void roundExpired(final RoundExpiry expire) { "Round has expired, creating PreparedCertificate and notifying peers. round={}", currentRound.getRoundIdentifier()); final Optional preparedCertificate = - currentRound.constructPreparedRoundCertificate(); + currentRound.constructPreparedCertificate(); if (preparedCertificate.isPresent()) { - latestPreparedRoundArtifacts = preparedCertificate; + latestPreparedCertificate = preparedCertificate; } startNewRound(currentRound.getRoundIdentifier().getRoundNumber() + 1); @@ -144,7 +142,7 @@ public void roundExpired(final RoundExpiry expire) { try { final RoundChange localRoundChange = messageFactory.createRoundChange( - currentRound.getRoundIdentifier(), latestPreparedRoundArtifacts); + currentRound.getRoundIdentifier(), latestPreparedCertificate); // Its possible the locally created RoundChange triggers the transmission of a NewRound // message - so it must be handled accordingly. @@ -153,8 +151,7 @@ public void roundExpired(final RoundExpiry expire) { LOG.warn("Failed to create signed RoundChange message.", e); } - transmitter.multicastRoundChange( - currentRound.getRoundIdentifier(), latestPreparedRoundArtifacts); + transmitter.multicastRoundChange(currentRound.getRoundIdentifier(), latestPreparedCertificate); } @Override @@ -229,7 +226,7 @@ public void handleRoundChangePayload(final RoundChange message) { startNewRound(targetRound.getRoundNumber()); } - final RoundChangeMetadata roundChangeMetadata = RoundChangeMetadata.create(result.get()); + final RoundChangeArtifacts roundChangeMetadata = RoundChangeArtifacts.create(result.get()); if (finalState.isLocalNodeProposerForRound(targetRound)) { currentRound.startRoundWith( diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java index 5004e9f58435..472f0af19e33 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRound.java @@ -31,8 +31,6 @@ import org.hyperledger.besu.consensus.qbft.network.QbftMessageTransmitter; import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; import org.hyperledger.besu.consensus.qbft.payload.PreparePayload; -import org.hyperledger.besu.consensus.qbft.payload.PreparedCertificate; -import org.hyperledger.besu.consensus.qbft.payload.RoundChangeMetadata; import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload; import org.hyperledger.besu.crypto.NodeKey; import org.hyperledger.besu.crypto.SECP256K1.Signature; @@ -102,10 +100,12 @@ public void createAndSendProposalMessage(final long headerTimeStampSeconds) { } public void startRoundWith( - final RoundChangeMetadata roundChangeMetadata, final long headerTimestamp) { - final Optional bestBlockFromRoundChange = roundChangeMetadata.getBlock(); + final RoundChangeArtifacts roundChangeArtifacts, final long headerTimestamp) { + final Optional bestPreparedCertificate = + roundChangeArtifacts.getBestPreparedPeer(); + Block blockToPublish; - if (bestBlockFromRoundChange.isEmpty()) { + if (bestPreparedCertificate.isEmpty()) { LOG.debug("Sending proposal with new block. round={}", roundState.getRoundIdentifier()); blockToPublish = blockCreator.createBlock(headerTimestamp); } else { @@ -113,15 +113,15 @@ public void startRoundWith( "Sending proposal from PreparedCertificate. round={}", roundState.getRoundIdentifier()); blockToPublish = BftBlockInterface.replaceRoundInBlock( - bestBlockFromRoundChange.get(), + bestPreparedCertificate.get().getBlock(), getRoundIdentifier().getRoundNumber(), BftBlockHeaderFunctions.forCommittedSeal()); } updateStateWithProposalAndTransmit( blockToPublish, - roundChangeMetadata.getRoundChangePayloads(), - roundChangeMetadata.getPrepares()); + roundChangeArtifacts.getRoundChanges(), + bestPreparedCertificate.map(PreparedCertificate::getPrepares).orElse(emptyList())); } private void updateStateWithProposalAndTransmit( @@ -137,13 +137,16 @@ private void updateStateWithProposalAndTransmit( } transmitter.multicastProposal( - proposal.getRoundIdentifier(), proposal.getBlock(), roundChanges, prepares); + proposal.getRoundIdentifier(), + proposal.getSignedPayload().getPayload().getProposedBlock(), + roundChanges, + prepares); updateStateWithProposedBlock(proposal); } public void handleProposalMessage(final Proposal msg) { LOG.debug("Received a proposal message. round={}", roundState.getRoundIdentifier()); - final Block block = msg.getBlock(); + final Block block = msg.getSignedPayload().getPayload().getProposedBlock(); if (updateStateWithProposedBlock(msg)) { LOG.debug("Sending prepare message. round={}", roundState.getRoundIdentifier()); @@ -169,8 +172,8 @@ public void handleCommitMessage(final Commit msg) { peerIsCommitted(msg); } - public Optional constructPreparedRoundCertificate() { - return roundState.constructPreparedRoundCertificate(); + public Optional constructPreparedCertificate() { + return roundState.constructPreparedCertificate(); } private boolean updateStateWithProposedBlock(final Proposal msg) { diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeArtifacts.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeArtifacts.java new file mode 100644 index 000000000000..cbd8f41d92ee --- /dev/null +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundChangeArtifacts.java @@ -0,0 +1,87 @@ +/* + * Copyright 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. + * + * SPDX-License-Identifier: Apache-2.0 + */ +package org.hyperledger.besu.consensus.qbft.statemachine; + +import org.hyperledger.besu.consensus.common.bft.payload.SignedData; +import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; +import org.hyperledger.besu.consensus.qbft.payload.RoundChangePayload; + +import java.util.Collection; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +public class RoundChangeArtifacts { + + private final List> roundChanges; + private final Optional bestPreparedPeer; + + public RoundChangeArtifacts( + final List> roundChanges, + final Optional bestPreparedPeer) { + this.roundChanges = roundChanges; + this.bestPreparedPeer = bestPreparedPeer; + } + + public Optional getBestPreparedPeer() { + return bestPreparedPeer; + } + + public List> getRoundChanges() { + return roundChanges; + } + + public static RoundChangeArtifacts create(final Collection roundChanges) { + + final Comparator preparedRoundComparator = + (o1, o2) -> { + if (o1.getPreparedRoundMetadata().isEmpty()) { + return -1; + } + if (o2.getPreparedRoundMetadata().isEmpty()) { + return 1; + } + + int o1Round = o1.getPreparedRoundMetadata().get().getPreparedRound(); + int o2Round = o2.getPreparedRoundMetadata().get().getPreparedRound(); + + return Integer.compare(o1Round, o2Round); + }; + + final Optional roundChangeWithNewestPrepare = + roundChanges.stream().max(preparedRoundComparator); + + final Optional prepCert; + if (roundChangeWithNewestPrepare.isPresent()) { + if (roundChangeWithNewestPrepare.get().getProposedBlock().isPresent()) { + prepCert = + Optional.of( + new PreparedCertificate( + roundChangeWithNewestPrepare.get().getProposedBlock().get(), + roundChangeWithNewestPrepare.get().getPrepares(), + roundChangeWithNewestPrepare.get().getPreparedRound().get())); + } else { + prepCert = Optional.empty(); + } + } else { + prepCert = Optional.empty(); + } + + return new RoundChangeArtifacts( + roundChanges.stream().map(RoundChange::getSignedPayload).collect(Collectors.toList()), + prepCert); + } +} diff --git a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundState.java b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundState.java index f4ecf0618789..c9f93cef5e9f 100644 --- a/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundState.java +++ b/consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundState.java @@ -19,7 +19,6 @@ import org.hyperledger.besu.consensus.qbft.messagewrappers.Commit; import org.hyperledger.besu.consensus.qbft.messagewrappers.Prepare; import org.hyperledger.besu.consensus.qbft.messagewrappers.Proposal; -import org.hyperledger.besu.consensus.qbft.payload.PreparedCertificate; import org.hyperledger.besu.consensus.qbft.validation.MessageValidator; import org.hyperledger.besu.crypto.SECP256K1.Signature; import org.hyperledger.besu.ethereum.core.Block; @@ -130,14 +129,13 @@ public Collection getCommitSeals() { .collect(Collectors.toList()); } - public Optional constructPreparedRoundCertificate() { + public Optional constructPreparedCertificate() { if (isPrepared()) { return Optional.of( new PreparedCertificate( proposalMessage.get().getSignedPayload().getPayload().getProposedBlock(), - prepareMessages.stream() - .map(Prepare::getSignedPayload) - .collect(Collectors.toList()))); + prepareMessages.stream().map(Prepare::getSignedPayload).collect(Collectors.toList()), + roundIdentifier.getRoundNumber())); } return Optional.empty(); } diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifactsTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifactsTest.java deleted file mode 100644 index b4952f50458f..000000000000 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/PreparedRoundArtifactsTest.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Copyright 2020 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. - * - * SPDX-License-Identifier: Apache-2.0 - */ -package org.hyperledger.besu.consensus.qbft.statemachine; - -import static java.util.Collections.emptyList; -import static org.assertj.core.api.Assertions.assertThat; - -import org.hyperledger.besu.consensus.common.bft.BftExtraData; -import org.hyperledger.besu.consensus.common.bft.ConsensusRoundIdentifier; -import org.hyperledger.besu.consensus.qbft.messagewrappers.Prepare; -import org.hyperledger.besu.consensus.qbft.messagewrappers.Proposal; -import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; -import org.hyperledger.besu.crypto.NodeKey; -import org.hyperledger.besu.crypto.NodeKeyUtils; -import org.hyperledger.besu.ethereum.core.Block; -import org.hyperledger.besu.ethereum.core.BlockBody; -import org.hyperledger.besu.ethereum.core.BlockHeader; -import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; - -import java.util.List; -import java.util.Optional; - -import org.apache.tuweni.bytes.Bytes; -import org.junit.Before; -import org.junit.Test; - -public class PreparedRoundArtifactsTest { - - private final NodeKey nodeKey = NodeKeyUtils.generate(); - private final ConsensusRoundIdentifier roundIdentifier = new ConsensusRoundIdentifier(1, 0); - private final MessageFactory messageFactory = new MessageFactory(nodeKey); - - private Block proposedBlock; - - @Before - public void setup() { - final BftExtraData proposedExtraData = - new BftExtraData(Bytes.wrap(new byte[32]), emptyList(), Optional.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(emptyList(), emptyList())); - } - - @Test - public void blockReportedIsThatTakenFromProposal() { - final Proposal proposal = - messageFactory.createProposal(roundIdentifier, proposedBlock, emptyList(), emptyList()); - - final PreparedRoundArtifacts preparedRoundArtifacts = - new PreparedRoundArtifacts(proposal, emptyList()); - - assertThat(preparedRoundArtifacts.getBlock()).isEqualTo(proposedBlock); - assertThat(preparedRoundArtifacts.getPreparedCertificate().getPrepares()).isEmpty(); - } - - @Test - public void prepareMessagesInConstructorAreInCertificate() { - final Proposal proposal = - messageFactory.createProposal(roundIdentifier, proposedBlock, emptyList(), emptyList()); - final Prepare prepare = - messageFactory.createPrepare(roundIdentifier, proposal.getBlock().getHash()); - - final PreparedRoundArtifacts preparedRoundArtifacts = - new PreparedRoundArtifacts(proposal, List.of(prepare)); - - assertThat(preparedRoundArtifacts.getBlock()).isEqualTo(proposedBlock); - assertThat(preparedRoundArtifacts.getPreparedCertificate().getPrepares()) - .containsOnly(prepare.getSignedPayload()); - } -} diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java index 3f9e02f4af8f..15c445cf2c54 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftBlockHeightManagerTest.java @@ -45,7 +45,6 @@ import org.hyperledger.besu.consensus.qbft.messagewrappers.RoundChange; import org.hyperledger.besu.consensus.qbft.network.QbftMessageTransmitter; import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; -import org.hyperledger.besu.consensus.qbft.payload.RoundChangeMetadata; import org.hyperledger.besu.consensus.qbft.validation.FutureRoundProposalMessageValidator; import org.hyperledger.besu.consensus.qbft.validation.MessageValidator; import org.hyperledger.besu.consensus.qbft.validation.MessageValidatorFactory; @@ -266,8 +265,8 @@ public void whenSufficientRoundChangesAreReceivedAProposalMessageIsTransmitted() final ConsensusRoundIdentifier futureRoundIdentifier = createFrom(roundIdentifier, 0, +2); final RoundChange roundChange = messageFactory.createRoundChange(futureRoundIdentifier, Optional.empty()); - final RoundChangeMetadata roundChangMetadata = - RoundChangeMetadata.create(singletonList(roundChange)); + final RoundChangeArtifacts roundChangArtifacts = + RoundChangeArtifacts.create(singletonList(roundChange)); when(roundChangeManager.appendRoundChangeMessage(any())) .thenReturn(Optional.of(singletonList(roundChange))); @@ -290,8 +289,8 @@ public void whenSufficientRoundChangesAreReceivedAProposalMessageIsTransmitted() .multicastProposal( eq(futureRoundIdentifier), any(), - eq(roundChangMetadata.getRoundChangePayloads()), - eq(roundChangMetadata.getPrepares())); + eq(roundChangArtifacts.getRoundChanges()), + eq(emptyList())); } @Test diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundTest.java index b0a61cefcc01..3f746a92156a 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/QbftRoundTest.java @@ -39,8 +39,6 @@ import org.hyperledger.besu.consensus.qbft.network.QbftMessageTransmitter; import org.hyperledger.besu.consensus.qbft.payload.MessageFactory; import org.hyperledger.besu.consensus.qbft.payload.PreparePayload; -import org.hyperledger.besu.consensus.qbft.payload.PreparedCertificate; -import org.hyperledger.besu.consensus.qbft.payload.RoundChangeMetadata; import org.hyperledger.besu.consensus.qbft.validation.MessageValidator; import org.hyperledger.besu.crypto.NodeKey; import org.hyperledger.besu.crypto.NodeKeyUtils; @@ -302,7 +300,7 @@ public void aProposalWithAnewBlockIsSentUponReceptionOfARoundChangeWithNoCertifi transmitter, roundTimer); - round.startRoundWith(new RoundChangeMetadata(empty(), emptyList(), emptyList()), 15); + round.startRoundWith(new RoundChangeArtifacts(emptyList(), Optional.empty()), 15); verify(transmitter, times(1)) .multicastProposal(eq(roundIdentifier), any(), eq(emptyList()), eq(emptyList())); } @@ -329,12 +327,12 @@ public void aProposalMessageWithTheSameBlockIsSentUponReceptionOfARoundChangeWit final RoundChange roundChange = messageFactory.createRoundChange( roundIdentifier, - Optional.of(new PreparedCertificate(proposedBlock, singletonList(preparedPayload)))); + Optional.of(new PreparedCertificate(proposedBlock, singletonList(preparedPayload), 2))); - final RoundChangeMetadata roundChangeMetadata = - RoundChangeMetadata.create(singletonList(roundChange)); + final RoundChangeArtifacts roundChangeArtifacts = + RoundChangeArtifacts.create(singletonList(roundChange)); - round.startRoundWith(roundChangeMetadata, 15); + round.startRoundWith(roundChangeArtifacts, 15); verify(transmitter, times(1)) .multicastProposal( eq(roundIdentifier), @@ -370,10 +368,10 @@ public void creatingNewBlockFromEmptyPreparedCertificateUpdatesInternalState() { final RoundChange roundChange = messageFactory.createRoundChange(roundIdentifier, Optional.empty()); - final RoundChangeMetadata roundChangeMetadata = - RoundChangeMetadata.create(List.of(roundChange)); + final RoundChangeArtifacts roundChangeArtifacts = + RoundChangeArtifacts.create(List.of(roundChange)); - round.startRoundWith(roundChangeMetadata, 15); + round.startRoundWith(roundChangeArtifacts, 15); verify(transmitter, times(1)) .multicastProposal( eq(roundIdentifier), @@ -484,7 +482,7 @@ public void exceptionDuringNodeKeySigningDoesNotEscape() { roundIdentifier, proposedBlock, Collections.emptyList(), Collections.emptyList())); // Verify that no prepare message was constructed by the QbftRound - assertThat(roundState.constructPreparedRoundCertificate().get().getPrepares()).isEmpty(); + assertThat(roundState.constructPreparedCertificate().get().getPrepares()).isEmpty(); verifyNoInteractions(transmitter); } diff --git a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundStateTest.java b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundStateTest.java index c14d636b5e2a..912c0f42f090 100644 --- a/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundStateTest.java +++ b/consensus/qbft/src/test/java/org/hyperledger/besu/consensus/qbft/statemachine/RoundStateTest.java @@ -75,7 +75,7 @@ public void defaultRoundIsNotPreparedOrCommittedAndHasNoPreparedCertificate() { assertThat(roundState.isPrepared()).isFalse(); assertThat(roundState.isCommitted()).isFalse(); - assertThat(roundState.constructPreparedRoundCertificate()).isEmpty(); + assertThat(roundState.constructPreparedCertificate()).isEmpty(); } @Test @@ -92,7 +92,7 @@ public void ifProposalMessageFailsValidationMethodReturnsFalse() { assertThat(roundState.setProposedBlock(proposal)).isFalse(); assertThat(roundState.isPrepared()).isFalse(); assertThat(roundState.isCommitted()).isFalse(); - assertThat(roundState.constructPreparedRoundCertificate()).isEmpty(); + assertThat(roundState.constructPreparedCertificate()).isEmpty(); } @Test @@ -108,8 +108,8 @@ public void singleValidatorIsPreparedWithJustProposal() { assertThat(roundState.setProposedBlock(proposal)).isTrue(); assertThat(roundState.isPrepared()).isTrue(); assertThat(roundState.isCommitted()).isFalse(); - assertThat(roundState.constructPreparedRoundCertificate()).isNotEmpty(); - assertThat(roundState.constructPreparedRoundCertificate().get().getPreparedBlock()) + assertThat(roundState.constructPreparedCertificate()).isNotEmpty(); + assertThat(roundState.constructPreparedCertificate().get().getBlock()) .isEqualTo(proposal.getBlock()); } @@ -141,7 +141,7 @@ public void singleValidatorRequiresCommitMessageToBeCommitted() { roundState.addCommitMessage(commit); assertThat(roundState.isPrepared()).isTrue(); assertThat(roundState.isCommitted()).isTrue(); - assertThat(roundState.constructPreparedRoundCertificate()).isNotEmpty(); + assertThat(roundState.constructPreparedCertificate()).isNotEmpty(); } @Test @@ -160,12 +160,12 @@ public void prepareMessagesCanBeReceivedPriorToProposal() { roundState.addPrepareMessage(firstPrepare); assertThat(roundState.isPrepared()).isFalse(); assertThat(roundState.isCommitted()).isFalse(); - assertThat(roundState.constructPreparedRoundCertificate()).isEmpty(); + assertThat(roundState.constructPreparedCertificate()).isEmpty(); roundState.addPrepareMessage(secondPrepare); assertThat(roundState.isPrepared()).isFalse(); assertThat(roundState.isCommitted()).isFalse(); - assertThat(roundState.constructPreparedRoundCertificate()).isEmpty(); + assertThat(roundState.constructPreparedCertificate()).isEmpty(); final Proposal proposal = validatorMessageFactories @@ -175,7 +175,7 @@ public void prepareMessagesCanBeReceivedPriorToProposal() { assertThat(roundState.setProposedBlock(proposal)).isTrue(); assertThat(roundState.isPrepared()).isTrue(); assertThat(roundState.isCommitted()).isFalse(); - assertThat(roundState.constructPreparedRoundCertificate()).isNotEmpty(); + assertThat(roundState.constructPreparedCertificate()).isNotEmpty(); } @Test @@ -207,7 +207,7 @@ public void invalidPriorPrepareMessagesAreDiscardedUponSubsequentProposal() { assertThat(roundState.setProposedBlock(proposal)).isTrue(); assertThat(roundState.isPrepared()).isFalse(); assertThat(roundState.isCommitted()).isFalse(); - assertThat(roundState.constructPreparedRoundCertificate()).isEmpty(); + assertThat(roundState.constructPreparedCertificate()).isEmpty(); } @Test