From 3bec71d4811589ba92c0e0d7feaa5d80dbeaaa0a Mon Sep 17 00:00:00 2001 From: tmohay Date: Fri, 8 Feb 2019 14:06:39 +1100 Subject: [PATCH 1/9] Shuffled log levels --- .../blockcreation/IbftMiningCoordinator.java | 2 +- .../statemachine/IbftBlockHeightManager.java | 24 ++++++++++--------- .../ibft/statemachine/IbftController.java | 9 +++---- .../ibft/statemachine/IbftRound.java | 16 +++++++------ .../validation/NewRoundPayloadValidator.java | 2 +- 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftMiningCoordinator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftMiningCoordinator.java index 3327469489..0196e020cd 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftMiningCoordinator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/blockcreation/IbftMiningCoordinator.java @@ -78,7 +78,7 @@ public void setExtraData(final BytesValue extraData) { @Override public void onBlockAdded(final BlockAddedEvent event, final Blockchain blockchain) { if (event.isNewCanonicalHead()) { - LOG.info("New canonical head detected"); + LOG.trace("New canonical head detected"); eventQueue.add(new NewChainHead(event.getBlock().getHeader())); } } 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 0bff35383f..763328d325 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 @@ -113,7 +113,7 @@ public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifie if (roundIdentifier.equals(currentRound.getRoundIdentifier())) { currentRound.createAndSendProposalMessage(clock.millis() / 1000); } else { - LOG.info( + LOG.trace( "Block timer expired for a round ({}) other than current ({})", roundIdentifier, currentRound.getRoundIdentifier()); @@ -123,14 +123,14 @@ public void handleBlockTimerExpiry(final ConsensusRoundIdentifier roundIdentifie @Override public void roundExpired(final RoundExpiry expire) { if (!expire.getView().equals(currentRound.getRoundIdentifier())) { - LOG.info( + LOG.trace( "Ignoring Round timer expired which does not match current round. round={}, timerRound={}", currentRound.getRoundIdentifier(), expire.getView()); return; } - LOG.info( + LOG.debug( "Round has expired, creating PreparedCertificate and notifying peers. round={}", currentRound.getRoundIdentifier()); final Optional preparedRoundArtifacts = @@ -155,21 +155,21 @@ public void roundExpired(final RoundExpiry expire) { @Override public void handleProposalPayload(final Proposal proposal) { - LOG.debug("Received a Proposal Payload."); + LOG.trace("Received a Proposal Payload."); actionOrBufferMessage( proposal, currentRound::handleProposalMessage, RoundState::setProposedBlock); } @Override public void handlePreparePayload(final Prepare prepare) { - LOG.debug("Received a Prepare Payload."); + LOG.trace("Received a Prepare Payload."); actionOrBufferMessage( prepare, currentRound::handlePrepareMessage, RoundState::addPrepareMessage); } @Override public void handleCommitPayload(final Commit commit) { - LOG.debug("Received a Commit Payload."); + LOG.trace("Received a Commit Payload."); actionOrBufferMessage(commit, currentRound::handleCommitMessage, RoundState::addCommitMessage); } @@ -193,18 +193,20 @@ private

> void actionOrBufferMessage @Override public void handleRoundChangePayload(final RoundChange message) { final ConsensusRoundIdentifier targetRound = message.getRoundIdentifier(); - LOG.info("Received a RoundChange Payload for {}", targetRound.toString()); + LOG.trace("Received a RoundChange Payload for {}", targetRound.toString()); final MessageAge messageAge = determineAgeOfPayload(message.getRoundIdentifier().getRoundNumber()); if (messageAge == PRIOR_ROUND) { - LOG.debug("Received RoundChange Payload for a prior round. targetRound={}", targetRound); + LOG.trace("Received RoundChange Payload for a prior round. targetRound={}", targetRound); return; } final Optional> result = roundChangeManager.appendRoundChangeMessage(message); if (result.isPresent()) { + LOG.debug("Received sufficient RoundChange messages to change round to targetRound={}", + targetRound); if (messageAge == FUTURE_ROUND) { startNewRound(targetRound.getRoundNumber()); } @@ -219,7 +221,7 @@ public void handleRoundChangePayload(final RoundChange message) { } private void startNewRound(final int roundNumber) { - LOG.info("Starting new round {}", roundNumber); + LOG.debug("Starting new round {}", roundNumber); if (futureRoundStateBuffer.containsKey(roundNumber)) { currentRound = roundFactory.createNewRoundWithState( @@ -240,10 +242,10 @@ public void handleNewRoundPayload(final NewRound newRound) { determineAgeOfPayload(newRound.getRoundIdentifier().getRoundNumber()); if (messageAge == PRIOR_ROUND) { - LOG.info("Received NewRound Payload for a prior round={}", newRound.getRoundIdentifier()); + LOG.trace("Received NewRound Payload for a prior round={}", newRound.getRoundIdentifier()); return; } - LOG.info("Received NewRound Payload for {}", newRound.getRoundIdentifier()); + LOG.trace("Received NewRound Payload for {}", newRound.getRoundIdentifier()); if (newRoundMessageValidator.validateNewRoundMessage(newRound)) { if (messageAge == FUTURE_ROUND) { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java index dc4a828541..0d42d91079 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java @@ -145,8 +145,9 @@ private

> void consumeMessage( public void handleNewBlockEvent(final NewChainHead newChainHead) { final BlockHeader newBlockHeader = newChainHead.getNewChainHeadHeader(); final BlockHeader currentMiningParent = currentHeightManager.getParentBlockHeader(); + LOG.info("Handling New Chain head event, chain length = {}", currentMiningParent.getNumber()); if (newBlockHeader.getNumber() < currentMiningParent.getNumber()) { - LOG.debug( + LOG.trace( "Discarding NewChainHead event, was for previous block height. chainHeight={} eventHeight={}", currentMiningParent.getNumber(), newBlockHeader.getNumber()); @@ -155,7 +156,7 @@ public void handleNewBlockEvent(final NewChainHead newChainHead) { if (newBlockHeader.getNumber() == currentMiningParent.getNumber()) { if (newBlockHeader.getHash().equals(currentMiningParent.getHash())) { - LOG.debug( + LOG.trace( "Discarding duplicate NewChainHead event. chainHeight={} newBlockHash={} parentBlockHash", newBlockHeader.getNumber(), newBlockHeader.getHash(), @@ -175,7 +176,7 @@ public void handleBlockTimerExpiry(final BlockTimerExpiry blockTimerExpiry) { if (isMsgForCurrentHeight(roundIndentifier)) { currentHeightManager.handleBlockTimerExpiry(roundIndentifier); } else { - LOG.debug( + LOG.trace( "Block timer event discarded as it is not for current block height chainHeight={} eventHeight={}", currentHeightManager.getChainHeight(), roundIndentifier.getSequenceNumber()); @@ -186,7 +187,7 @@ public void handleRoundExpiry(final RoundExpiry roundExpiry) { if (isMsgForCurrentHeight(roundExpiry.getView())) { currentHeightManager.roundExpired(roundExpiry); } else { - LOG.debug( + LOG.trace( "Round expiry event discarded as it is not for current block height chainHeight={} eventHeight={}", currentHeightManager.getChainHeight(), roundExpiry.getView().getSequenceNumber()); 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 5ec8fdab1e..5f677c7798 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 @@ -81,7 +81,7 @@ public ConsensusRoundIdentifier getRoundIdentifier() { public void createAndSendProposalMessage(final long headerTimeStampSeconds) { final Block block = blockCreator.createBlock(headerTimeStampSeconds); final IbftExtraData extraData = IbftExtraData.decode(block.getHeader().getExtraData()); - LOG.info( + LOG.debug( "Creating proposed block. round={} extraData={} blockHeader={}", roundState.getRoundIdentifier(), extraData, @@ -98,11 +98,11 @@ public void startRoundWith( Proposal proposal; if (!bestBlockFromRoundChange.isPresent()) { - LOG.trace("Multicasting NewRound with new block. round={}", roundState.getRoundIdentifier()); + LOG.debug("Multicasting NewRound with new block. round={}", roundState.getRoundIdentifier()); final Block block = blockCreator.createBlock(headerTimestamp); proposal = messageFactory.createProposal(getRoundIdentifier(), block); } else { - LOG.trace( + LOG.debug( "Multicasting NewRound from PreparedCertificate. round={}", roundState.getRoundIdentifier()); proposal = createProposalAroundBlock(bestBlockFromRoundChange.get()); @@ -124,7 +124,7 @@ private Proposal createProposalAroundBlock(final Block block) { } public void handleProposalMessage(final Proposal msg) { - LOG.info("Handling a Proposal message."); + LOG.debug("Handling a Proposal message."); if (getRoundIdentifier().getRoundNumber() != 0) { LOG.error("Illegally received a Proposal message when not in Round 0."); @@ -134,7 +134,7 @@ public void handleProposalMessage(final Proposal msg) { } public void handleProposalFromNewRound(final NewRound msg) { - LOG.info("Handling a New Round Proposal."); + LOG.debug("Handling a New Round Proposal."); if (getRoundIdentifier().getRoundNumber() == 0) { LOG.error("Illegally received a NewRound message when in Round 0."); @@ -147,7 +147,7 @@ private void actionReceivedProposal(final Proposal msg) { final Block block = msg.getBlock(); if (updateStateWithProposedBlock(msg)) { - LOG.info("Sending prepare message."); + LOG.debug("Sending prepare message."); transmitter.multicastPrepare(getRoundIdentifier(), block.getHash()); final Prepare localPrepareMessage = messageFactory.createPrepare(roundState.getRoundIdentifier(), block.getHash()); @@ -220,7 +220,9 @@ private void importBlockToChain() { final long blockNumber = blockToImport.getHeader().getNumber(); final IbftExtraData extraData = IbftExtraData.decode(blockToImport.getHeader().getExtraData()); - LOG.info("Importing block to chain. block={} extraData={}", blockNumber, extraData); + LOG.info("Importing block to chain. round={}, hash={}", getRoundIdentifier(), + blockToImport.getHash()); + LOG.debug("ExtraData = {}", extraData); final boolean result = blockImporter.importBlock(protocolContext, blockToImport, HeaderValidationMode.FULL); if (!result) { diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java index 5fe6b70c81..01b0c8cfda 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java @@ -144,7 +144,7 @@ private boolean validateProposalMessageMatchesLatestPrepareCertificate( findLatestPreparedCertificate(roundChangeMsgs); if (!latestPreparedCertificate.isPresent()) { - LOG.info( + LOG.trace( "No round change messages have a preparedCertificate, any valid block may be proposed."); return true; } From f395020894329ef6991f4ba8694d00bbc02bedf9 Mon Sep 17 00:00:00 2001 From: tmohay Date: Fri, 8 Feb 2019 14:57:35 +1100 Subject: [PATCH 2/9] fix spotless --- .../consensus/ibft/statemachine/IbftBlockHeightManager.java | 3 ++- .../pantheon/consensus/ibft/statemachine/IbftRound.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) 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 763328d325..496b84a1fe 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 @@ -205,7 +205,8 @@ public void handleRoundChangePayload(final RoundChange message) { final Optional> result = roundChangeManager.appendRoundChangeMessage(message); if (result.isPresent()) { - LOG.debug("Received sufficient RoundChange messages to change round to targetRound={}", + LOG.debug( + "Received sufficient RoundChange messages to change round to targetRound={}", targetRound); if (messageAge == FUTURE_ROUND) { startNewRound(targetRound.getRoundNumber()); 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 5f677c7798..2c45e61e45 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 @@ -220,7 +220,9 @@ private void importBlockToChain() { final long blockNumber = blockToImport.getHeader().getNumber(); final IbftExtraData extraData = IbftExtraData.decode(blockToImport.getHeader().getExtraData()); - LOG.info("Importing block to chain. round={}, hash={}", getRoundIdentifier(), + LOG.info( + "Importing block to chain. round={}, hash={}", + getRoundIdentifier(), blockToImport.getHash()); LOG.debug("ExtraData = {}", extraData); final boolean result = From cf6c55f22254dc84494e5ec65640c93956a6eb0e Mon Sep 17 00:00:00 2001 From: Jason Frame Date: Fri, 8 Feb 2019 14:42:33 +1000 Subject: [PATCH 3/9] Prevent duplicate ibft messages being processed by state machine (#811) --- .../pantheon/config/IbftConfigOptions.java | 5 ++ .../config/IbftConfigOptionsTest.java | 20 +++++ .../ibft/support/TestContextBuilder.java | 13 ++- .../consensus/ibft/MessageTracker.java | 38 +++++++++ .../ibft/UniqueMessageMulticaster.java | 23 +++--- .../ibft/statemachine/IbftController.java | 25 ++++-- .../consensus/ibft/MessageTrackerTest.java | 59 +++++++++++++ .../ibft/UniqueMessageMulticasterTest.java | 82 ++++--------------- .../ibft/statemachine/IbftControllerTest.java | 38 ++++++--- .../controller/IbftPantheonController.java | 3 +- 10 files changed, 204 insertions(+), 102 deletions(-) create mode 100644 consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/MessageTracker.java create mode 100644 consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/MessageTrackerTest.java diff --git a/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java b/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java index b8fcfab8b2..fbc4988df9 100644 --- a/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java +++ b/config/src/main/java/tech/pegasys/pantheon/config/IbftConfigOptions.java @@ -25,6 +25,7 @@ public class IbftConfigOptions { // protection for on a typical 20 node validator network with multiple rounds private static final int DEFAULT_GOSSIPED_HISTORY_LIMIT = 1000; private static final int DEFAULT_MESSAGE_QUEUE_LIMIT = 1000; + private static final int DEFAULT_DUPLICATE_MESSAGE_LIMIT = 100; private final JsonObject ibftConfigRoot; @@ -51,4 +52,8 @@ public int getGossipedHistoryLimit() { public int getMessageQueueLimit() { return ibftConfigRoot.getInteger("messagequeuelimit", DEFAULT_MESSAGE_QUEUE_LIMIT); } + + public int getDuplicateMessageLimit() { + return ibftConfigRoot.getInteger("duplicatemessagelimit", DEFAULT_DUPLICATE_MESSAGE_LIMIT); + } } diff --git a/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java b/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java index a6a2003f15..54a52ef9bc 100644 --- a/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java +++ b/config/src/test/java/tech/pegasys/pantheon/config/IbftConfigOptionsTest.java @@ -28,6 +28,7 @@ public class IbftConfigOptionsTest { private static final int EXPECTED_DEFAULT_REQUEST_TIMEOUT = 1; private static final int EXPECTED_DEFAULT_GOSSIPED_HISTORY_LIMIT = 1000; private static final int EXPECTED_DEFAULT_MESSAGE_QUEUE_LIMIT = 1000; + private static final int EXPECTED_DEFAULT_DUPLICATE_MESSAGE_LIMIT = 100; @Test public void shouldGetEpochLengthFromConfig() { @@ -118,6 +119,25 @@ public void shouldGetDefaultMessageQueueLimitFromDefaultConfig() { .isEqualTo(EXPECTED_DEFAULT_MESSAGE_QUEUE_LIMIT); } + @Test + public void shouldGetDuplicateMessageLimitFromConfig() { + final IbftConfigOptions config = fromConfigOptions(singletonMap("DuplicateMessageLimit", 50)); + assertThat(config.getDuplicateMessageLimit()).isEqualTo(50); + } + + @Test + public void shouldFallbackToDefaultDuplicateMessageLimit() { + final IbftConfigOptions config = fromConfigOptions(emptyMap()); + assertThat(config.getDuplicateMessageLimit()) + .isEqualTo(EXPECTED_DEFAULT_DUPLICATE_MESSAGE_LIMIT); + } + + @Test + public void shouldGetDefaultDuplicateMessageLimitFromDefaultConfig() { + assertThat(IbftConfigOptions.DEFAULT.getDuplicateMessageLimit()) + .isEqualTo(EXPECTED_DEFAULT_DUPLICATE_MESSAGE_LIMIT); + } + private IbftConfigOptions fromConfigOptions(final Map ibftConfigOptions) { return GenesisConfigFile.fromConfig( new JsonObject(singletonMap("config", singletonMap("ibft", ibftConfigOptions)))) diff --git a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java index 366e15de0b..e7083af57b 100644 --- a/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java +++ b/consensus/ibft/src/integration-test/java/tech/pegasys/pantheon/consensus/ibft/support/TestContextBuilder.java @@ -69,7 +69,6 @@ import java.time.Instant; import java.time.ZoneId; import java.util.Collections; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; @@ -112,11 +111,12 @@ public EventMultiplexer getEventMultiplexer() { public static final int EPOCH_LENGTH = 10_000; public static final int BLOCK_TIMER_SEC = 3; public static final int ROUND_TIMER_SEC = 12; - public static final int EVENT_QUEUE_SIZE = 1000; - public static final int SEEN_MESSAGE_SIZE = 100; + public static final int MESSAGE_QUEUE_LIMIT = 1000; + public static final int GOSSIPED_HISTORY_LIMIT = 100; + public static final int DUPLICATE_MESSAGE_LIMIT = 100; private Clock clock = Clock.fixed(Instant.MIN, ZoneId.of("UTC")); - private IbftEventQueue ibftEventQueue = new IbftEventQueue(EVENT_QUEUE_SIZE); + private IbftEventQueue ibftEventQueue = new IbftEventQueue(MESSAGE_QUEUE_LIMIT); private int validatorCount = 4; private int indexOfFirstLocallyProposedBlock = 0; // Meaning first block is from remote peer. private boolean useGossip = false; @@ -159,9 +159,8 @@ public TestContext build() { // Use a stubbed version of the multicaster, to prevent creating PeerConnections etc. final StubValidatorMulticaster multicaster = new StubValidatorMulticaster(); - final UniqueMessageMulticaster uniqueMulticaster = - new UniqueMessageMulticaster(multicaster, SEEN_MESSAGE_SIZE); + new UniqueMessageMulticaster(multicaster, GOSSIPED_HISTORY_LIMIT); final Gossiper gossiper = useGossip ? new IbftGossip(uniqueMulticaster) : mock(Gossiper.class); @@ -308,7 +307,7 @@ private static ControllerAndState createControllerAndFinalState( messageValidatorFactory), messageValidatorFactory), gossiper, - new HashMap<>()); + DUPLICATE_MESSAGE_LIMIT); final EventMultiplexer eventMultiplexer = new EventMultiplexer(ibftController); //////////////////////////// END IBFT PantheonController //////////////////////////// diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/MessageTracker.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/MessageTracker.java new file mode 100644 index 0000000000..5d5e067d9b --- /dev/null +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/MessageTracker.java @@ -0,0 +1,38 @@ +/* + * 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; + +import static java.util.Collections.newSetFromMap; + +import tech.pegasys.pantheon.ethereum.core.Hash; +import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; + +import java.util.Set; + +public class MessageTracker { + private final Set seenMessages; + + public MessageTracker(final int messageTrackingLimit) { + this.seenMessages = newSetFromMap(new SizeLimitedMap<>(messageTrackingLimit)); + } + + public void addSeenMessage(final MessageData message) { + final Hash uniqueID = Hash.hash(message.getData()); + seenMessages.add(uniqueID); + } + + public boolean hasSeenMessage(final MessageData message) { + final Hash uniqueID = Hash.hash(message.getData()); + return seenMessages.contains(uniqueID); + } +} diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticaster.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticaster.java index d5b5961bdb..382635c96c 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticaster.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticaster.java @@ -12,20 +12,18 @@ */ package tech.pegasys.pantheon.consensus.ibft; -import static java.util.Collections.newSetFromMap; - import tech.pegasys.pantheon.consensus.ibft.network.ValidatorMulticaster; import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.core.Hash; import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import java.util.Collection; import java.util.Collections; -import java.util.Set; + +import com.google.common.annotations.VisibleForTesting; public class UniqueMessageMulticaster implements ValidatorMulticaster { private final ValidatorMulticaster multicaster; - private final Set seenMessages; + private final MessageTracker gossipedMessageTracker; /** * Constructor that attaches gossip logic to a set of multicaster @@ -36,8 +34,14 @@ public class UniqueMessageMulticaster implements ValidatorMulticaster { public UniqueMessageMulticaster( final ValidatorMulticaster multicaster, final int gossipHistoryLimit) { this.multicaster = multicaster; - // Set that starts evicting members when it hits capacity - this.seenMessages = newSetFromMap(new SizeLimitedMap<>(gossipHistoryLimit)); + this.gossipedMessageTracker = new MessageTracker(gossipHistoryLimit); + } + + @VisibleForTesting + public UniqueMessageMulticaster( + final ValidatorMulticaster multicaster, final MessageTracker gossipedMessageTracker) { + this.multicaster = multicaster; + this.gossipedMessageTracker = gossipedMessageTracker; } @Override @@ -47,11 +51,10 @@ public void send(final MessageData message) { @Override public void send(final MessageData message, final Collection

blackList) { - final Hash uniqueID = Hash.hash(message.getData()); - if (seenMessages.contains(uniqueID)) { + if (gossipedMessageTracker.hasSeenMessage(message)) { return; } multicaster.send(message, blackList); - seenMessages.add(uniqueID); + gossipedMessageTracker.addSeenMessage(message); } } diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java index 0d42d91079..558686b670 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java @@ -16,7 +16,7 @@ import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.Gossiper; -import tech.pegasys.pantheon.consensus.ibft.IbftGossip; +import tech.pegasys.pantheon.consensus.ibft.MessageTracker; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; @@ -45,7 +45,6 @@ import org.apache.logging.log4j.Logger; public class IbftController { - private static final Logger LOG = LogManager.getLogger(); private final Blockchain blockchain; private final IbftFinalState ibftFinalState; @@ -53,13 +52,21 @@ public class IbftController { private final Map> futureMessages; private BlockHeightManager currentHeightManager; private final Gossiper gossiper; + private final MessageTracker duplicateMessageTracker; public IbftController( final Blockchain blockchain, final IbftFinalState ibftFinalState, final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory, - final IbftGossip gossiper) { - this(blockchain, ibftFinalState, ibftBlockHeightManagerFactory, gossiper, Maps.newHashMap()); + final Gossiper gossiper, + final int duplicateMessageLimit) { + this( + blockchain, + ibftFinalState, + ibftBlockHeightManagerFactory, + gossiper, + Maps.newHashMap(), + new MessageTracker(duplicateMessageLimit)); } @VisibleForTesting @@ -68,12 +75,14 @@ public IbftController( final IbftFinalState ibftFinalState, final IbftBlockHeightManagerFactory ibftBlockHeightManagerFactory, final Gossiper gossiper, - final Map> futureMessages) { + final Map> futureMessages, + final MessageTracker duplicateMessageTracker) { this.blockchain = blockchain; this.ibftFinalState = ibftFinalState; this.ibftBlockHeightManagerFactory = ibftBlockHeightManagerFactory; this.futureMessages = futureMessages; this.gossiper = gossiper; + this.duplicateMessageTracker = duplicateMessageTracker; } public void start() { @@ -81,7 +90,11 @@ public void start() { } public void handleMessageEvent(final IbftReceivedMessageEvent msg) { - handleMessage(msg.getMessage()); + final MessageData data = msg.getMessage().getData(); + if (!duplicateMessageTracker.hasSeenMessage(data)) { + duplicateMessageTracker.addSeenMessage(data); + handleMessage(msg.getMessage()); + } } private void handleMessage(final Message message) { diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/MessageTrackerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/MessageTrackerTest.java new file mode 100644 index 0000000000..1f028bc755 --- /dev/null +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/MessageTrackerTest.java @@ -0,0 +1,59 @@ +/* + * 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; + +import static org.assertj.core.api.Assertions.assertThat; + +import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; +import tech.pegasys.pantheon.util.bytes.BytesValue; + +import org.junit.Test; + +public class MessageTrackerTest { + private final MessageTracker messageTracker = new MessageTracker(5); + + @Test + public void duplicateMessagesAreConsideredSeen() { + final MessageData arbitraryMessage_1 = + createAnonymousMessageData(BytesValue.wrap(new byte[4]), 1); + + final MessageData arbitraryMessage_2 = + createAnonymousMessageData(BytesValue.wrap(new byte[4]), 1); + + assertThat(messageTracker.hasSeenMessage(arbitraryMessage_1)).isFalse(); + assertThat(messageTracker.hasSeenMessage(arbitraryMessage_2)).isFalse(); + + messageTracker.addSeenMessage(arbitraryMessage_1); + assertThat(messageTracker.hasSeenMessage(arbitraryMessage_2)).isTrue(); + } + + private MessageData createAnonymousMessageData(final BytesValue content, final int code) { + return new MessageData() { + + @Override + public int getSize() { + return content.size(); + } + + @Override + public int getCode() { + return code; + } + + @Override + public BytesValue getData() { + return content; + } + }; + } +} diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticasterTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticasterTest.java index 51931ef46b..d91606d560 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticasterTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/UniqueMessageMulticasterTest.java @@ -18,11 +18,11 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; import tech.pegasys.pantheon.consensus.ibft.network.ValidatorMulticaster; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.AddressHelpers; -import tech.pegasys.pantheon.ethereum.p2p.api.MessageData; import tech.pegasys.pantheon.ethereum.p2p.wire.RawMessage; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -36,95 +36,43 @@ @RunWith(MockitoJUnitRunner.class) public class UniqueMessageMulticasterTest { + private final MessageTracker messageTracker = mock(MessageTracker.class); private final ValidatorMulticaster multicaster = mock(ValidatorMulticaster.class); - private final UniqueMessageMulticaster messageTracker = - new UniqueMessageMulticaster(multicaster, 5); + private final UniqueMessageMulticaster uniqueMessageMulticaster = + new UniqueMessageMulticaster(multicaster, messageTracker); private final RawMessage messageSent = new RawMessage(5, BytesValue.wrap(new byte[5])); @Test public void previouslySentMessageIsNotSentAgain() { - - messageTracker.send(messageSent); + when(messageTracker.hasSeenMessage(messageSent)).thenReturn(false); + uniqueMessageMulticaster.send(messageSent); verify(multicaster, times(1)).send(messageSent, emptyList()); reset(multicaster); - messageTracker.send(messageSent); - messageTracker.send(messageSent, emptyList()); + when(messageTracker.hasSeenMessage(messageSent)).thenReturn(true); + uniqueMessageMulticaster.send(messageSent); + uniqueMessageMulticaster.send(messageSent, emptyList()); verifyZeroInteractions(multicaster); } @Test public void messagesSentWithABlackListAreNotRetransmitted() { - messageTracker.send(messageSent, emptyList()); + when(messageTracker.hasSeenMessage(messageSent)).thenReturn(false); + uniqueMessageMulticaster.send(messageSent, emptyList()); verify(multicaster, times(1)).send(messageSent, emptyList()); reset(multicaster); - messageTracker.send(messageSent, emptyList()); - messageTracker.send(messageSent); - verifyZeroInteractions(multicaster); - } - - @Test - public void oldMessagesAreEvictedWhenFullAndCanThenBeRetransmitted() { - final List messagesSent = Lists.newArrayList(); - - for (int i = 0; i < 6; i++) { - final RawMessage msg = new RawMessage(i, BytesValue.wrap(new byte[i])); - messagesSent.add(msg); - messageTracker.send(msg); - verify(multicaster, times(1)).send(msg, emptyList()); - } - reset(multicaster); - - messageTracker.send(messagesSent.get(5)); + when(messageTracker.hasSeenMessage(messageSent)).thenReturn(true); + uniqueMessageMulticaster.send(messageSent, emptyList()); + uniqueMessageMulticaster.send(messageSent); verifyZeroInteractions(multicaster); - - messageTracker.send(messagesSent.get(0)); - verify(multicaster, times(1)).send(messagesSent.get(0), emptyList()); } @Test public void passedInBlackListIsPassedToUnderlyingValidator() { final List
blackList = Lists.newArrayList(AddressHelpers.ofValue(0), AddressHelpers.ofValue(1)); - messageTracker.send(messageSent, blackList); + uniqueMessageMulticaster.send(messageSent, blackList); verify(multicaster, times(1)).send(messageSent, blackList); } - - @Test - public void anonymousMessageDataClassesContainingTheSameDataAreConsideredIdentical() { - - final MessageData arbitraryMessage_1 = - createAnonymousMessageData(BytesValue.wrap(new byte[4]), 1); - - final MessageData arbitraryMessage_2 = - createAnonymousMessageData(BytesValue.wrap(new byte[4]), 1); - - messageTracker.send(arbitraryMessage_1); - verify(multicaster, times(1)).send(arbitraryMessage_1, emptyList()); - reset(multicaster); - - messageTracker.send(arbitraryMessage_2); - verifyZeroInteractions(multicaster); - } - - private MessageData createAnonymousMessageData(final BytesValue content, final int code) { - return new MessageData() { - - @Override - public int getSize() { - return content.size(); - } - - @Override - public int getCode() { - return code; - } - - @Override - public BytesValue getData() { - return content; - } - }; - } } diff --git a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java index 276b70f787..2e6a5f1446 100644 --- a/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java +++ b/consensus/ibft/src/test/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftControllerTest.java @@ -24,6 +24,7 @@ import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; import tech.pegasys.pantheon.consensus.ibft.IbftGossip; +import tech.pegasys.pantheon.consensus.ibft.MessageTracker; import tech.pegasys.pantheon.consensus.ibft.ibftevent.BlockTimerExpiry; import tech.pegasys.pantheon.consensus.ibft.ibftevent.IbftReceivedMessageEvent; import tech.pegasys.pantheon.consensus.ibft.ibftevent.NewChainHead; @@ -39,11 +40,6 @@ import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Prepare; import tech.pegasys.pantheon.consensus.ibft.messagewrappers.Proposal; import tech.pegasys.pantheon.consensus.ibft.messagewrappers.RoundChange; -import tech.pegasys.pantheon.consensus.ibft.payload.CommitPayload; -import tech.pegasys.pantheon.consensus.ibft.payload.NewRoundPayload; -import tech.pegasys.pantheon.consensus.ibft.payload.PreparePayload; -import tech.pegasys.pantheon.consensus.ibft.payload.ProposalPayload; -import tech.pegasys.pantheon.consensus.ibft.payload.RoundChangePayload; import tech.pegasys.pantheon.ethereum.chain.Blockchain; import tech.pegasys.pantheon.ethereum.core.Address; import tech.pegasys.pantheon.ethereum.core.BlockHeader; @@ -75,28 +71,24 @@ public class IbftControllerTest { @Mock private Proposal proposal; private Message proposalMessage; @Mock private ProposalMessageData proposalMessageData; - @Mock private ProposalPayload proposalPayload; @Mock private Prepare prepare; private Message prepareMessage; @Mock private PrepareMessageData prepareMessageData; - @Mock private PreparePayload preparePayload; @Mock private Commit commit; private Message commitMessage; @Mock private CommitMessageData commitMessageData; - @Mock private CommitPayload commitPayload; @Mock private NewRound newRound; private Message newRoundMessage; @Mock private NewRoundMessageData newRoundMessageData; - @Mock private NewRoundPayload newRoundPayload; @Mock private RoundChange roundChange; private Message roundChangeMessage; @Mock private RoundChangeMessageData roundChangeMessageData; - @Mock private RoundChangePayload roundChangePayload; + @Mock private MessageTracker messageTracker; private final Map> futureMessages = new HashMap<>(); private final Address validator = Address.fromHexString("0x0"); private final Address unknownValidator = Address.fromHexString("0x2"); @@ -112,7 +104,12 @@ public void setup() { when(ibftFinalState.getValidators()).thenReturn(ImmutableList.of(validator)); ibftController = new IbftController( - blockChain, ibftFinalState, blockHeightManagerFactory, ibftGossip, futureMessages); + blockChain, + ibftFinalState, + blockHeightManagerFactory, + ibftGossip, + futureMessages, + messageTracker); when(chainHeadBlockHeader.getNumber()).thenReturn(1L); when(chainHeadBlockHeader.getHash()).thenReturn(Hash.ZERO); @@ -122,6 +119,7 @@ public void setup() { when(nextBlock.getNumber()).thenReturn(2L); when(ibftFinalState.isLocalNodeValidator()).thenReturn(true); + when(messageTracker.hasSeenMessage(any())).thenReturn(false); } @Test @@ -432,6 +430,24 @@ public void roundChangeForFutureHeightIsBuffered() { verifyHasFutureMessages(new IbftReceivedMessageEvent(roundChangeMessage), expectedFutureMsgs); } + @Test + public void duplicatedMessagesAreNotProcessed() { + when(messageTracker.hasSeenMessage(proposalMessageData)).thenReturn(true); + setupProposal(roundIdentifier, validator); + verifyNotHandledAndNoFutureMsgs(new IbftReceivedMessageEvent(proposalMessage)); + verify(messageTracker, never()).addSeenMessage(proposalMessageData); + } + + @Test + public void uniqueMessagesAreAddedAsSeen() { + when(messageTracker.hasSeenMessage(proposalMessageData)).thenReturn(false); + setupProposal(roundIdentifier, validator); + ibftController.start(); + ibftController.handleMessageEvent(new IbftReceivedMessageEvent(proposalMessage)); + + verify(messageTracker).addSeenMessage(proposalMessageData); + } + private void verifyNotHandledAndNoFutureMsgs(final IbftReceivedMessageEvent msg) { ibftController.start(); ibftController.handleMessageEvent(msg); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java index db388e7473..cd6e34099c 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/controller/IbftPantheonController.java @@ -251,7 +251,8 @@ public static PantheonController init( minedBlockObservers, messageValidatorFactory), messageValidatorFactory), - gossiper); + gossiper, + ibftConfig.getDuplicateMessageLimit()); ibftController.start(); final EventMultiplexer eventMultiplexer = new EventMultiplexer(ibftController); From 9f98d6f353299e9e6743d7f653419e98aa25e52d Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Fri, 8 Feb 2019 14:56:50 +1000 Subject: [PATCH 4/9] Parallel build stages (#816) --- Jenkinsfile | 136 +++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 98 insertions(+), 38 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index b36e2a9bfc..32f96376f4 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -18,49 +18,109 @@ if (env.BRANCH_NAME == "master") { ]) } +def docker_image = 'docker:18.06.0-ce-dind' +def build_image = 'pegasyseng/pantheon-build:0.0.5-jdk11' + try { - node { - checkout scm - docker.image('docker:18.06.0-ce-dind').withRun('--privileged') { d -> - docker.image('pegasyseng/pantheon-build:0.0.5-jdk11').inside("--link ${d.id}:docker") { - try { - stage('Compile') { - sh './gradlew --no-daemon --parallel clean compileJava' - } - stage('compile tests') { - sh './gradlew --no-daemon --parallel compileTestJava' - } - stage('assemble') { - sh './gradlew --no-daemon --parallel assemble' - } - stage('Build') { - sh './gradlew --no-daemon --parallel build' - } - stage('Reference tests') { - sh './gradlew --no-daemon --parallel referenceTest' - } - stage('Integration Tests') { - sh './gradlew --no-daemon --parallel integrationTest' - } - stage('Acceptance Tests') { - sh './gradlew --no-daemon --parallel acceptanceTest' - } - stage('Check Licenses') { - sh './gradlew --no-daemon --parallel checkLicenses' + parallel UnitTests: { + def stage_name = "Unit tests node: " + node { + checkout scm + docker.image(docker_image).withRun('--privileged') { d -> + docker.image(build_image).inside("--link ${d.id}:docker") { + try { + stage(stage_name + 'Prepare') { + sh './gradlew --no-daemon --parallel clean compileJava compileTestJava assemble' + } + stage(stage_name + 'Unit tests') { + sh './gradlew --no-daemon --parallel build' + } + } finally { + archiveArtifacts '**/build/reports/**' + archiveArtifacts '**/build/test-results/**' + archiveArtifacts 'build/reports/**' + archiveArtifacts 'build/distributions/**' + + junit '**/build/test-results/**/*.xml' } - stage('Check javadoc') { - sh './gradlew --no-daemon --parallel javadoc' + } + } + } + }, ReferenceTests: { + def stage_name = "Reference tests node: " + node { + checkout scm + docker.image(docker_image).withRun('--privileged') { d -> + docker.image(build_image).inside("--link ${d.id}:docker") { + try { + stage(stage_name + 'Prepare') { + sh './gradlew --no-daemon --parallel clean compileJava compileTestJava assemble' + } + stage(stage_name + 'Reference tests') { + sh './gradlew --no-daemon --parallel referenceTest' + } + } finally { + archiveArtifacts '**/build/reports/**' + archiveArtifacts '**/build/test-results/**' + archiveArtifacts 'build/reports/**' + archiveArtifacts 'build/distributions/**' + + junit '**/build/test-results/**/*.xml' } - stage('Jacoco root report') { - sh './gradlew --no-daemon jacocoRootReport' + } + } + } + }, IntegrationTests: { + def stage_name = "Integration tests node: " + node { + checkout scm + docker.image(docker_image).withRun('--privileged') { d -> + docker.image(build_image).inside("--link ${d.id}:docker") { + try { + stage(stage_name + 'Prepare') { + sh './gradlew --no-daemon --parallel clean compileJava compileTestJava assemble' + } + stage(stage_name + 'Integration Tests') { + sh './gradlew --no-daemon --parallel integrationTest' + } + stage(stage_name + 'Check Licenses') { + sh './gradlew --no-daemon --parallel checkLicenses' + } + stage(stage_name + 'Check javadoc') { + sh './gradlew --no-daemon --parallel javadoc' + } + } finally { + archiveArtifacts '**/build/reports/**' + archiveArtifacts '**/build/test-results/**' + archiveArtifacts 'build/reports/**' + archiveArtifacts 'build/distributions/**' + + junit '**/build/test-results/**/*.xml' } - } finally { - archiveArtifacts '**/build/reports/**' - archiveArtifacts '**/build/test-results/**' - archiveArtifacts 'build/reports/**' - archiveArtifacts 'build/distributions/**' + } + } + } + }, AcceptanceTests: { + def stage_name = "Acceptance tests node: " + node { + checkout scm + docker.image(docker_image).withRun('--privileged') { d -> + docker.image(build_image).inside("--link ${d.id}:docker") { + try { + stage(stage_name + 'Prepare') { + sh './gradlew --no-daemon --parallel clean compileJava compileTestJava assemble' + } + stage(stage_name + 'Acceptance Tests') { + sh './gradlew --no-daemon --parallel acceptanceTest' + } + } finally { + archiveArtifacts '**/build/reports/**' + archiveArtifacts '**/build/test-results/**' + archiveArtifacts 'build/reports/**' + archiveArtifacts 'build/distributions/**' - junit '**/build/test-results/**/*.xml' + junit '**/build/test-results/**/*.xml' + } } } } From 16ee776afa107572b81b79850385ec4d809a1760 Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Fri, 8 Feb 2019 15:37:23 +1000 Subject: [PATCH 5/9] abort previous builds (#817) --- Jenkinsfile | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/Jenkinsfile b/Jenkinsfile index 32f96376f4..4c6b957246 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1,5 +1,9 @@ #!/usr/bin/env groovy +import hudson.model.Result +import hudson.model.Run +import jenkins.model.CauseOfInterruption.UserInterruption + if (env.BRANCH_NAME == "master") { properties([ buildDiscarder( @@ -21,6 +25,26 @@ if (env.BRANCH_NAME == "master") { def docker_image = 'docker:18.06.0-ce-dind' def build_image = 'pegasyseng/pantheon-build:0.0.5-jdk11' +def abortPreviousBuilds() { + Run previousBuild = currentBuild.rawBuild.getPreviousBuildInProgress() + + while (previousBuild != null) { + if (previousBuild.isInProgress()) { + def executor = previousBuild.getExecutor() + if (executor != null) { + echo ">> Aborting older build #${previousBuild.number}" + executor.interrupt(Result.ABORTED, new UserInterruption( + "Aborted by newer build #${currentBuild.number}" + )) + } + } + + previousBuild = previousBuild.getPreviousBuildInProgress() + } +} + +abortPreviousBuilds() + try { parallel UnitTests: { def stage_name = "Unit tests node: " From c127591a2f39ed486c2328b6344d28a17707ef59 Mon Sep 17 00:00:00 2001 From: tmohay Date: Fri, 8 Feb 2019 21:23:08 +1100 Subject: [PATCH 6/9] rebase and spotless and stuffs --- .../ibft/statemachine/IbftController.java | 2 +- .../validation/NewRoundPayloadValidator.java | 46 +------------------ 2 files changed, 2 insertions(+), 46 deletions(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java index 558686b670..aa4aa21167 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java @@ -158,7 +158,7 @@ private

> void consumeMessage( public void handleNewBlockEvent(final NewChainHead newChainHead) { final BlockHeader newBlockHeader = newChainHead.getNewChainHeadHeader(); final BlockHeader currentMiningParent = currentHeightManager.getParentBlockHeader(); - LOG.info("Handling New Chain head event, chain length = {}", currentMiningParent.getNumber()); + LOG.debug("Handling New Chain head event, chain length = {}", currentMiningParent.getNumber()); if (newBlockHeader.getNumber() < currentMiningParent.getNumber()) { LOG.trace( "Discarding NewChainHead event, was for previous block height. chainHeight={} eventHeight={}", diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java index 01b0c8cfda..1f5ffac9c6 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/validation/NewRoundPayloadValidator.java @@ -12,24 +12,19 @@ */ package tech.pegasys.pantheon.consensus.ibft.validation; -import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.findLatestPreparedCertificate; import static tech.pegasys.pantheon.consensus.ibft.IbftHelpers.prepareMessageCountForQuorum; import tech.pegasys.pantheon.consensus.ibft.ConsensusRoundIdentifier; -import tech.pegasys.pantheon.consensus.ibft.IbftBlockHashing; import tech.pegasys.pantheon.consensus.ibft.blockcreation.ProposerSelector; import tech.pegasys.pantheon.consensus.ibft.payload.NewRoundPayload; -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.RoundChangePayload; import tech.pegasys.pantheon.consensus.ibft.payload.SignedData; import tech.pegasys.pantheon.consensus.ibft.validation.RoundChangePayloadValidator.MessageValidatorForHeightFactory; import tech.pegasys.pantheon.ethereum.core.Address; -import tech.pegasys.pantheon.ethereum.core.Hash; import java.util.Collection; -import java.util.Optional; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -92,7 +87,7 @@ public boolean validateNewRoundMessage(final SignedData msg) { return false; } - return validateProposalMessageMatchesLatestPrepareCertificate(payload); + return true; } private boolean validateRoundChangeMessagesAndEnsureTargetRoundMatchesRoot( @@ -132,43 +127,4 @@ private boolean validateRoundChangeMessagesAndEnsureTargetRoundMatchesRoot( } return true; } - - private boolean validateProposalMessageMatchesLatestPrepareCertificate( - final NewRoundPayload payload) { - - final RoundChangeCertificate roundChangeCert = payload.getRoundChangeCertificate(); - final Collection> roundChangeMsgs = - roundChangeCert.getRoundChangePayloads(); - - final Optional latestPreparedCertificate = - findLatestPreparedCertificate(roundChangeMsgs); - - if (!latestPreparedCertificate.isPresent()) { - LOG.trace( - "No round change messages have a preparedCertificate, any valid block may be proposed."); - return true; - } - - // Get the hash of the block in latest prepareCert, not including the Round field. - final Hash roundAgnosticBlockHashPreparedCert = - IbftBlockHashing.calculateHashOfIbftBlockOnChain( - latestPreparedCertificate - .get() - .getProposalPayload() - .getPayload() - .getBlock() - .getHeader()); - - final Hash roundAgnosticBlockHashProposal = - IbftBlockHashing.calculateHashOfIbftBlockOnChain( - payload.getProposalPayload().getPayload().getBlock().getHeader()); - - if (!roundAgnosticBlockHashPreparedCert.equals(roundAgnosticBlockHashProposal)) { - LOG.info( - "Invalid NewRound message, block in latest RoundChange does not match proposed block."); - return false; - } - - return true; - } } From afe9989732a5e10d22710436bd4e2054ab9fb128 Mon Sep 17 00:00:00 2001 From: tmohay Date: Mon, 11 Feb 2019 12:08:48 +1100 Subject: [PATCH 7/9] post review --- .../consensus/ibft/statemachine/IbftBlockHeightManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 496b84a1fe..9b4be95fbf 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 @@ -193,7 +193,7 @@ private

> void actionOrBufferMessage @Override public void handleRoundChangePayload(final RoundChange message) { final ConsensusRoundIdentifier targetRound = message.getRoundIdentifier(); - LOG.trace("Received a RoundChange Payload for {}", targetRound.toString()); + LOG.trace("Received a RoundChange Payload for {}", targetRound); final MessageAge messageAge = determineAgeOfPayload(message.getRoundIdentifier().getRoundNumber()); From b9d69ca20359c2c52be0ea94d1e684bab85d73bc Mon Sep 17 00:00:00 2001 From: tmohay Date: Mon, 11 Feb 2019 13:23:54 +1100 Subject: [PATCH 8/9] chain length to chain Height --- .../pantheon/consensus/ibft/statemachine/IbftController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java index aa4aa21167..c8cb5d6223 100644 --- a/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java +++ b/consensus/ibft/src/main/java/tech/pegasys/pantheon/consensus/ibft/statemachine/IbftController.java @@ -158,7 +158,7 @@ private

> void consumeMessage( public void handleNewBlockEvent(final NewChainHead newChainHead) { final BlockHeader newBlockHeader = newChainHead.getNewChainHeadHeader(); final BlockHeader currentMiningParent = currentHeightManager.getParentBlockHeader(); - LOG.debug("Handling New Chain head event, chain length = {}", currentMiningParent.getNumber()); + LOG.debug("Handling New Chain head event, chain height = {}", currentMiningParent.getNumber()); if (newBlockHeader.getNumber() < currentMiningParent.getNumber()) { LOG.trace( "Discarding NewChainHead event, was for previous block height. chainHeight={} eventHeight={}", From cf82f00fc0efe746b8077313c319bc5bf68e364f Mon Sep 17 00:00:00 2001 From: tmohay Date: Mon, 11 Feb 2019 13:24:57 +1100 Subject: [PATCH 9/9] NewRoundMsgValidator info --> trace --- .../consensus/ibft/validation/NewRoundMessageValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 abbd7817e4..06cd05bb47 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 @@ -100,7 +100,7 @@ private boolean validateProposalMessageMatchesLatestPrepareCertificate( findLatestPreparedCertificate(roundChangePayloads); if (!latestPreparedCertificate.isPresent()) { - LOG.info( + LOG.trace( "No round change messages have a preparedCertificate, any valid block may be proposed."); return true; }