diff --git a/common/src/main/java/com/radixdlt/monitoring/Metrics.java b/common/src/main/java/com/radixdlt/monitoring/Metrics.java index 2c34276f73..6ef6dba06c 100644 --- a/common/src/main/java/com/radixdlt/monitoring/Metrics.java +++ b/common/src/main/java/com/radixdlt/monitoring/Metrics.java @@ -192,7 +192,7 @@ public record Bft( Summary leaderTransactionBytesIncludedInProposal, Summary leaderTransactionBytesIncludedInProposalAndPreviousVertices, Summary numSignaturesInCertificate, - Counter divergentVertexExecutions) { + LabelledCounter divergentVertexExecutions) { public record SuccessfullyProcessedVote(boolean isTimeout, VoteProcessingResult result) {} @@ -234,6 +234,8 @@ public record Sync( public record VertexStore( Gauge size, Counter forks, Counter rebuilds, Counter indirectParents) {} + + public record DivergentVertexExecution(int numDistinctExecutionResults) {} } public record BerkeleyDb(AddressBook addressBook, SafetyState safetyState) { diff --git a/core/src/main/java/com/radixdlt/consensus/DivergentVertexExecutionDetector.java b/core/src/main/java/com/radixdlt/consensus/DivergentVertexExecutionDetector.java new file mode 100644 index 0000000000..a790e8fe1e --- /dev/null +++ b/core/src/main/java/com/radixdlt/consensus/DivergentVertexExecutionDetector.java @@ -0,0 +1,176 @@ +/* Copyright 2021 Radix Publishing Ltd incorporated in Jersey (Channel Islands). + * + * Licensed under the Radix License, Version 1.0 (the "License"); you may not use this + * file except in compliance with the License. You may obtain a copy of the License at: + * + * radixfoundation.org/licenses/LICENSE-v1 + * + * The Licensor hereby grants permission for the Canonical version of the Work to be + * published, distributed and used under or by reference to the Licensor’s trademark + * Radix ® and use of any unregistered trade names, logos or get-up. + * + * The Licensor provides the Work (and each Contributor provides its Contributions) on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, + * including, without limitation, any warranties or conditions of TITLE, NON-INFRINGEMENT, + * MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. + * + * Whilst the Work is capable of being deployed, used and adopted (instantiated) to create + * a distributed ledger it is your responsibility to test and validate the code, together + * with all logic and performance of that code under all foreseeable scenarios. + * + * The Licensor does not make or purport to make and hereby excludes liability for all + * and any representation, warranty or undertaking in any form whatsoever, whether express + * or implied, to any entity or person, including any representation, warranty or + * undertaking, as to the functionality security use, value or other characteristics of + * any distributed ledger nor in respect the functioning or value of any tokens which may + * be created stored or transferred using the Work. The Licensor does not warrant that the + * Work or any use of the Work complies with any law or regulation in any territory where + * it may be implemented or used or that it will be appropriate for any specific purpose. + * + * Neither the licensor nor any current or former employees, officers, directors, partners, + * trustees, representatives, agents, advisors, contractors, or volunteers of the Licensor + * shall be liable for any direct or indirect, special, incidental, consequential or other + * losses of any kind, in tort, contract or otherwise (including but not limited to loss + * of revenue, income or profits, or loss of use or data, or loss of reputation, or loss + * of any economic or other opportunity of whatsoever nature or howsoever arising), arising + * out of or in connection with (without limitation of any use, misuse, of any ledger system + * or use made or its functionality or any performance or operation of any code or protocol + * caused by bugs or programming or logic errors or otherwise); + * + * A. any offer, purchase, holding, use, sale, exchange or transmission of any + * cryptographic keys, tokens or assets created, exchanged, stored or arising from any + * interaction with the Work; + * + * B. any failure in a transmission or loss of any token or assets keys or other digital + * artefacts due to errors in transmission; + * + * C. bugs, hacks, logic errors or faults in the Work or any communication; + * + * D. system software or apparatus including but not limited to losses caused by errors + * in holding or transmitting tokens by any third-party; + * + * E. breaches or failure of security including hacker attacks, loss or disclosure of + * password, loss of private key, unauthorised use or misuse of such passwords or keys; + * + * F. any losses including loss of anticipated savings or other benefits resulting from + * use of the Work or any changes to the Work (however implemented). + * + * You are solely responsible for; testing, validating and evaluation of all operation + * logic, functionality, security and appropriateness of using the Work for any commercial + * or non-commercial purpose and for any reproduction or redistribution by You of the + * Work. You assume all risks associated with Your use of the Work and the exercise of + * permissions under this License. + */ + +package com.radixdlt.consensus; + +import com.google.common.hash.HashCode; +import com.google.common.util.concurrent.RateLimiter; +import com.radixdlt.consensus.bft.BFTValidatorId; +import com.radixdlt.consensus.bft.BFTValidatorSet; +import com.radixdlt.consensus.bft.Round; +import com.radixdlt.monitoring.Metrics; +import java.math.BigDecimal; +import java.math.RoundingMode; +import java.util.*; +import java.util.stream.Collectors; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +/** + * A utility for detecting divergent vertex execution, i.e. a situation where two (or more) + * validators from the validator set have voted for the same vertexId, but their resultant ledger + * headers were different. + */ +@SuppressWarnings("UnstableApiUsage") +public final class DivergentVertexExecutionDetector { + private static final Logger log = LogManager.getLogger(); + + private final RateLimiter logRatelimiter = RateLimiter.create(0.05); // At most one log every 20s + + private final Metrics metrics; + private final BFTValidatorSet validatorSet; + + // vertexId -> ledgerHeader -> validators who voted for (vertexId, ledgerHeader) + private final Map>> votesByVertexId = + new HashMap<>(); + + public DivergentVertexExecutionDetector(Metrics metrics, BFTValidatorSet validatorSet) { + this.metrics = metrics; + this.validatorSet = validatorSet; + } + + /** + * Processes a received vote. No additional filtering is applied, the caller should ensure that + * only relevant votes are being processed (e.g. only votes from a single round). + */ + public void processVote(Vote vote) { + final var ledgerHeadersByVertexId = + votesByVertexId.computeIfAbsent( + vote.getVoteData().getProposed().getVertexId(), unused -> new HashMap<>()); + final var authorsByLedgerHeader = + ledgerHeadersByVertexId.computeIfAbsent( + vote.getVoteData().getProposed().getLedgerHeader(), unused -> new HashSet<>()); + authorsByLedgerHeader.add(vote.getAuthor()); + } + + public void summarizeAfterRoundAndReset(Round round) { + // Divergent executions are the ones that have more than one resultant header + // for the same vertexId. + final StringBuilder logBuilder = new StringBuilder(); + votesByVertexId.entrySet().stream() + .filter(e -> e.getValue().size() > 1) + .forEach( + e -> { + final var vertexId = e.getKey(); + final var distinctResults = e.getValue(); + + metrics + .bft() + .divergentVertexExecutions() + .label(new Metrics.Bft.DivergentVertexExecution(distinctResults.size())) + .inc(); + + logBuilder.append( + String.format( + "In round %s validators have voted for vertex %s but they've computed %s" + + " distinct results:\n", + round, vertexId, distinctResults.size())); + final var totalStakeDec = new BigDecimal(validatorSet.getTotalPower().toBigInt()); + for (var result : distinctResults.entrySet()) { + final var stakeVoted = + result.getValue().stream() + .map(v -> new BigDecimal(validatorSet.getPower(v).toBigInt())) + .reduce(BigDecimal.ZERO, BigDecimal::add); + final var stakeVotedProportion = + stakeVoted.divide(totalStakeDec, 4, RoundingMode.HALF_UP); + // Let's list the actual validators if they represent less than 10% stake + final var validatorsDetails = + stakeVotedProportion.compareTo(BigDecimal.valueOf(0.1)) < 0 + ? " (" + + result.getValue().stream() + .map(BFTValidatorId::toString) + .collect(Collectors.joining(",")) + + ")" + : ""; + logBuilder.append( + String.format( + " * %s validator(s)%s representing %s stake computed %s\n", + result.getValue().size(), + validatorsDetails, + stakeVotedProportion, + result.getKey())); + } + }); + + if (logBuilder.length() > 0 && logRatelimiter.tryAcquire()) { + logBuilder.append( + "This is informational only, this node is unaffected unless other error messages" + + " follow.\n"); + log.info(logBuilder.toString()); + } + + // Reset this detector to its initial (empty) state + this.votesByVertexId.clear(); + } +} diff --git a/core/src/main/java/com/radixdlt/consensus/NextEpoch.java b/core/src/main/java/com/radixdlt/consensus/NextEpoch.java index 142282eb91..e178c9a7e7 100644 --- a/core/src/main/java/com/radixdlt/consensus/NextEpoch.java +++ b/core/src/main/java/com/radixdlt/consensus/NextEpoch.java @@ -133,6 +133,17 @@ public int hashCode() { @Override public String toString() { - return "NextEpoch{" + "validators=" + validators + ", epoch=" + epoch + '}'; + final StringBuilder builder = new StringBuilder("NextEpoch{epoch="); + builder.append(epoch); + builder.append(", stake_by_validator_id=["); + validators.forEach( + v -> { + builder.append(v.getValidatorId()); + builder.append("->"); + builder.append(v.getPower()); + builder.append(","); + }); + builder.append("]}"); + return builder.toString(); } } diff --git a/core/src/main/java/com/radixdlt/consensus/PendingVotes.java b/core/src/main/java/com/radixdlt/consensus/PendingVotes.java index adb3997db4..620210b5f1 100644 --- a/core/src/main/java/com/radixdlt/consensus/PendingVotes.java +++ b/core/src/main/java/com/radixdlt/consensus/PendingVotes.java @@ -67,7 +67,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Maps; import com.google.common.hash.HashCode; -import com.google.common.util.concurrent.RateLimiter; import com.radixdlt.SecurityCritical; import com.radixdlt.SecurityCritical.SecurityKind; import com.radixdlt.consensus.bft.BFTValidatorId; @@ -79,13 +78,10 @@ import com.radixdlt.crypto.ECDSASecp256k1Signature; import com.radixdlt.crypto.Hasher; import com.radixdlt.environment.EventDispatcher; -import com.radixdlt.monitoring.Metrics; import java.util.Map; import java.util.Objects; import java.util.Optional; import javax.annotation.concurrent.NotThreadSafe; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; /** * Manages pending votes for various vertices. @@ -97,120 +93,20 @@ @NotThreadSafe @SecurityCritical({SecurityKind.SIG_VERIFY, SecurityKind.GENERAL}) public final class PendingVotes { - private static final Logger log = LogManager.getLogger(); - - private final BFTValidatorId self; private final Map voteState = Maps.newHashMap(); private final Map timeoutVoteState = Maps.newHashMap(); private final Map previousVotes = Maps.newHashMap(); private final Hasher hasher; private final EventDispatcher doubleVoteEventDispatcher; private final BFTValidatorSet validatorSet; - private final Metrics metrics; - private final RateLimiter divergentVertexExecutionLogRateLimiter = RateLimiter.create(0.1); public PendingVotes( - BFTValidatorId self, Hasher hasher, EventDispatcher doubleVoteEventDispatcher, - BFTValidatorSet validatorSet, - Metrics metrics) { - this.self = self; + BFTValidatorSet validatorSet) { this.hasher = Objects.requireNonNull(hasher); this.doubleVoteEventDispatcher = Objects.requireNonNull(doubleVoteEventDispatcher); this.validatorSet = Objects.requireNonNull(validatorSet); - this.metrics = metrics; - } - - private void checkForDivergentVertexExecution(Vote incomingVote) { - final var voteVertexId = incomingVote.getVoteData().getProposed().getVertexId(); - final var voteLedgerHeader = incomingVote.getVoteData().getProposed().getLedgerHeader(); - for (var otherVote : this.previousVotes.entrySet()) { - final var otherVertexId = otherVote.getValue().proposedHeader().getVertexId(); - final var otherLedgerHeader = otherVote.getValue().proposedHeader().getLedgerHeader(); - if (voteVertexId.equals(otherVertexId) && !voteLedgerHeader.equals(otherLedgerHeader)) { - if (!voteLedgerHeader - .nextProtocolVersion() - .equals(otherLedgerHeader.nextProtocolVersion())) { - logDivergentProtocolUpdateVote(incomingVote, otherVote.getKey(), otherVote.getValue()); - } else { - if (divergentVertexExecutionLogRateLimiter.tryAcquire()) { - log.warn( - "Divergent vertex execution detected! An incoming vote (from {}) for vertex {}" - + " claims the following resultant ledger header: {}, while validator {} thinks" - + " that the resultant ledger header is {}. [this_vote={}, other_vote={}]", - incomingVote.getAuthor(), - voteVertexId, - voteLedgerHeader, - otherVote.getKey(), - otherLedgerHeader, - incomingVote, - otherVote); - } - } - - // Regardless of the reason and whether the divergence applies to this node, - // we're bumping the metrics. - this.metrics.bft().divergentVertexExecutions().inc(); - } - } - } - - private void logDivergentProtocolUpdateVote( - Vote incomingVote, BFTValidatorId otherVoteAuthor, PreviousVote otherVote) { - // Protocol update-related divergence is most likely caused by some nodes running an - // outdated version. This is somewhat expected, so we're not going to log these occurrences - // unless they apply to this node. - final var isIncomingVoteFromSelf = self.equals(incomingVote.getAuthor()); - final var isExistingVoteFromSelf = self.equals(otherVoteAuthor); - if (isIncomingVoteFromSelf || isExistingVoteFromSelf) { - final var selfProposedHeader = - isIncomingVoteFromSelf - ? incomingVote.getVoteData().getProposed() - : otherVote.proposedHeader(); - final var otherProposedHeader = - isIncomingVoteFromSelf - ? otherVote.proposedHeader() - : incomingVote.getVoteData().getProposed(); - final var selfNextVersion = selfProposedHeader.getLedgerHeader().nextProtocolVersion(); - final var otherNextVersion = otherProposedHeader.getLedgerHeader().nextProtocolVersion(); - final var otherAuthor = isIncomingVoteFromSelf ? otherVoteAuthor : incomingVote.getAuthor(); - if (selfNextVersion.isPresent() && otherNextVersion.isEmpty()) { - // We're enacting, they don't: just a debug log. - if (divergentVertexExecutionLogRateLimiter.tryAcquire()) { - log.debug( - """ - Received a BFT vote from {} that doesn't enact a protocol update, - while we're enacting {}. - This indicates that they're likely running an outdated node version. - This node is unaffected, unless other error messages follow.""", - otherAuthor, - selfNextVersion); - } else if (selfNextVersion.isEmpty() && otherNextVersion.isPresent()) { - // We're not enacting, but they are: we're potentially outdated, log an early warning - if (divergentVertexExecutionLogRateLimiter.tryAcquire()) { - log.warn( - """ - Received a BFT vote from {} that enacts a protocol update {}, but this node doesn't enact it. - This node might be running an outdated version (but it's not certain).""", - otherAuthor, - otherNextVersion); - } - } else if (selfNextVersion.isPresent() && otherNextVersion.isPresent()) { - // We're enacting a different version, one of us is likely outdated. - if (divergentVertexExecutionLogRateLimiter.tryAcquire()) { - log.warn( - "Received a BFT vote from {} that enacts a protocol update {}, but this node enacts" - + " {}. It is likely that one of the nodes (this node or {}) is running an" - + " outdated version.", - otherAuthor, - otherNextVersion, - selfNextVersion, - otherAuthor); - } - } - } /* else: no-op; we don't care about other cases */ - } /* else: no-op; we don't care if the vote doesn't affect this node */ } /** @@ -230,10 +126,6 @@ public VoteProcessingResult insertVote(Vote vote) { return VoteProcessingResult.rejected(VoteRejectedReason.INVALID_AUTHOR); } - // This doesn't do anything, other than logging and bumping the metrics, - // when divergent execution is detected (which should hopefully never happen). - checkForDivergentVertexExecution(vote); - if (!replacePreviousVote(author, vote, voteDataHash)) { return VoteProcessingResult.rejected(VoteRejectedReason.DUPLICATE_VOTE); } diff --git a/core/src/main/java/com/radixdlt/consensus/bft/BFTBuilder.java b/core/src/main/java/com/radixdlt/consensus/bft/BFTBuilder.java index c70e93c8bb..1f06396014 100644 --- a/core/src/main/java/com/radixdlt/consensus/bft/BFTBuilder.java +++ b/core/src/main/java/com/radixdlt/consensus/bft/BFTBuilder.java @@ -207,8 +207,7 @@ public BFTEventProcessor build() { if (!validatorSet.containsValidator(self)) { return EmptyBFTEventProcessor.INSTANCE; } - final PendingVotes pendingVotes = - new PendingVotes(self, hasher, doubleVoteDispatcher, validatorSet, metrics); + final PendingVotes pendingVotes = new PendingVotes(hasher, doubleVoteDispatcher, validatorSet); /* Setting up the following BFT event processing pipeline: ObsoleteEventsFilter (filters out obsolete events for past rounds) @@ -227,6 +226,7 @@ public BFTEventProcessor build() { roundQuorumResolutionDispatcher, timeoutQuorumDelayedResolutionDispatcher, metrics, + new DivergentVertexExecutionDetector(metrics, validatorSet), pendingVotes, roundUpdate, timeoutQuorumResolutionDelayMs); diff --git a/core/src/main/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssembler.java b/core/src/main/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssembler.java index 4c768d1567..8055ea9431 100644 --- a/core/src/main/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssembler.java +++ b/core/src/main/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssembler.java @@ -64,6 +64,7 @@ package com.radixdlt.consensus.bft.processor; +import com.radixdlt.consensus.DivergentVertexExecutionDetector; import com.radixdlt.consensus.PendingVotes; import com.radixdlt.consensus.Vote; import com.radixdlt.consensus.bft.BFTValidatorId; @@ -87,7 +88,6 @@ * Processes BFT Vote events and assembles a quorum (either a regular or timeout quorum). Warning: * operates under the assumption that all received events are for the current round. */ -@SuppressWarnings("OptionalUsedAsFieldOrParameterType") public final class BFTQuorumAssembler implements BFTEventProcessorAtCurrentRound { private static final Logger log = LogManager.getLogger(); @@ -110,6 +110,8 @@ public record TimeoutQuorumDelayedResolution( private boolean hasCurrentRoundBeenResolved = false; private boolean hasTimeoutQuorumResolutionBeenDelayedInCurrentRound = false; + private final DivergentVertexExecutionDetector divergentVertexExecutionDetector; + public BFTQuorumAssembler( BFTEventProcessorAtCurrentRound forwardTo, BFTValidatorId self, @@ -117,6 +119,7 @@ public BFTQuorumAssembler( ScheduledEventDispatcher timeoutQuorumDelayedResolutionDispatcher, Metrics metrics, + DivergentVertexExecutionDetector divergentVertexExecutionDetector, PendingVotes pendingVotes, RoundUpdate initialRoundUpdate, long timeoutQuorumResolutionDelayMs) { @@ -129,13 +132,20 @@ public BFTQuorumAssembler( this.pendingVotes = Objects.requireNonNull(pendingVotes); this.latestRoundUpdate = Objects.requireNonNull(initialRoundUpdate); this.timeoutQuorumResolutionDelayMs = timeoutQuorumResolutionDelayMs; + this.divergentVertexExecutionDetector = divergentVertexExecutionDetector; } @Override public void processRoundUpdate(RoundUpdate roundUpdate) { + final var previousRound = this.latestRoundUpdate.getCurrentRound(); this.latestRoundUpdate = roundUpdate; this.hasCurrentRoundBeenResolved = false; this.hasTimeoutQuorumResolutionBeenDelayedInCurrentRound = false; + + // Process the divergent vertex executions we've collected in the previous round + // and set it up for the next round. + this.divergentVertexExecutionDetector.summarizeAfterRoundAndReset(previousRound); + forwardTo.processRoundUpdate(roundUpdate); } @@ -162,7 +172,7 @@ private void processVoteInternal(Vote vote) { switch (this.pendingVotes.insertVote(vote)) { case VoteAccepted unused -> { log.trace("Vote has been processed but didn't form a quorum"); - + divergentVertexExecutionDetector.processVote(vote); yield Metrics.Bft.VoteProcessingResult.ACCEPTED_NO_QUORUM; } case VoteRejected voteRejected -> { @@ -174,8 +184,8 @@ yield switch (voteRejected.reason()) { }; } case QuorumReached quorumReached -> { + divergentVertexExecutionDetector.processVote(vote); this.processQuorum(quorumReached.roundQuorum(), vote); - yield switch (quorumReached.roundQuorum()) { case RoundQuorum.RegularRoundQuorum unused -> Metrics.Bft.VoteProcessingResult .ACCEPTED_FORMED_QC; diff --git a/core/src/test/java/com/radixdlt/consensus/DivergentVertexExecutionDetectorTest.java b/core/src/test/java/com/radixdlt/consensus/DivergentVertexExecutionDetectorTest.java new file mode 100644 index 0000000000..e513494eac --- /dev/null +++ b/core/src/test/java/com/radixdlt/consensus/DivergentVertexExecutionDetectorTest.java @@ -0,0 +1,148 @@ +/* Copyright 2021 Radix Publishing Ltd incorporated in Jersey (Channel Islands). + * + * Licensed under the Radix License, Version 1.0 (the "License"); you may not use this + * file except in compliance with the License. You may obtain a copy of the License at: + * + * radixfoundation.org/licenses/LICENSE-v1 + * + * The Licensor hereby grants permission for the Canonical version of the Work to be + * published, distributed and used under or by reference to the Licensor’s trademark + * Radix ® and use of any unregistered trade names, logos or get-up. + * + * The Licensor provides the Work (and each Contributor provides its Contributions) on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied, + * including, without limitation, any warranties or conditions of TITLE, NON-INFRINGEMENT, + * MERCHANTABILITY, or FITNESS FOR A PARTICULAR PURPOSE. + * + * Whilst the Work is capable of being deployed, used and adopted (instantiated) to create + * a distributed ledger it is your responsibility to test and validate the code, together + * with all logic and performance of that code under all foreseeable scenarios. + * + * The Licensor does not make or purport to make and hereby excludes liability for all + * and any representation, warranty or undertaking in any form whatsoever, whether express + * or implied, to any entity or person, including any representation, warranty or + * undertaking, as to the functionality security use, value or other characteristics of + * any distributed ledger nor in respect the functioning or value of any tokens which may + * be created stored or transferred using the Work. The Licensor does not warrant that the + * Work or any use of the Work complies with any law or regulation in any territory where + * it may be implemented or used or that it will be appropriate for any specific purpose. + * + * Neither the licensor nor any current or former employees, officers, directors, partners, + * trustees, representatives, agents, advisors, contractors, or volunteers of the Licensor + * shall be liable for any direct or indirect, special, incidental, consequential or other + * losses of any kind, in tort, contract or otherwise (including but not limited to loss + * of revenue, income or profits, or loss of use or data, or loss of reputation, or loss + * of any economic or other opportunity of whatsoever nature or howsoever arising), arising + * out of or in connection with (without limitation of any use, misuse, of any ledger system + * or use made or its functionality or any performance or operation of any code or protocol + * caused by bugs or programming or logic errors or otherwise); + * + * A. any offer, purchase, holding, use, sale, exchange or transmission of any + * cryptographic keys, tokens or assets created, exchanged, stored or arising from any + * interaction with the Work; + * + * B. any failure in a transmission or loss of any token or assets keys or other digital + * artefacts due to errors in transmission; + * + * C. bugs, hacks, logic errors or faults in the Work or any communication; + * + * D. system software or apparatus including but not limited to losses caused by errors + * in holding or transmitting tokens by any third-party; + * + * E. breaches or failure of security including hacker attacks, loss or disclosure of + * password, loss of private key, unauthorised use or misuse of such passwords or keys; + * + * F. any losses including loss of anticipated savings or other benefits resulting from + * use of the Work or any changes to the Work (however implemented). + * + * You are solely responsible for; testing, validating and evaluation of all operation + * logic, functionality, security and appropriateness of using the Work for any commercial + * or non-commercial purpose and for any reproduction or redistribution by You of the + * Work. You assume all risks associated with Your use of the Work and the exercise of + * permissions under this License. + */ + +package com.radixdlt.consensus; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.*; + +import com.google.common.hash.HashCode; +import com.radixdlt.consensus.bft.BFTValidator; +import com.radixdlt.consensus.bft.BFTValidatorId; +import com.radixdlt.consensus.bft.BFTValidatorSet; +import com.radixdlt.consensus.bft.Round; +import com.radixdlt.crypto.HashUtils; +import com.radixdlt.monitoring.Metrics; +import com.radixdlt.monitoring.MetricsInitializer; +import com.radixdlt.utils.UInt192; +import java.util.List; +import org.junit.Test; + +public final class DivergentVertexExecutionDetectorTest { + @Test + public void when_divergent_execution__then_should_update_the_metrics() { + final var metrics = new MetricsInitializer().initialize(); + final var author1 = BFTValidatorId.random(); + final var author2 = BFTValidatorId.random(); + final var author3 = BFTValidatorId.random(); + final var validatorSet = + BFTValidatorSet.from( + List.of( + BFTValidator.from(author1, UInt192.ONE), + BFTValidator.from(author2, UInt192.ONE), + BFTValidator.from(author3, UInt192.ONE))); + final var detector = new DivergentVertexExecutionDetector(metrics, validatorSet); + final var vertex1 = HashUtils.random256(); + final var vertex2 = HashUtils.random256(); + + // No divergence for vertex1 + detector.processVote(createVoteForVertexId(author1, vertex1, 1L)); + detector.processVote(createVoteForVertexId(author2, vertex1, 1L)); + + // Divergence for vertex2 + detector.processVote(createVoteForVertexId(author1, vertex2, 2L)); + detector.processVote(createVoteForVertexId(author2, vertex2, 2L)); + detector.processVote(createVoteForVertexId(author3, vertex2, 3L)); + + detector.summarizeAfterRoundAndReset(Round.epochInitial()); + + assertEquals( + 1, // Expecting 1 divergent execution with 2 conflicting results (label) + (int) + metrics + .bft() + .divergentVertexExecutions() + .label(new Metrics.Bft.DivergentVertexExecution(2)) + .get()); + + assertEquals( + 1, // Expecting no more results from other labels + (int) metrics.bft().divergentVertexExecutions().getSum()); + } + + private Vote createVoteForVertexId( + BFTValidatorId author, HashCode vertexId, long resultDiscriminator) { + final var ledgerHeader = + LedgerHeader.create( + 0L, + Round.epochInitial(), + resultDiscriminator, // Using a different state version as a discriminator + LedgerHashes.zero(), + 0L, + 0L); + + final var bftHeader = mock(BFTHeader.class); + when(bftHeader.getVertexId()).thenReturn(vertexId); + when(bftHeader.getLedgerHeader()).thenReturn(ledgerHeader); + + final var voteData = mock(VoteData.class); + when(voteData.getProposed()).thenReturn(bftHeader); + + final var vote = mock(Vote.class); + when(vote.getAuthor()).thenReturn(author); + when(vote.getVoteData()).thenReturn(voteData); + + return vote; + } +} diff --git a/core/src/test/java/com/radixdlt/consensus/PendingVotesTest.java b/core/src/test/java/com/radixdlt/consensus/PendingVotesTest.java index 6b52b68c03..59a8ac4a31 100644 --- a/core/src/test/java/com/radixdlt/consensus/PendingVotesTest.java +++ b/core/src/test/java/com/radixdlt/consensus/PendingVotesTest.java @@ -78,9 +78,6 @@ import com.radixdlt.crypto.ECDSASecp256k1Signature; import com.radixdlt.crypto.HashUtils; import com.radixdlt.crypto.Hasher; -import com.radixdlt.lang.Option; -import com.radixdlt.monitoring.Metrics; -import com.radixdlt.monitoring.MetricsInitializer; import com.radixdlt.utils.RandomHasher; import com.radixdlt.utils.UInt192; import java.util.Arrays; @@ -91,15 +88,11 @@ import org.junit.Test; public class PendingVotesTest { - private BFTValidatorId self; private Hasher hasher; - private Metrics metrics; @Before public void setup() { - this.self = BFTValidatorId.random(); this.hasher = new RandomHasher(); - this.metrics = new MetricsInitializer().initialize(); } @Test @@ -118,7 +111,7 @@ public void when_inserting_valid_but_unaccepted_votes__then_no_qc_is_returned() BFTValidatorSet validatorSet = BFTValidatorSet.from( Collections.singleton(BFTValidator.from(vote1.getAuthor(), UInt192.ONE))); - final var pendingVotes = new PendingVotes(self, hasher, e -> {}, validatorSet, metrics); + final var pendingVotes = new PendingVotes(hasher, e -> {}, validatorSet); VoteData voteData = mock(VoteData.class); BFTHeader proposed = vote1.getVoteData().getProposed(); @@ -147,7 +140,7 @@ public void when_inserting_valid_and_accepted_votes__then_qc_is_formed() { BFTHeader proposed = vote.getVoteData().getProposed(); when(voteData.getProposed()).thenReturn(proposed); - final var pendingVotes = new PendingVotes(self, hasher, e -> {}, validatorSet, metrics); + final var pendingVotes = new PendingVotes(hasher, e -> {}, validatorSet); assertTrue(pendingVotes.insertVote(vote) instanceof VoteProcessingResult.QuorumReached); } @@ -169,7 +162,7 @@ public void when_inserting_valid_timeout_votes__then_tc_is_formed() { BFTValidator.from(vote1.getAuthor(), UInt192.ONE), BFTValidator.from(vote2.getAuthor(), UInt192.ONE))); - final var pendingVotes = new PendingVotes(self, hasher, e -> {}, validatorSet, metrics); + final var pendingVotes = new PendingVotes(hasher, e -> {}, validatorSet); assertTrue(pendingVotes.insertVote(vote1) instanceof VoteProcessingResult.VoteAccepted); @@ -199,7 +192,7 @@ public void when_voting_again__previous_vote_is_removed() { BFTHeader proposed = vote.getVoteData().getProposed(); when(voteData.getProposed()).thenReturn(proposed); - final var pendingVotes = new PendingVotes(self, hasher, e -> {}, validatorSet, metrics); + final var pendingVotes = new PendingVotes(hasher, e -> {}, validatorSet); // Preconditions assertEquals(VoteProcessingResult.accepted(), pendingVotes.insertVote(vote)); @@ -232,7 +225,7 @@ public void when_voting_again__previous_timeoutvote_is_removed() { BFTHeader proposed = vote.getVoteData().getProposed(); when(voteData.getProposed()).thenReturn(proposed); - final var pendingVotes = new PendingVotes(self, hasher, e -> {}, validatorSet, metrics); + final var pendingVotes = new PendingVotes(hasher, e -> {}, validatorSet); // Preconditions assertEquals(VoteProcessingResult.accepted(), pendingVotes.insertVote(vote)); @@ -267,7 +260,7 @@ public void when_submitting_a_duplicate_vote__then_can_be_replaced_if_has_timeou BFTValidator.from(vote1.getAuthor(), UInt192.ONE), BFTValidator.from(vote2.getAuthor(), UInt192.ONE))); - final var pendingVotes = new PendingVotes(self, hasher, e -> {}, validatorSet, metrics); + final var pendingVotes = new PendingVotes(hasher, e -> {}, validatorSet); assertTrue(pendingVotes.insertVote(vote1) instanceof VoteProcessingResult.VoteAccepted); @@ -294,52 +287,6 @@ public void when_submitting_a_duplicate_vote__then_can_be_replaced_if_has_timeou instanceof RoundQuorum.TimeoutRoundQuorum); } - @Test - public void divergent_vertex_execution_should_be_detected() { - // Arrange: create two votes on the same vertexId, but having - // different ledger headers - final var vertexId = HashUtils.random256(); - - final var ledgerHeader1 = mock(LedgerHeader.class); - when(ledgerHeader1.nextProtocolVersion()).thenReturn(Option.empty()); - final var bftHeader1 = mock(BFTHeader.class); - when(bftHeader1.getLedgerHeader()).thenReturn(ledgerHeader1); - when(bftHeader1.getVertexId()).thenReturn(vertexId); - final var voteData1 = mock(VoteData.class); - when(voteData1.getProposed()).thenReturn(bftHeader1); - final var vote1 = - makeVoteWithoutSignatureFor(mock(BFTValidatorId.class), Round.epochInitial(), vertexId); - when(vote1.getSignature()).thenReturn(ECDSASecp256k1Signature.zeroSignature()); - when(vote1.getVoteData()).thenReturn(voteData1); - - final var ledgerHeader2 = mock(LedgerHeader.class); - when(ledgerHeader2.nextProtocolVersion()).thenReturn(Option.empty()); - final var bftHeader2 = mock(BFTHeader.class); - when(bftHeader2.getLedgerHeader()).thenReturn(ledgerHeader2); - when(bftHeader2.getVertexId()).thenReturn(vertexId); - final var voteData2 = mock(VoteData.class); - when(voteData2.getProposed()).thenReturn(bftHeader2); - final var vote2 = - makeVoteWithoutSignatureFor(mock(BFTValidatorId.class), Round.epochInitial(), vertexId); - when(vote2.getSignature()).thenReturn(ECDSASecp256k1Signature.zeroSignature()); - when(vote2.getVoteData()).thenReturn(voteData2); - - final var validatorSet = - BFTValidatorSet.from( - Arrays.asList( - BFTValidator.from(vote1.getAuthor(), UInt192.ONE), - BFTValidator.from(vote2.getAuthor(), UInt192.ONE))); - - final var pendingVotes = new PendingVotes(self, hasher, e -> {}, validatorSet, metrics); - - // Should still accept both votes... - assertTrue(pendingVotes.insertVote(vote1) instanceof VoteProcessingResult.VoteAccepted); - assertTrue(pendingVotes.insertVote(vote2) instanceof VoteProcessingResult.VoteAccepted); - - // ...but produce a warning and bump the metrics - assertEquals((int) metrics.bft().divergentVertexExecutions().get(), 1); - } - private Vote makeSignedVoteFor(BFTValidatorId author, Round parentRound, HashCode vertexId) { Vote vote = makeVoteWithoutSignatureFor(author, parentRound, vertexId); when(vote.getSignature()).thenReturn(ECDSASecp256k1Signature.zeroSignature()); diff --git a/core/src/test/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssemblerTest.java b/core/src/test/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssemblerTest.java index b60a74de62..229a2ad025 100644 --- a/core/src/test/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssemblerTest.java +++ b/core/src/test/java/com/radixdlt/consensus/bft/processor/BFTQuorumAssemblerTest.java @@ -84,6 +84,7 @@ public final class BFTQuorumAssemblerTest { private BFTValidatorId self = mock(BFTValidatorId.class); private Metrics metrics = new MetricsInitializer().initialize(); private PendingVotes pendingVotes = mock(PendingVotes.class); + private BFTValidatorSet validatorSet = mock(BFTValidatorSet.class); private VertexStoreAdapter vertexStore = mock(VertexStoreAdapter.class); private Pacemaker pacemaker = mock(Pacemaker.class); private EventDispatcher roundQuorumResolutionDispatcher = @@ -102,6 +103,7 @@ public void setUp() { this.roundQuorumResolutionDispatcher, this.timeoutQuorumDelayedResolutionDispatcher, this.metrics, + new DivergentVertexExecutionDetector(metrics, validatorSet), this.pendingVotes, mock(RoundUpdate.class), 1000L); @@ -118,6 +120,11 @@ public void when_process_vote_with_quorum__then_processed() { QuorumCertificate highestCommittedQc = mock(QuorumCertificate.class); when(highQc.highestCommittedQC()).thenReturn(highestCommittedQc); when(vote.getRound()).thenReturn(Round.of(1)); + final var bftHeader = mock(BFTHeader.class); + when(bftHeader.getLedgerHeader()).thenReturn(mock(LedgerHeader.class)); + final var voteData = mock(VoteData.class); + when(voteData.getProposed()).thenReturn(bftHeader); + when(vote.getVoteData()).thenReturn(voteData); when(this.pendingVotes.insertVote(any())).thenReturn(VoteProcessingResult.regularQuorum(qc)); when(this.vertexStore.highQC()).thenReturn(highQc);