diff --git a/.gitignore b/.gitignore index 48883106db3..e02bb0b5d0b 100644 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ .DS_Store .externalToolBuilders/ .gradle/ +.vscode/ .idea/ .loadpath .metadata diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a6b80cdeda..a0b2387fcb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Implement debug_traceCall [#5885](https://github.com/hyperledger/besu/pull/5885) - Transactions that takes too long to evaluate, during block creation, are dropped from the txpool [#6163](https://github.com/hyperledger/besu/pull/6163) - New option `tx-pool-min-gas-price` to set a lower bound when accepting txs to the pool [#6098](https://github.com/hyperledger/besu/pull/6098) +- Allow a transaction selection plugin to specify custom selection results [#6190](https://github.com/hyperledger/besu/pull/6190) ## 23.10.2 diff --git a/besu/build.gradle b/besu/build.gradle index 73977d81ba7..9d72f50b021 100644 --- a/besu/build.gradle +++ b/besu/build.gradle @@ -90,7 +90,7 @@ dependencies { testImplementation 'com.squareup.okhttp3:okhttp' testImplementation 'commons-io:commons-io' testImplementation 'io.opentelemetry:opentelemetry-api' - testImplementation 'junit:junit' + testImplementation 'org.mockito:mockito-junit-jupiter' testImplementation 'org.apache.commons:commons-text' testImplementation 'io.tmio:tuweni-bytes' testImplementation 'io.tmio:tuweni-units' diff --git a/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java b/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java index f9d543aca9f..d65569535aa 100644 --- a/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java +++ b/besu/src/test/java/org/hyperledger/besu/chainimport/RlpBlockImporterTest.java @@ -41,23 +41,21 @@ import java.util.concurrent.CompletionException; import org.apache.tuweni.units.bigints.UInt256; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.TemporaryFolder; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; +import org.mockito.junit.jupiter.MockitoExtension; /** Tests for {@link RlpBlockImporter}. */ -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public final class RlpBlockImporterTest { - @Rule public final TemporaryFolder folder = new TemporaryFolder(); + @TempDir Path dataDir; private final RlpBlockImporter rlpBlockImporter = new RlpBlockImporter(); @Test public void blockImport() throws IOException { - final Path dataDir = folder.newFolder().toPath(); final Path source = dataDir.resolve("1000.blocks"); BlockTestUtil.write1000Blocks(source); final BesuController targetController = @@ -90,7 +88,6 @@ public void blockImportRejectsBadPow() throws IOException { // set merge flag to false, otherwise this test can fail if a merge test runs first MergeConfigOptions.setMergeEnabled(false); - final Path dataDir = folder.newFolder().toPath(); final Path source = dataDir.resolve("badpow.blocks"); BlockTestUtil.writeBadPowBlocks(source); final BesuController targetController = @@ -120,7 +117,6 @@ public void blockImportRejectsBadPow() throws IOException { @Test public void blockImportCanSkipPow() throws IOException { - final Path dataDir = folder.newFolder().toPath(); final Path source = dataDir.resolve("badpow.blocks"); BlockTestUtil.writeBadPowBlocks(source); final BesuController targetController = diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java index fc8ee0bd6c1..4a0707e76f7 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/BlockTransactionSelector.java @@ -57,6 +57,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; +import com.google.common.base.Stopwatch; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -227,11 +228,11 @@ private TransactionSelectionResult evaluateTransaction( final PendingTransaction pendingTransaction) { checkCancellation(); - final long evaluationStartedAt = System.currentTimeMillis(); + final Stopwatch evaluationTimer = Stopwatch.createStarted(); TransactionSelectionResult selectionResult = evaluatePreProcessing(pendingTransaction); if (!selectionResult.selected()) { - return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationStartedAt); + return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationTimer); } final WorldUpdater txWorldStateUpdater = blockWorldStateUpdater.updater(); @@ -243,13 +244,10 @@ private TransactionSelectionResult evaluateTransaction( if (postProcessingSelectionResult.selected()) { return handleTransactionSelected( - pendingTransaction, processingResult, txWorldStateUpdater, evaluationStartedAt); + pendingTransaction, processingResult, txWorldStateUpdater, evaluationTimer); } return handleTransactionNotSelected( - pendingTransaction, - postProcessingSelectionResult, - txWorldStateUpdater, - evaluationStartedAt); + pendingTransaction, postProcessingSelectionResult, txWorldStateUpdater, evaluationTimer); } /** @@ -333,14 +331,14 @@ private TransactionProcessingResult processTransaction( * @param pendingTransaction The pending transaction. * @param processingResult The result of the transaction processing. * @param txWorldStateUpdater The world state updater. - * @param evaluationStartedAt when the evaluation of this tx started + * @param evaluationTimer tracks the evaluation elapsed time * @return The result of the transaction selection process. */ private TransactionSelectionResult handleTransactionSelected( final PendingTransaction pendingTransaction, final TransactionProcessingResult processingResult, final WorldUpdater txWorldStateUpdater, - final long evaluationStartedAt) { + final Stopwatch evaluationTimer) { final Transaction transaction = pendingTransaction.getTransaction(); final long gasUsedByTransaction = @@ -369,20 +367,19 @@ private TransactionSelectionResult handleTransactionSelected( } } - final long evaluationTime = System.currentTimeMillis() - evaluationStartedAt; if (tooLate) { // even if this tx passed all the checks, it is too late to include it in this block, // so we need to treat it as not selected // check if this tx took too much to evaluate, and in case remove it from the pool final TransactionSelectionResult timeoutSelectionResult; - if (evaluationTime > blockTxsSelectionMaxTime) { + if (evaluationTimer.elapsed(TimeUnit.MILLISECONDS) > blockTxsSelectionMaxTime) { LOG.atWarn() .setMessage( - "Transaction {} is too late for inclusion, evaluated in {}ms that is over the max limit of {}" + "Transaction {} is too late for inclusion, evaluated in {} that is over the max limit of {}ms" + ", removing it from the pool") .addArgument(transaction::toTraceLog) - .addArgument(evaluationTime) + .addArgument(evaluationTimer) .addArgument(blockTxsSelectionMaxTime) .log(); timeoutSelectionResult = TX_EVALUATION_TOO_LONG; @@ -390,7 +387,7 @@ private TransactionSelectionResult handleTransactionSelected( LOG.atTrace() .setMessage("Transaction {} is too late for inclusion") .addArgument(transaction::toTraceLog) - .addArgument(evaluationTime) + .addArgument(evaluationTimer) .log(); timeoutSelectionResult = BLOCK_SELECTION_TIMEOUT; } @@ -398,15 +395,15 @@ private TransactionSelectionResult handleTransactionSelected( // do not rely on the presence of this result, since by the time it is added, the code // reading it could have been already executed by another thread return handleTransactionNotSelected( - pendingTransaction, timeoutSelectionResult, txWorldStateUpdater, evaluationStartedAt); + pendingTransaction, timeoutSelectionResult, txWorldStateUpdater, evaluationTimer); } pluginTransactionSelector.onTransactionSelected(pendingTransaction, processingResult); blockWorldStateUpdater = worldState.updater(); LOG.atTrace() - .setMessage("Selected {} for block creation, evaluated in {}ms") + .setMessage("Selected {} for block creation, evaluated in {}") .addArgument(transaction::toTraceLog) - .addArgument(evaluationTime) + .addArgument(evaluationTimer) .log(); return SELECTED; } @@ -418,22 +415,22 @@ private TransactionSelectionResult handleTransactionSelected( * * @param pendingTransaction The unselected pending transaction. * @param selectionResult The result of the transaction selection process. - * @param evaluationStartedAt when the evaluation of this tx started + * @param evaluationTimer tracks the evaluation elapsed time * @return The result of the transaction selection process. */ private TransactionSelectionResult handleTransactionNotSelected( final PendingTransaction pendingTransaction, final TransactionSelectionResult selectionResult, - final long evaluationStartedAt) { + final Stopwatch evaluationTimer) { transactionSelectionResults.updateNotSelected( pendingTransaction.getTransaction(), selectionResult); pluginTransactionSelector.onTransactionNotSelected(pendingTransaction, selectionResult); LOG.atTrace() - .setMessage("Not selected {} for block creation with result {}, evaluated in {}ms") + .setMessage("Not selected {} for block creation with result {}, evaluated in {}") .addArgument(pendingTransaction::toTraceLog) .addArgument(selectionResult) - .addArgument(() -> System.currentTimeMillis() - evaluationStartedAt) + .addArgument(evaluationTimer) .log(); return selectionResult; @@ -443,9 +440,9 @@ private TransactionSelectionResult handleTransactionNotSelected( final PendingTransaction pendingTransaction, final TransactionSelectionResult selectionResult, final WorldUpdater txWorldStateUpdater, - final long evaluationStartedAt) { + final Stopwatch evaluationTimer) { txWorldStateUpdater.revert(); - return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationStartedAt); + return handleTransactionNotSelected(pendingTransaction, selectionResult, evaluationTimer); } private void checkCancellation() { diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/TransactionSelectionResults.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/TransactionSelectionResults.java index 848789d9968..dcbd871aaec 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/TransactionSelectionResults.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/TransactionSelectionResults.java @@ -111,7 +111,7 @@ public void logSelectionStats() { "Selection stats: Totals[Evaluated={}, Selected={}, NotSelected={}, Discarded={}]; Detailed[{}]", selectedTransactions.size() + notSelectedTxs.size(), selectedTransactions.size(), - notSelectedStats.size(), + notSelectedTxs.size(), notSelectedStats.entrySet().stream() .filter(e -> e.getKey().discard()) .map(Map.Entry::getValue) diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java index a38fd4a0a4d..defd75cb77c 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/txselection/selectors/PriceTransactionSelector.java @@ -89,8 +89,9 @@ private boolean transactionCurrentPriceBelowMin(final PendingTransaction pending .setMessage( "Current gas price of {} is {} and lower than the configured minimum {}, skipping") .addArgument(pendingTransaction::toTraceLog) - .addArgument(transactionGasPriceInBlock) - .addArgument(context.miningParameters()::getMinTransactionGasPrice) + .addArgument(transactionGasPriceInBlock::toHumanReadableString) + .addArgument( + context.miningParameters().getMinTransactionGasPrice()::toHumanReadableString) .log(); return true; } diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java index 8efe368d707..926f377bafd 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockTransactionSelectorTest.java @@ -587,9 +587,9 @@ public void transactionSelectionPluginShouldWork_PreProcessing() { public TransactionSelectionResult evaluateTransactionPreProcessing( final PendingTransaction pendingTransaction) { if (pendingTransaction.getTransaction().equals(notSelectedTransient)) - return TransactionSelectionResult.invalidTransient("transient"); + return PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT; if (pendingTransaction.getTransaction().equals(notSelectedInvalid)) - return TransactionSelectionResult.invalid("invalid"); + return PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID; return SELECTED; } @@ -622,8 +622,10 @@ public TransactionSelectionResult evaluateTransactionPostProcessing( assertThat(transactionSelectionResults.getSelectedTransactions()).containsOnly(selected); assertThat(transactionSelectionResults.getNotSelectedTransactions()) .containsOnly( - entry(notSelectedTransient, TransactionSelectionResult.invalidTransient("transient")), - entry(notSelectedInvalid, TransactionSelectionResult.invalid("invalid"))); + entry( + notSelectedTransient, + PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT), + entry(notSelectedInvalid, PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID)); } @Test @@ -658,7 +660,7 @@ public TransactionSelectionResult evaluateTransactionPostProcessing( processingResult) { // the transaction with max gas +1 should fail if (processingResult.getEstimateGasUsedByTransaction() > maxGasUsedByTransaction) { - return TransactionSelectionResult.invalidTransient("Invalid"); + return PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT; } return SELECTED; } @@ -682,7 +684,8 @@ public TransactionSelectionResult evaluateTransactionPostProcessing( assertThat(transactionSelectionResults.getSelectedTransactions()).contains(selected, selected3); assertThat(transactionSelectionResults.getNotSelectedTransactions()) - .containsOnly(entry(notSelected, TransactionSelectionResult.invalidTransient("Invalid"))); + .containsOnly( + entry(notSelected, PluginTransactionSelectionResult.GENERIC_PLUGIN_INVALID_TRANSIENT)); } @Test @@ -1193,4 +1196,48 @@ protected MiningParameters createMiningParameters( .build()) .build(); } + + private static class PluginTransactionSelectionResult extends TransactionSelectionResult { + private enum PluginStatus implements Status { + PLUGIN_INVALID(false, true), + PLUGIN_INVALID_TRANSIENT(false, false); + + private final boolean stop; + private final boolean discard; + + PluginStatus(final boolean stop, final boolean discard) { + this.stop = stop; + this.discard = discard; + } + + @Override + public boolean stop() { + return stop; + } + + @Override + public boolean discard() { + return discard; + } + } + + public static final TransactionSelectionResult GENERIC_PLUGIN_INVALID_TRANSIENT = + invalidTransient("GENERIC_PLUGIN_INVALID_TRANSIENT"); + + public static final TransactionSelectionResult GENERIC_PLUGIN_INVALID = + invalid("GENERIC_PLUGIN_INVALID"); + + private PluginTransactionSelectionResult(final Status status, final String invalidReason) { + super(status, invalidReason); + } + + public static TransactionSelectionResult invalidTransient(final String invalidReason) { + return new PluginTransactionSelectionResult( + PluginStatus.PLUGIN_INVALID_TRANSIENT, invalidReason); + } + + public static TransactionSelectionResult invalid(final String invalidReason) { + return new PluginTransactionSelectionResult(PluginStatus.PLUGIN_INVALID, invalidReason); + } + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java index a5af86551bb..36f57b15686 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthPeers.java @@ -197,7 +197,7 @@ private boolean registerDisconnect( disconnectCallbacks.forEach(callback -> callback.onDisconnect(peer)); peer.handleDisconnect(); abortPendingRequestsAssignedToDisconnectedPeers(); - LOG.debug("Disconnected EthPeer {}", peer.getShortNodeId()); + LOG.debug("Disconnected EthPeer {}...", peer.getShortNodeId()); LOG.trace("Disconnected EthPeer {}", peer); } } @@ -391,7 +391,7 @@ public void disconnectWorstUselessPeer() { peer -> { LOG.atDebug() .setMessage( - "disconnecting peer {}. Waiting for better peers. Current {} of max {}") + "disconnecting peer {}... Waiting for better peers. Current {} of max {}") .addArgument(peer::getShortNodeId) .addArgument(this::peerCount) .addArgument(this::getMaxPeers) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java index dc9aad18c66..6a00fb90e76 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java @@ -398,7 +398,7 @@ public void handleDisconnect( "Disconnect - {} - {} - {}... - {} peers left\n{}", initiatedByPeer ? "Inbound" : "Outbound", reason, - connection.getPeer().getId().slice(0, 16), + connection.getPeer().getId().slice(0, 8), ethPeers.peerCount(), ethPeers); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputation.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputation.java index e4c263ea783..ff7617b0f46 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputation.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputation.java @@ -34,10 +34,13 @@ public class PeerReputation implements Comparable { static final long USELESS_RESPONSE_WINDOW_IN_MILLIS = TimeUnit.MILLISECONDS.convert(1, TimeUnit.MINUTES); - static final int DEFAULT_MAX_SCORE = 150; + static final int DEFAULT_MAX_SCORE = 200; + // how much above the initial score you need to be to not get disconnected for timeouts/useless + // responses + private final int hasBeenUsefulThreshold; static final int DEFAULT_INITIAL_SCORE = 100; private static final Logger LOG = LoggerFactory.getLogger(PeerReputation.class); - private static final int TIMEOUT_THRESHOLD = 3; + private static final int TIMEOUT_THRESHOLD = 5; private static final int USELESS_RESPONSE_THRESHOLD = 5; private final ConcurrentMap timeoutCountByRequestType = @@ -45,8 +48,7 @@ public class PeerReputation implements Comparable { private final Queue uselessResponseTimes = new ConcurrentLinkedQueue<>(); private static final int SMALL_ADJUSTMENT = 1; - private static final int LARGE_ADJUSTMENT = 10; - + private static final int LARGE_ADJUSTMENT = 5; private int score; private final int maxScore; @@ -59,22 +61,37 @@ public PeerReputation(final int initialScore, final int maxScore) { checkArgument( initialScore <= maxScore, "Initial score must be less than or equal to max score"); this.maxScore = maxScore; + this.hasBeenUsefulThreshold = Math.min(maxScore, initialScore + 10); this.score = initialScore; } public Optional recordRequestTimeout(final int requestCode) { final int newTimeoutCount = getOrCreateTimeoutCount(requestCode).incrementAndGet(); if (newTimeoutCount >= TIMEOUT_THRESHOLD) { - LOG.debug( - "Disconnection triggered by {} repeated timeouts for requestCode {}", - newTimeoutCount, - requestCode); score -= LARGE_ADJUSTMENT; - return Optional.of(DisconnectReason.TIMEOUT); + // don't trigger disconnect if this peer has a sufficiently high reputation score + if (peerHasNotBeenUseful()) { + LOG.debug( + "Disconnection triggered by {} repeated timeouts for requestCode {}, peer score {}", + newTimeoutCount, + requestCode, + score); + return Optional.of(DisconnectReason.TIMEOUT); + } + + LOG.trace( + "Not triggering disconnect for {} repeated timeouts for requestCode {} because peer has high score {}", + newTimeoutCount, + requestCode, + score); } else { score -= SMALL_ADJUSTMENT; - return Optional.empty(); } + return Optional.empty(); + } + + private boolean peerHasNotBeenUseful() { + return score < hasBeenUsefulThreshold; } public void resetTimeoutCount(final int requestCode) { @@ -96,12 +113,19 @@ public Optional recordUselessResponse(final long timestamp) { } if (uselessResponseTimes.size() >= USELESS_RESPONSE_THRESHOLD) { score -= LARGE_ADJUSTMENT; - LOG.debug("Disconnection triggered by exceeding useless response threshold"); - return Optional.of(DisconnectReason.USELESS_PEER); + // don't trigger disconnect if this peer has a sufficiently high reputation score + if (peerHasNotBeenUseful()) { + LOG.debug( + "Disconnection triggered by exceeding useless response threshold, score {}", score); + return Optional.of(DisconnectReason.USELESS_PEER); + } + LOG.trace( + "Not triggering disconnect for exceeding useless response threshold because peer has high score {}", + score); } else { score -= SMALL_ADJUSTMENT; - return Optional.empty(); } + return Optional.empty(); } public void recordUsefulResponse() { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputationTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputationTest.java index 9706369a069..f49abee8bb9 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputationTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/PeerReputationTest.java @@ -36,6 +36,8 @@ public void shouldThrowOnInvalidInitialScore() { @Test public void shouldOnlyDisconnectWhenTimeoutLimitReached() { + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).contains(TIMEOUT); @@ -45,6 +47,11 @@ public void shouldOnlyDisconnectWhenTimeoutLimitReached() { public void shouldTrackTimeoutsSeparatelyForDifferentRequestTypes() { assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); + + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); @@ -57,6 +64,8 @@ public void shouldResetTimeoutCountForRequestType() { assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_HEADERS)).isEmpty(); + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); + assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); assertThat(reputation.recordRequestTimeout(EthPV62.GET_BLOCK_BODIES)).isEmpty(); diff --git a/nat/build.gradle b/nat/build.gradle index 311183b0015..f6c814ad113 100644 --- a/nat/build.gradle +++ b/nat/build.gradle @@ -41,7 +41,7 @@ dependencies { testImplementation project(path: ':ethereum:core', configuration: 'testSupportArtifacts') testImplementation project(':testutil') - testImplementation 'junit:junit' + testImplementation 'org.mockito:mockito-junit-jupiter' testImplementation 'org.assertj:assertj-core' testImplementation 'org.junit.jupiter:junit-jupiter' testImplementation 'org.mockito:mockito-core' diff --git a/nat/src/test/java/org/hyperledger/besu/nat/NatServiceTest.java b/nat/src/test/java/org/hyperledger/besu/nat/NatServiceTest.java index 2209c25e3f5..ae51ab6a352 100644 --- a/nat/src/test/java/org/hyperledger/besu/nat/NatServiceTest.java +++ b/nat/src/test/java/org/hyperledger/besu/nat/NatServiceTest.java @@ -33,11 +33,11 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.junit.MockitoJUnitRunner; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class NatServiceTest { @Test diff --git a/nat/src/test/java/org/hyperledger/besu/nat/core/AbstractNatManagerTest.java b/nat/src/test/java/org/hyperledger/besu/nat/core/AbstractNatManagerTest.java index 974f8d5df18..768d2a2bd01 100644 --- a/nat/src/test/java/org/hyperledger/besu/nat/core/AbstractNatManagerTest.java +++ b/nat/src/test/java/org/hyperledger/besu/nat/core/AbstractNatManagerTest.java @@ -29,12 +29,12 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mockito; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public class AbstractNatManagerTest { @Test diff --git a/nat/src/test/java/org/hyperledger/besu/nat/docker/DockerNatManagerTest.java b/nat/src/test/java/org/hyperledger/besu/nat/docker/DockerNatManagerTest.java index 84c8548c7f6..a035882ef06 100644 --- a/nat/src/test/java/org/hyperledger/besu/nat/docker/DockerNatManagerTest.java +++ b/nat/src/test/java/org/hyperledger/besu/nat/docker/DockerNatManagerTest.java @@ -30,13 +30,13 @@ import java.util.concurrent.ExecutionException; import org.assertj.core.api.Assertions; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public final class DockerNatManagerTest { private final String advertisedHost = "99.45.69.12"; @@ -49,7 +49,7 @@ public final class DockerNatManagerTest { private DockerNatManager natManager; - @Before + @BeforeEach public void initialize() throws NatInitializationException { hostBasedIpDetector = mock(HostBasedIpDetector.class); when(hostBasedIpDetector.detectAdvertisedIp()).thenReturn(Optional.of(detectedAdvertisedHost)); diff --git a/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesClusterIpNatManagerTest.java b/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesClusterIpNatManagerTest.java index 2164c172d14..da218eb95cf 100644 --- a/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesClusterIpNatManagerTest.java +++ b/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesClusterIpNatManagerTest.java @@ -33,13 +33,13 @@ import io.kubernetes.client.openapi.models.V1Service; import io.kubernetes.client.openapi.models.V1ServicePort; import io.kubernetes.client.openapi.models.V1ServiceSpec; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public final class KubernetesClusterIpNatManagerTest { private final String detectedAdvertisedHost = "199.45.69.12"; @@ -51,7 +51,7 @@ public final class KubernetesClusterIpNatManagerTest { private KubernetesNatManager natManager; - @Before + @BeforeEach public void initialize() throws IOException { when(v1Service.getSpec()) diff --git a/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesLoadManagerNatManagerTest.java b/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesLoadManagerNatManagerTest.java index 931d13ffbc5..b38314e6eca 100644 --- a/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesLoadManagerNatManagerTest.java +++ b/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesLoadManagerNatManagerTest.java @@ -36,13 +36,13 @@ import io.kubernetes.client.openapi.models.V1ServicePort; import io.kubernetes.client.openapi.models.V1ServiceSpec; import io.kubernetes.client.openapi.models.V1ServiceStatus; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public final class KubernetesLoadManagerNatManagerTest { private final String detectedAdvertisedHost = "199.45.69.12"; @@ -54,7 +54,7 @@ public final class KubernetesLoadManagerNatManagerTest { private KubernetesNatManager natManager; - @Before + @BeforeEach public void initialize() throws IOException { final V1ServiceStatus v1ServiceStatus = new V1ServiceStatus() diff --git a/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesUnknownNatManagerTest.java b/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesUnknownNatManagerTest.java index fb180fde815..d3d53f75dda 100644 --- a/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesUnknownNatManagerTest.java +++ b/nat/src/test/java/org/hyperledger/besu/nat/kubernetes/KubernetesUnknownNatManagerTest.java @@ -21,20 +21,20 @@ import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Service; import io.kubernetes.client.openapi.models.V1ServiceSpec; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; -@RunWith(MockitoJUnitRunner.class) +@ExtendWith(MockitoExtension.class) public final class KubernetesUnknownNatManagerTest { @Mock private V1Service v1Service; private KubernetesNatManager natManager; - @Before + @BeforeEach public void initialize() { when(v1Service.getSpec()).thenReturn(new V1ServiceSpec().type("Unknown")); diff --git a/nat/src/test/java/org/hyperledger/besu/nat/upnp/UpnpNatManagerTest.java b/nat/src/test/java/org/hyperledger/besu/nat/upnp/UpnpNatManagerTest.java index 4c8aa93f020..1090f2d0269 100644 --- a/nat/src/test/java/org/hyperledger/besu/nat/upnp/UpnpNatManagerTest.java +++ b/nat/src/test/java/org/hyperledger/besu/nat/upnp/UpnpNatManagerTest.java @@ -30,8 +30,8 @@ import java.net.URI; import java.net.URL; -import org.junit.Before; -import org.junit.Test; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.jupnp.UpnpService; import org.jupnp.controlpoint.ControlPoint; import org.jupnp.model.meta.DeviceDetails; @@ -54,7 +54,7 @@ public final class UpnpNatManagerTest { private UpnpNatManager upnpManager; - @Before + @BeforeEach public void initialize() { mockedRegistry = mock(Registry.class); diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index 3b5c11431d1..81723ff1e42 100644 --- a/plugin-api/build.gradle +++ b/plugin-api/build.gradle @@ -69,7 +69,7 @@ Calculated : ${currentHash} tasks.register('checkAPIChanges', FileStateChecker) { description = "Checks that the API for the Plugin-API project does not change without deliberate thought" files = sourceSets.main.allJava.files - knownHash = 'gKRXd2Ow7wYKSgeGrDMRj0+2LdCzjOhLx8UEno9btGw=' + knownHash = 'nB1LhUpMWYFQpBdNJ/3Q79c8kLgUgPmEFzlRMlLUl1Y=' } check.dependsOn('checkAPIChanges') diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java index 2bbf04649ba..5c037983d79 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/data/TransactionSelectionResult.java @@ -24,7 +24,34 @@ */ public class TransactionSelectionResult { - private enum Status { + /** + * Represent the status of a transaction selection result. Plugin can extend this to implement its + * own statuses. + */ + protected interface Status { + /** + * Should the selection process be stopped? + * + * @return true if the selection process needs to be stopped + */ + boolean stop(); + + /** + * Should the current transaction be removed from the txpool? + * + * @return yes if the transaction should be removed from the txpool + */ + boolean discard(); + + /** + * Name of this status + * + * @return the name + */ + String name(); + } + + private enum BaseStatus implements Status { SELECTED, BLOCK_FULL(true, false), BLOCK_OCCUPANCY_ABOVE_THRESHOLD(true, false), @@ -36,12 +63,12 @@ private enum Status { private final boolean stop; private final boolean discard; - Status() { + BaseStatus() { this.stop = false; this.discard = false; } - Status(final boolean stop, final boolean discard) { + BaseStatus(final boolean stop, final boolean discard) { this.stop = stop; this.discard = discard; } @@ -50,26 +77,36 @@ private enum Status { public String toString() { return name() + " (stop=" + stop + ", discard=" + discard + ")"; } + + @Override + public boolean stop() { + return stop; + } + + @Override + public boolean discard() { + return discard; + } } /** The transaction has been selected to be included in the new block */ public static final TransactionSelectionResult SELECTED = - new TransactionSelectionResult(Status.SELECTED); + new TransactionSelectionResult(BaseStatus.SELECTED); /** The transaction has not been selected since the block is full. */ public static final TransactionSelectionResult BLOCK_FULL = - new TransactionSelectionResult(Status.BLOCK_FULL); + new TransactionSelectionResult(BaseStatus.BLOCK_FULL); /** There was no more time to add transaction to the block */ public static final TransactionSelectionResult BLOCK_SELECTION_TIMEOUT = - new TransactionSelectionResult(Status.BLOCK_SELECTION_TIMEOUT); + new TransactionSelectionResult(BaseStatus.BLOCK_SELECTION_TIMEOUT); /** Transaction took too much to evaluate */ public static final TransactionSelectionResult TX_EVALUATION_TOO_LONG = - new TransactionSelectionResult(Status.TX_EVALUATION_TOO_LONG); + new TransactionSelectionResult(BaseStatus.TX_EVALUATION_TOO_LONG); /** * The transaction has not been selected since too large and the occupancy of the block is enough * to stop the selection. */ public static final TransactionSelectionResult BLOCK_OCCUPANCY_ABOVE_THRESHOLD = - new TransactionSelectionResult(Status.BLOCK_OCCUPANCY_ABOVE_THRESHOLD); + new TransactionSelectionResult(BaseStatus.BLOCK_OCCUPANCY_ABOVE_THRESHOLD); /** * The transaction has not been selected since its gas limit is greater than the block remaining * gas, but the selection should continue. @@ -99,11 +136,22 @@ public String toString() { private final Status status; private final Optional maybeInvalidReason; - private TransactionSelectionResult(final Status status) { + /** + * Create a new transaction selection result with the passed status + * + * @param status the selection result status + */ + protected TransactionSelectionResult(final Status status) { this(status, null); } - private TransactionSelectionResult(final Status status, final String invalidReason) { + /** + * Create a new transaction selection result with the passed status and invalid reason + * + * @param status the selection result status + * @param invalidReason string with a custom invalid reason + */ + protected TransactionSelectionResult(final Status status, final String invalidReason) { this.status = status; this.maybeInvalidReason = Optional.ofNullable(invalidReason); } @@ -116,7 +164,7 @@ private TransactionSelectionResult(final Status status, final String invalidReas * @return the selection result */ public static TransactionSelectionResult invalidTransient(final String invalidReason) { - return new TransactionSelectionResult(Status.INVALID_TRANSIENT, invalidReason); + return new TransactionSelectionResult(BaseStatus.INVALID_TRANSIENT, invalidReason); } /** @@ -127,7 +175,7 @@ public static TransactionSelectionResult invalidTransient(final String invalidRe * @return the selection result */ public static TransactionSelectionResult invalid(final String invalidReason) { - return new TransactionSelectionResult(Status.INVALID, invalidReason); + return new TransactionSelectionResult(BaseStatus.INVALID, invalidReason); } /** @@ -136,7 +184,7 @@ public static TransactionSelectionResult invalid(final String invalidReason) { * @return true if the selection process should stop, false otherwise */ public boolean stop() { - return status.stop; + return status.stop(); } /** @@ -146,7 +194,7 @@ public boolean stop() { * otherwise */ public boolean discard() { - return status.discard; + return status.discard(); } /** @@ -155,7 +203,7 @@ public boolean discard() { * @return true if the candidate transaction is included in the new block, false otherwise */ public boolean selected() { - return Status.SELECTED.equals(status); + return BaseStatus.SELECTED.equals(status); } /**