Separate round change reception from RoundChangeCertificate (#754)

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.
Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>
pull/2/head
tmohay 6 years ago committed by GitHub
parent 46c888c792
commit fb019d8f18
  1. 22
      consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/IbftBlockInterface.java
  2. 9
      consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManager.java
  3. 51
      consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRound.java
  4. 62
      consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeArtefacts.java
  5. 14
      consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/RoundChangeManager.java
  6. 5
      consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftBlockHeightManagerTest.java
  7. 60
      consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftRoundTest.java

@ -16,7 +16,10 @@ import tech.pegasys.pantheon.consensus.common.BlockInterface;
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,23 @@ public class IbftBlockInterface implements BlockInterface {
final IbftExtraData ibftExtraData = IbftExtraData.decode(header.getExtraData());
return ibftExtraData.getValidators();
}
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(blockHashFunction);
final BlockHeader newHeader = headerBuilder.buildBlockHeader();
return new Block(newHeader, block.getBody());
}
}

@ -29,12 +29,12 @@ import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
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.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 class IbftBlockHeightManager implements BlockHeightManager {
return;
}
final Optional<RoundChangeCertificate> result =
final Optional<Collection<RoundChange>> 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()));
}
}
}

@ -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;
@ -26,8 +25,6 @@ import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare;
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.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 +32,6 @@ 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;
@ -97,12 +93,11 @@ public class IbftRound {
}
public void startRoundWith(
final RoundChangeCertificate roundChangeCertificate, final long headerTimestamp) {
final Optional<PreparedCertificate> latestCertificate =
findLatestPreparedCertificate(roundChangeCertificate.getRoundChangePayloads());
final RoundChangeArtefacts roundChangeArtefacts, final long headerTimestamp) {
final Optional<Block> 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 +105,22 @@ public class IbftRound {
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) {

@ -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> block;
private final Collection<SignedData<RoundChangePayload>> roundChangePayloads;
public RoundChangeArtefacts(
final Optional<Block> block,
final Collection<SignedData<RoundChangePayload>> roundChangePayloads) {
this.block = block;
this.roundChangePayloads = roundChangePayloads;
}
public Optional<Block> getBlock() {
return block;
}
public RoundChangeCertificate getRoundChangeCertificate() {
return new RoundChangeCertificate(roundChangePayloads);
}
public static RoundChangeArtefacts create(final Collection<RoundChange> roundChanges) {
final Collection<SignedData<RoundChangePayload>> payloads =
roundChanges
.stream()
.map(roundChange -> roundChange.getSignedPayload())
.collect(Collectors.toList());
final Optional<PreparedCertificate> latestPreparedCertificate =
IbftHelpers.findLatestPreparedCertificate(payloads);
return new RoundChangeArtefacts(
latestPreparedCertificate.map(cert -> cert.getProposalPayload().getPayload().getBlock()),
payloads);
}
}

@ -14,13 +14,12 @@ package tech.pegasys.pantheon.consensus.ibft.statemachine;
import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier;
import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange;
import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangeCertificate;
import tech.pegasys.pantheon.consensus.ibft.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 class RoundChangeManager {
return receivedMessages.size() >= quorum && !actioned;
}
public RoundChangeCertificate createRoundChangeCertificate() {
public Collection<RoundChange> 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 class RoundChangeManager {
* @return Empty if the round change threshold hasn't been hit, otherwise a round change
* certificate
*/
public Optional<RoundChangeCertificate> appendRoundChangeMessage(final RoundChange msg) {
public Optional<Collection<RoundChange>> appendRoundChangeMessage(final RoundChange msg) {
if (!isMessageValid(msg)) {
LOG.info("RoundChange message was invalid.");

@ -218,8 +218,7 @@ public class IbftBlockHeightManagerTest {
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 =
@ -266,7 +265,7 @@ public class IbftBlockHeightManagerTest {
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 =

@ -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;
@ -93,26 +95,20 @@ public class IbftRoundTest {
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.validateCommitMessage(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);
@ -274,10 +270,9 @@ public class IbftRoundTest {
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());
}
@ -297,24 +292,25 @@ public class IbftRoundTest {
messageFactory,
transmitter);
final RoundChangeCertificate roundChangeCertificate =
new RoundChangeCertificate(
final RoundChangeArtefacts roundChangeArtefacts =
RoundChangeArtefacts.create(
Collections.singletonList(
messageFactory
.createSignedRoundChangePayload(
roundIdentifier,
Optional.of(
new TerminatedRoundArtefacts(
messageFactory.createSignedProposalPayload(
priorRoundChange, proposedBlock),
Collections.emptyList())))
.getSignedPayload()));
messageFactory.createSignedRoundChangePayload(
roundIdentifier,
Optional.of(
new TerminatedRoundArtefacts(
messageFactory.createSignedProposalPayload(
priorRoundChange, proposedBlock),
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(
@ -342,17 +338,17 @@ public class IbftRoundTest {
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)

Loading…
Cancel
Save