From 3b246940719f5cfa8fbddf70542c078a63f8d1b1 Mon Sep 17 00:00:00 2001 From: Abdelhamid Bakhta <45264458+abdelhamidbakhta@users.noreply.github.com> Date: Fri, 15 May 2020 06:04:42 +0200 Subject: [PATCH] Check transaction types when replacing in the transaction pool. (#924) * Add checks on the replacement of a transaction in the pool: - reject EIP-1559 for pre-fork blocks - accept both frontier and EIP-1559 transactions during phase 1 - reject frontier transactions after phase 2 is finalized Signed-off-by: Abdelhamid Bakhta Signed-off-by: Danno Ferrin --- .../blockcreation/CliqueBlockCreatorTest.java | 13 ++++-- .../CliqueMinerExecutorTest.java | 10 +++-- .../ibft/support/TestContextBuilder.java | 3 +- .../blockcreation/IbftBlockCreatorTest.java | 3 +- .../blockcreation/IbftBlockCreatorTest.java | 3 +- .../EthGetFilterChangesIntegrationTest.java | 3 +- .../BlockTransactionSelectorTest.java | 3 +- .../EthHashBlockCreatorTest.java | 13 ++++-- .../EthHashMinerExecutorTest.java | 7 ++- .../besu/ethereum/core/fees/EIP1559.java | 13 ++++++ .../eth/transactions/PendingTransactions.java | 17 +++---- .../transactions/TransactionPoolFactory.java | 3 +- .../TransactionPoolReplacementHandler.java | 29 ++++++++---- .../transactions/PendingTransactionsTest.java | 21 +++++++-- ...TransactionPoolReplacementHandlerTest.java | 45 +++++++++---------- .../eth/transactions/TransactionPoolTest.java | 3 +- 16 files changed, 122 insertions(+), 67 deletions(-) diff --git a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java index be097176d50..564d25d7695 100644 --- a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java +++ b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreatorTest.java @@ -54,6 +54,7 @@ import org.hyperledger.besu.testutil.TestClock; import java.util.List; +import java.util.Optional; import com.google.common.collect.Lists; import org.apache.tuweni.bytes.Bytes; @@ -127,7 +128,8 @@ public void proposerAddressCanBeExtractFromAConstructedBlock() { 5, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader), + blockchain::getChainHeadHeader, + Optional.empty()), protocolContext, protocolSchedule, gasLimit -> gasLimit, @@ -161,7 +163,8 @@ public void insertsValidVoteIntoConstructedBlock() { 5, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader), + blockchain::getChainHeadHeader, + Optional.empty()), protocolContext, protocolSchedule, gasLimit -> gasLimit, @@ -194,7 +197,8 @@ public void insertsNoVoteWhenAuthInValidators() { 5, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader), + blockchain::getChainHeadHeader, + Optional.empty()), protocolContext, protocolSchedule, gasLimit -> gasLimit, @@ -230,7 +234,8 @@ public void insertsNoVoteWhenAtEpoch() { 5, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader), + blockchain::getChainHeadHeader, + Optional.empty()), protocolContext, protocolSchedule, gasLimit -> gasLimit, diff --git a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java index b6f4c224e4e..4b79cb50794 100644 --- a/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java +++ b/consensus/clique/src/test/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutorTest.java @@ -47,6 +47,7 @@ import org.hyperledger.besu.testutil.TestClock; import java.util.List; +import java.util.Optional; import java.util.Random; import java.util.function.Function; @@ -101,7 +102,8 @@ public void extraDataCreatedOnEpochBlocksContainsValidators() { 5, TestClock.fixed(), metricsSystem, - () -> null), + () -> null, + Optional.empty()), proposerNodeKey, new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, vanityData, false), mock(CliqueBlockScheduler.class), @@ -140,7 +142,8 @@ public void extraDataForNonEpochBlocksDoesNotContainValidaors() { 5, TestClock.fixed(), metricsSystem, - () -> null), + () -> null, + Optional.empty()), proposerNodeKey, new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, vanityData, false), mock(CliqueBlockScheduler.class), @@ -179,7 +182,8 @@ public void shouldUseLatestVanityData() { 5, TestClock.fixed(), metricsSystem, - () -> null), + () -> null, + Optional.empty()), proposerNodeKey, new MiningParameters(AddressHelpers.ofValue(1), Wei.ZERO, initialVanityData, false), mock(CliqueBlockScheduler.class), diff --git a/consensus/ibft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java b/consensus/ibft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java index 66fcd3b0b61..99584571a81 100644 --- a/consensus/ibft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java +++ b/consensus/ibft/src/integration-test/java/org/hyperledger/besu/consensus/ibft/support/TestContextBuilder.java @@ -303,7 +303,8 @@ private static ControllerAndState createControllerAndFinalState( 1, clock, metricsSystem, - blockChain::getChainHeadHeader); + blockChain::getChainHeadHeader, + Optional.empty()); final IbftBlockCreatorFactory blockCreatorFactory = new IbftBlockCreatorFactory( diff --git a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/IbftBlockCreatorTest.java b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/IbftBlockCreatorTest.java index 0541312812c..a7d28d8f2d9 100644 --- a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/IbftBlockCreatorTest.java +++ b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/IbftBlockCreatorTest.java @@ -90,7 +90,8 @@ public void createdBlockPassesValidationRulesAndHasAppropriateHashAndMixHash() { 5, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader); + blockchain::getChainHeadHeader, + Optional.empty()); final IbftBlockCreator blockCreator = new IbftBlockCreator( diff --git a/consensus/ibftlegacy/src/test/java/org/hyperledger/besu/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java b/consensus/ibftlegacy/src/test/java/org/hyperledger/besu/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java index 7e53b9fd5f2..d15157743e2 100644 --- a/consensus/ibftlegacy/src/test/java/org/hyperledger/besu/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java +++ b/consensus/ibftlegacy/src/test/java/org/hyperledger/besu/consensus/ibftlegacy/blockcreation/IbftBlockCreatorTest.java @@ -104,7 +104,8 @@ public void headerProducedPassesValidationRules() { 5, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader), + blockchain::getChainHeadHeader, + Optional.empty()), protContext, protocolSchedule, parentGasLimit -> parentGasLimit, diff --git a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java index 4f17b319aa0..7f1f022f258 100644 --- a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java +++ b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java @@ -101,7 +101,8 @@ public void setUp() { MAX_HASHES, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader); + blockchain::getChainHeadHeader, + Optional.empty()); final ProtocolContext protocolContext = executionContext.getProtocolContext(); PeerTransactionTracker peerTransactionTracker = mock(PeerTransactionTracker.class); diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelectorTest.java index 834a2a3802f..983b0592198 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelectorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/BlockTransactionSelectorTest.java @@ -77,7 +77,8 @@ public class BlockTransactionSelectorTest { 5, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader); + blockchain::getChainHeadHeader, + Optional.empty()); private final MutableWorldState worldState = InMemoryStorageProvider.createInMemoryWorldState(); private final Supplier isCancelled = () -> false; private final TransactionProcessor transactionProcessor = mock(TransactionProcessor.class); diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashBlockCreatorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashBlockCreatorTest.java index f535687e9ac..0bd67bb1966 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashBlockCreatorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashBlockCreatorTest.java @@ -41,6 +41,7 @@ import java.io.IOException; import java.math.BigInteger; import java.util.Collections; +import java.util.Optional; import java.util.function.Function; import com.google.common.collect.Lists; @@ -85,7 +86,8 @@ public void createMainnetBlock1() throws IOException { 5, TestClock.fixed(), metricsSystem, - executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader); + executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader, + Optional.empty()); final EthHashBlockCreator blockCreator = new EthHashBlockCreator( @@ -137,7 +139,8 @@ public void createMainnetBlock1_fixedDifficulty1() { 5, TestClock.fixed(), metricsSystem, - executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader); + executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader, + Optional.empty()); final EthHashBlockCreator blockCreator = new EthHashBlockCreator( @@ -184,7 +187,8 @@ public void rewardBeneficiary_zeroReward_skipZeroRewardsFalse() { 5, TestClock.fixed(), metricsSystem, - executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader); + executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader, + Optional.empty()); final EthHashBlockCreator blockCreator = new EthHashBlockCreator( @@ -247,7 +251,8 @@ public void rewardBeneficiary_zeroReward_skipZeroRewardsTrue() { 5, TestClock.fixed(), metricsSystem, - executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader); + executionContextTestFixture.getProtocolContext().getBlockchain()::getChainHeadHeader, + Optional.empty()); final EthHashBlockCreator blockCreator = new EthHashBlockCreator( diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java index 29219908734..636fe389ce6 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/EthHashMinerExecutorTest.java @@ -25,6 +25,7 @@ import org.hyperledger.besu.testutil.TestClock; import org.hyperledger.besu.util.Subscribers; +import java.util.Optional; import java.util.function.Function; import org.junit.Test; @@ -44,7 +45,8 @@ public void startingMiningWithoutCoinbaseThrowsException() { 5, TestClock.fixed(), metricsSystem, - () -> null); + () -> null, + Optional.empty()); final EthHashMinerExecutor executor = new EthHashMinerExecutor( @@ -71,7 +73,8 @@ public void settingCoinbaseToNullThrowsException() { 5, TestClock.fixed(), metricsSystem, - () -> null); + () -> null, + Optional.empty()); final EthHashMinerExecutor executor = new EthHashMinerExecutor( diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/fees/EIP1559.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/fees/EIP1559.java index 1e04d7825d7..133c42f9ee8 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/fees/EIP1559.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/fees/EIP1559.java @@ -15,6 +15,9 @@ package org.hyperledger.besu.ethereum.core.fees; import static java.lang.Math.floorDiv; +import static org.hyperledger.besu.ethereum.core.AcceptedTransactionTypes.FEE_MARKET_TRANSACTIONS; +import static org.hyperledger.besu.ethereum.core.AcceptedTransactionTypes.FEE_MARKET_TRANSITIONAL_TRANSACTIONS; +import static org.hyperledger.besu.ethereum.core.AcceptedTransactionTypes.FRONTIER_TRANSACTIONS; import org.hyperledger.besu.config.experimental.ExperimentalEIPs; import org.hyperledger.besu.ethereum.core.AcceptedTransactionTypes; @@ -117,6 +120,16 @@ public boolean isValidFormat( } } + public boolean isValidTransaction(final long blockNumber, final Transaction transaction) { + return isValidFormat( + transaction, + isEIP1559Finalized(blockNumber) + ? FEE_MARKET_TRANSACTIONS + : isEIP1559(blockNumber) + ? FEE_MARKET_TRANSITIONAL_TRANSACTIONS + : FRONTIER_TRANSACTIONS); + } + public boolean isValidGasLimit(final Transaction transaction) { if (transaction == null) { return false; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java index eb2bcf25c89..f2d49a5ab34 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java @@ -19,12 +19,12 @@ import static org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionAddedStatus.ALREADY_KNOWN; import static org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionAddedStatus.REJECTED_UNDERPRICED_REPLACEMENT; -import org.hyperledger.besu.config.experimental.ExperimentalEIPs; import org.hyperledger.besu.ethereum.core.AccountTransactionOrder; import org.hyperledger.besu.ethereum.core.Address; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.Hash; import org.hyperledger.besu.ethereum.core.Transaction; +import org.hyperledger.besu.ethereum.core.fees.EIP1559; import org.hyperledger.besu.ethereum.mainnet.TransactionValidator.TransactionInvalidReason; import org.hyperledger.besu.metrics.BesuMetricCategory; import org.hyperledger.besu.plugin.services.MetricsSystem; @@ -88,8 +88,7 @@ public class PendingTransactions { private final Counter localTransactionHashesAddedCounter; private final long maxPendingTransactions; - private final TransactionPoolReplacementHandler transactionReplacementHandler = - new TransactionPoolReplacementHandler(); + private final TransactionPoolReplacementHandler transactionReplacementHandler; private final Supplier chainHeadHeaderSupplier; public PendingTransactions( @@ -98,12 +97,14 @@ public PendingTransactions( final int maxPooledTransactionHashes, final Clock clock, final MetricsSystem metricsSystem, - final Supplier chainHeadHeaderSupplier) { + final Supplier chainHeadHeaderSupplier, + final Optional eip1559) { this.maxTransactionRetentionHours = maxTransactionRetentionHours; this.maxPendingTransactions = maxPendingTransactions; this.clock = clock; this.newPooledHashes = EvictingQueue.create(maxPooledTransactionHashes); this.chainHeadHeaderSupplier = chainHeadHeaderSupplier; + this.transactionReplacementHandler = new TransactionPoolReplacementHandler(eip1559); final LabelledMetric transactionAddedCounter = metricsSystem.createLabelledCounter( BesuMetricCategory.TRANSACTION_POOL, @@ -275,14 +276,8 @@ private TransactionAddedStatus addTransactionForSenderAndNonce( final TransactionInfo existingTransaction = getTrackedTransactionBySenderAndNonce(transactionInfo); if (existingTransaction != null) { - final Optional baseFee = - ExperimentalEIPs.eip1559Enabled - && (existingTransaction.getTransaction().isEIP1559Transaction() - || transactionInfo.getTransaction().isEIP1559Transaction()) - ? chainHeadHeaderSupplier.get().getBaseFee() - : Optional.empty(); if (!transactionReplacementHandler.shouldReplace( - existingTransaction, transactionInfo, baseFee)) { + existingTransaction, transactionInfo, chainHeadHeaderSupplier.get())) { return REJECTED_UNDERPRICED_REPLACEMENT; } removeTransaction(existingTransaction.getTransaction()); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java index e917ab744fd..341bfcab287 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolFactory.java @@ -51,7 +51,8 @@ public static TransactionPool createTransactionPool( transactionPoolConfiguration.getPooledTransactionHashesSize(), clock, metricsSystem, - protocolContext.getBlockchain()::getChainHeadHeader); + protocolContext.getBlockchain()::getChainHeadHeader, + eip1559); final PeerTransactionTracker transactionTracker = new PeerTransactionTracker(); final TransactionsMessageSender transactionsMessageSender = diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandler.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandler.java index c77300609e8..980cc4348ca 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandler.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandler.java @@ -16,6 +16,8 @@ import static java.util.Arrays.asList; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.fees.EIP1559; import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionInfo; import java.util.List; @@ -23,28 +25,39 @@ import com.google.common.annotations.VisibleForTesting; -public class TransactionPoolReplacementHandler implements TransactionPoolReplacementRule { +public class TransactionPoolReplacementHandler { private final List rules; + private final Optional eip1559; - public TransactionPoolReplacementHandler() { - this(asList(new TransactionReplacementByPriceRule())); + public TransactionPoolReplacementHandler(final Optional eip1559) { + this(asList(new TransactionReplacementByPriceRule()), eip1559); } @VisibleForTesting - TransactionPoolReplacementHandler(final List rules) { + TransactionPoolReplacementHandler( + final List rules, final Optional eip1559) { this.rules = rules; + this.eip1559 = eip1559; } - @Override public boolean shouldReplace( final TransactionInfo existingTransactionInfo, final TransactionInfo newTransactionInfo, - final Optional baseFee) { + final BlockHeader chainHeadHeader) { assert existingTransactionInfo != null; if (newTransactionInfo == null) { return false; } - return rules.stream() - .anyMatch(rule -> rule.shouldReplace(existingTransactionInfo, newTransactionInfo, baseFee)); + return eip1559 + .map( + eip -> + eip.isValidTransaction( + chainHeadHeader.getNumber(), newTransactionInfo.getTransaction())) + .orElse(newTransactionInfo.getTransaction().isFrontierTransaction()) + && rules.stream() + .anyMatch( + rule -> + rule.shouldReplace( + existingTransactionInfo, newTransactionInfo, chainHeadHeader.getBaseFee())); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java index 6375c81f6b6..25bd9fe0840 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java @@ -19,9 +19,11 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; import org.hyperledger.besu.crypto.SECP256K1.KeyPair; import org.hyperledger.besu.ethereum.core.Address; +import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.Hash; import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionTestFixture; @@ -34,6 +36,7 @@ import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.OptionalLong; import com.google.common.collect.Lists; @@ -60,7 +63,8 @@ public class PendingTransactionsTest { MAX_TRANSACTION_HASHES, TestClock.fixed(), metricsSystem, - () -> null); + PendingTransactionsTest::mockBlockHeader, + Optional.empty()); private final Transaction transaction1 = createTransaction(2); private final Transaction transaction2 = createTransaction(1); @@ -561,7 +565,8 @@ public void shouldEvictMultipleOldTransactions() { MAX_TRANSACTION_HASHES, clock, metricsSystem, - () -> null); + () -> null, + Optional.empty()); transactions.addRemoteTransaction(transaction1); assertThat(transactions.size()).isEqualTo(1); @@ -584,7 +589,8 @@ public void shouldEvictSingleOldTransaction() { MAX_TRANSACTION_HASHES, clock, metricsSystem, - () -> null); + () -> null, + Optional.empty()); transactions.addRemoteTransaction(transaction1); assertThat(transactions.size()).isEqualTo(1); clock.step(2L, ChronoUnit.HOURS); @@ -603,7 +609,8 @@ public void shouldEvictExclusivelyOldTransactions() { MAX_TRANSACTION_HASHES, clock, metricsSystem, - () -> null); + () -> null, + Optional.empty()); transactions.addRemoteTransaction(transaction1); assertThat(transactions.size()).isEqualTo(1); clock.step(3L, ChronoUnit.HOURS); @@ -663,4 +670,10 @@ private void addLocalTransactions(final int... nonces) { transactions.addLocalTransaction(createTransaction(nonce)); } } + + private static BlockHeader mockBlockHeader() { + final BlockHeader blockHeader = mock(BlockHeader.class); + when(blockHeader.getBaseFee()).thenReturn(Optional.empty()); + return blockHeader; + } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandlerTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandlerTest.java index 064694cf5c5..eb5e173e4e0 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandlerTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolReplacementHandlerTest.java @@ -17,10 +17,12 @@ import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; -import static java.util.Optional.empty; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import org.hyperledger.besu.ethereum.core.BlockHeader; +import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions.TransactionInfo; import java.util.Collection; @@ -39,26 +41,13 @@ public class TransactionPoolReplacementHandlerTest { public static Collection data() { return asList( new Object[][] { - {emptyList(), mock(TransactionInfo.class), mock(TransactionInfo.class), empty(), false}, - { - singletonList(constantRule(false)), - mock(TransactionInfo.class), - mock(TransactionInfo.class), - empty(), - false - }, - { - singletonList(constantRule(true)), - mock(TransactionInfo.class), - mock(TransactionInfo.class), - empty(), - true - }, + {emptyList(), mockTransactionInfo(), mockTransactionInfo(), false}, + {singletonList(constantRule(false)), mockTransactionInfo(), mockTransactionInfo(), false}, + {singletonList(constantRule(true)), mockTransactionInfo(), mockTransactionInfo(), true}, { constantRules(asList(false, false, false, true)), - mock(TransactionInfo.class), - mock(TransactionInfo.class), - empty(), + mockTransactionInfo(), + mockTransactionInfo(), true }, }); @@ -67,27 +56,27 @@ public static Collection data() { private final List rules; private final TransactionInfo oldTransactionInfo; private final TransactionInfo newTransactionInfo; - private final Optional baseFee; private final boolean expectedResult; + private final BlockHeader header; public TransactionPoolReplacementHandlerTest( final List rules, final TransactionInfo oldTransactionInfo, final TransactionInfo newTransactionInfo, - final Optional baseFee, final boolean expectedResult) { this.rules = rules; this.oldTransactionInfo = oldTransactionInfo; this.newTransactionInfo = newTransactionInfo; - this.baseFee = baseFee; this.expectedResult = expectedResult; + header = mock(BlockHeader.class); + when(header.getBaseFee()).thenReturn(Optional.empty()); } @Test public void shouldReplace() { assertThat( - new TransactionPoolReplacementHandler(rules) - .shouldReplace(oldTransactionInfo, newTransactionInfo, baseFee)) + new TransactionPoolReplacementHandler(rules, Optional.empty()) + .shouldReplace(oldTransactionInfo, newTransactionInfo, header)) .isEqualTo(expectedResult); } @@ -101,4 +90,12 @@ private static List constantRules( .map(TransactionPoolReplacementHandlerTest::constantRule) .collect(Collectors.toList()); } + + private static TransactionInfo mockTransactionInfo() { + final TransactionInfo transactionInfo = mock(TransactionInfo.class); + final Transaction transaction = mock(Transaction.class); + when(transaction.isFrontierTransaction()).thenReturn(true); + when(transactionInfo.getTransaction()).thenReturn(transaction); + return transactionInfo; + } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolTest.java index dc45f2f6adf..f1880b359c4 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolTest.java @@ -122,7 +122,8 @@ public void setUp() { MAX_TRANSACTION_HASHES, TestClock.fixed(), metricsSystem, - blockchain::getChainHeadHeader); + blockchain::getChainHeadHeader, + Optional.empty()); when(protocolSchedule.getByBlockNumber(anyLong())).thenReturn(protocolSpec); when(protocolSpec.getTransactionValidator()).thenReturn(transactionValidator); genesisBlockGasLimit = executionContext.getGenesis().getHeader().getGasLimit();