From 8507bb7181532cbed7e446ef3998eb477fccef6f Mon Sep 17 00:00:00 2001 From: ahamlat Date: Fri, 13 Oct 2023 18:40:29 +0200 Subject: [PATCH 01/12] Optimize Eth_feeHistory RPC method (#6011) * Add a cache to EthFeeHistory and improve rewards algorithm * cache the rewards by hash instead of block number * Add final on some fields Signed-off-by: Ameziane H --- ethereum/api/build.gradle | 1 + .../internal/methods/EthFeeHistory.java | 71 ++++++++++++++----- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/ethereum/api/build.gradle b/ethereum/api/build.gradle index d6c230ae018..ed395df8c5d 100644 --- a/ethereum/api/build.gradle +++ b/ethereum/api/build.gradle @@ -78,6 +78,7 @@ dependencies { implementation 'io.tmio:tuweni-bytes' implementation 'io.tmio:tuweni-units' implementation 'org.web3j:abi' + implementation 'com.github.ben-manes.caffeine:caffeine' annotationProcessor "org.immutables:value" implementation "org.immutables:value-annotations" diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java index d505568bd4e..240cdaeb655 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java @@ -16,6 +16,7 @@ import static java.util.stream.Collectors.toUnmodifiableList; +import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.api.jsonrpc.RpcMethod; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; @@ -36,25 +37,30 @@ import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket; import org.hyperledger.besu.ethereum.mainnet.feemarket.FeeMarket; -import java.util.AbstractMap; import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.stream.LongStream; import java.util.stream.Stream; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.collect.Streams; public class EthFeeHistory implements JsonRpcMethod { private final ProtocolSchedule protocolSchedule; private final Blockchain blockchain; + private final Cache> cache; + private static final int MAXIMUM_CACHE_SIZE = 100_000; + + record RewardCacheKey(Hash blockHash, List rewardPercentiles) {} public EthFeeHistory(final ProtocolSchedule protocolSchedule, final Blockchain blockchain) { this.protocolSchedule = protocolSchedule; this.blockchain = blockchain; + this.cache = Caffeine.newBuilder().maximumSize(MAXIMUM_CACHE_SIZE).build(); } @Override @@ -96,6 +102,7 @@ public JsonRpcResponse response(final JsonRpcRequestContext request) { final List blockHeaders = LongStream.range(oldestBlock, lastBlock) + .parallel() .mapToObj(blockchain::getBlockHeader) .flatMap(Optional::stream) .collect(toUnmodifiableList()); @@ -143,14 +150,31 @@ public JsonRpcResponse response(final JsonRpcRequestContext request) { final Optional>> maybeRewards = maybeRewardPercentiles.map( rewardPercentiles -> - LongStream.range(oldestBlock, lastBlock) - .mapToObj(blockchain::getBlockByNumber) - .flatMap(Optional::stream) + blockHeaders.stream() + .parallel() .map( - block -> - computeRewards( - rewardPercentiles.stream().sorted().collect(toUnmodifiableList()), - block)) + blockHeader -> { + final RewardCacheKey key = + new RewardCacheKey(blockHeader.getBlockHash(), rewardPercentiles); + return Optional.ofNullable(cache.getIfPresent(key)) + .or( + () -> { + Optional block = + blockchain.getBlockByHash(blockHeader.getBlockHash()); + return block.map( + b -> { + List rewards = + computeRewards( + rewardPercentiles.stream() + .sorted() + .collect(toUnmodifiableList()), + b); + cache.put(key, rewards); + return rewards; + }); + }); + }) + .flatMap(Optional::stream) .collect(toUnmodifiableList())); return new JsonRpcSuccessResponse( @@ -188,13 +212,21 @@ private List computeRewards(final List rewardPercentiles, final Blo : transactionReceipt.getCumulativeGasUsed() - transactionsGasUsed.get(transactionsGasUsed.size() - 1)); } - final List> transactionsAndGasUsedAscendingEffectiveGasFee = + + record TransactionInfo(Transaction transaction, Long gasUsed, Wei effectivePriorityFeePerGas) {} + + final List transactionsInfo = Streams.zip( - transactions.stream(), transactionsGasUsed.stream(), AbstractMap.SimpleEntry::new) - .sorted( - Comparator.comparing( - transactionAndGasUsed -> - transactionAndGasUsed.getKey().getEffectivePriorityFeePerGas(baseFee))) + transactions.stream(), + transactionsGasUsed.stream(), + (transaction, gasUsed) -> + new TransactionInfo( + transaction, gasUsed, transaction.getEffectivePriorityFeePerGas(baseFee))) + .collect(toUnmodifiableList()); + + final List transactionsAndGasUsedAscendingEffectiveGasFee = + transactionsInfo.stream() + .sorted(Comparator.comparing(TransactionInfo::effectivePriorityFeePerGas)) .collect(toUnmodifiableList()); // We need to weight the percentile of rewards by the gas used in the transaction. @@ -203,18 +235,21 @@ private List computeRewards(final List rewardPercentiles, final Blo final ArrayList rewards = new ArrayList<>(); int rewardPercentileIndex = 0; long gasUsed = 0; - for (final Map.Entry transactionAndGasUsed : + for (final TransactionInfo transactionAndGasUsed : transactionsAndGasUsedAscendingEffectiveGasFee) { - gasUsed += transactionAndGasUsed.getValue(); + gasUsed += transactionAndGasUsed.gasUsed(); while (rewardPercentileIndex < rewardPercentiles.size() && 100.0 * gasUsed / block.getHeader().getGasUsed() >= rewardPercentiles.get(rewardPercentileIndex)) { - rewards.add(transactionAndGasUsed.getKey().getEffectivePriorityFeePerGas(baseFee)); + rewards.add(transactionAndGasUsed.effectivePriorityFeePerGas); rewardPercentileIndex++; } } + // Put the computed rewards in the cache + cache.put(new RewardCacheKey(block.getHeader().getBlockHash(), rewardPercentiles), rewards); + return rewards; } } From e0b33166124051eed03a2023bbe9c282ae87ed50 Mon Sep 17 00:00:00 2001 From: ahamlat Date: Fri, 13 Oct 2023 19:13:50 +0200 Subject: [PATCH 02/12] Cache last blocks data (block headers, block bodies, transactions' receipts and total difficulty) (#6009) * Add a flag --cache-last-blocks to cache last n blocks, The default value is 0 Signed-off-by: Ameziane H --- CHANGELOG.md | 3 +- .../org/hyperledger/besu/cli/BesuCommand.java | 8 +- .../controller/BesuControllerBuilder.java | 16 ++- .../hyperledger/besu/cli/BesuCommandTest.java | 12 ++ .../besu/cli/CommandTestAbstract.java | 2 + .../src/test/resources/everything_config.toml | 3 +- .../ethereum/chain/DefaultBlockchain.java | 126 ++++++++++++++++-- .../ethereum/chain/DefaultBlockchainTest.java | 76 +++++++++++ 8 files changed, 233 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 359b1135ad4..ef334703528 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog -## Next Release +## Next release +- Cache last n blocks by using a new Besu flag --cache-last-blocks=n ### Breaking Changes diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index a1818d281f6..47463d2c997 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -1294,6 +1294,11 @@ static class PermissionsOptionGroup { "Specifies the maximum number of blocks to retrieve logs from via RPC. Must be >=0. 0 specifies no limit (default: ${DEFAULT-VALUE})") private final Long rpcMaxLogsRange = 5000L; + @CommandLine.Option( + names = {"--cache-last-blocks"}, + description = "Specifies the number of last blocks to cache (default: ${DEFAULT-VALUE})") + private final Integer numberOfblocksToCache = 0; + @Mixin private P2PTLSConfigOptions p2pTLSConfigOptions; @Mixin private PkiBlockCreationOptions pkiBlockCreationOptions; @@ -2290,7 +2295,8 @@ public BesuControllerBuilder getControllerBuilder() { .lowerBoundPeers(peersLowerBound) .maxRemotelyInitiatedPeers(maxRemoteInitiatedPeers) .randomPeerPriority(p2PDiscoveryOptionGroup.randomPeerPriority) - .chainPruningConfiguration(unstableChainPruningOptions.toDomainObject()); + .chainPruningConfiguration(unstableChainPruningOptions.toDomainObject()) + .cacheLastBlocks(numberOfblocksToCache); } @NotNull diff --git a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java index c297bafde47..802be1215b5 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java @@ -188,6 +188,8 @@ public abstract class BesuControllerBuilder implements MiningParameterOverrides private PluginTransactionValidatorFactory pluginTransactionValidatorFactory; + private int numberOfBlocksToCache = 0; + /** * Provide a BesuComponent which can be used to get other dependencies * @@ -505,6 +507,17 @@ public BesuControllerBuilder chainPruningConfiguration( return this; } + /** + * Chain pruning configuration besu controller builder. + * + * @param numberOfBlocksToCache the number of blocks to cache + * @return the besu controller builder + */ + public BesuControllerBuilder cacheLastBlocks(final Integer numberOfBlocksToCache) { + this.numberOfBlocksToCache = numberOfBlocksToCache; + return this; + } + /** * sets the networkConfiguration in the builder * @@ -592,7 +605,8 @@ public BesuController build() { blockchainStorage, metricsSystem, reorgLoggingThreshold, - dataDirectory.toString()); + dataDirectory.toString(), + numberOfBlocksToCache); final CachedMerkleTrieLoader cachedMerkleTrieLoader = besuComponent diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 553220c9431..af28e7ba7c2 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -5585,4 +5585,16 @@ public void snapsyncForHealingFeaturesShouldFailWhenHealingIsNotEnabled() { .contains( "--Xsnapsync-synchronizer-flat option can only be used when -Xsnapsync-synchronizer-flat-db-healing-enabled is true"); } + + @Test + public void cacheLastBlocksOptionShouldWork() { + int numberOfBlocksToCache = 512; + parseCommand("--cache-last-blocks", String.valueOf(numberOfBlocksToCache)); + verify(mockControllerBuilder).cacheLastBlocks(intArgumentCaptor.capture()); + verify(mockControllerBuilder).build(); + + assertThat(intArgumentCaptor.getValue()).isEqualTo(numberOfBlocksToCache); + assertThat(commandOutput.toString(UTF_8)).isEmpty(); + assertThat(commandErrorOutput.toString(UTF_8)).isEmpty(); + } } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 126052df0b9..1c991d64935 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -245,6 +245,8 @@ public void initMocks() throws Exception { .thenReturn(mockControllerBuilder); when(mockControllerBuilder.besuComponent(any(BesuComponent.class))) .thenReturn(mockControllerBuilder); + when(mockControllerBuilder.cacheLastBlocks(any())).thenReturn(mockControllerBuilder); + // doReturn used because of generic BesuController doReturn(mockController).when(mockControllerBuilder).build(); lenient().when(mockController.getProtocolManager()).thenReturn(mockEthProtocolManager); diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml index e88acec6dc9..185b443c941 100644 --- a/besu/src/test/resources/everything_config.toml +++ b/besu/src/test/resources/everything_config.toml @@ -88,6 +88,7 @@ rpc-http-max-batch-size=1 rpc-http-max-request-content-length = 5242880 rpc-max-logs-range=100 json-pretty-print-enabled=false +cache-last-blocks=512 # PRIVACY TLS privacy-tls-enabled=false @@ -226,4 +227,4 @@ Xp2p-tls-crl-file="none.file" Xp2p-tls-clienthello-sni=false #contracts -Xevm-jumpdest-cache-weight-kb=32000 +Xevm-jumpdest-cache-weight-kb=32000 \ No newline at end of file diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java index 22c60862450..cbde737684b 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchain.java @@ -31,6 +31,7 @@ import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionReceipt; import org.hyperledger.besu.metrics.BesuMetricCategory; +import org.hyperledger.besu.metrics.prometheus.PrometheusMetricsSystem; import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.util.InvalidConfigurationException; import org.hyperledger.besu.util.Subscribers; @@ -49,8 +50,11 @@ import java.util.stream.Stream; import com.google.common.annotations.VisibleForTesting; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import com.google.common.collect.Lists; import com.google.common.collect.Streams; +import io.prometheus.client.guava.cache.CacheMetricsCollector; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,12 +77,18 @@ public class DefaultBlockchain implements MutableBlockchain { private Comparator blockChoiceRule; + private final int numberOfBlocksToCache; + private final Optional> blockHeadersCache; + private final Optional> blockBodiesCache; + private final Optional>> transactionReceiptsCache; + private final Optional> totalDifficultyCache; + private DefaultBlockchain( final Optional genesisBlock, final BlockchainStorage blockchainStorage, final MetricsSystem metricsSystem, final long reorgLoggingThreshold) { - this(genesisBlock, blockchainStorage, metricsSystem, reorgLoggingThreshold, null); + this(genesisBlock, blockchainStorage, metricsSystem, reorgLoggingThreshold, null, 0); } private DefaultBlockchain( @@ -86,7 +96,8 @@ private DefaultBlockchain( final BlockchainStorage blockchainStorage, final MetricsSystem metricsSystem, final long reorgLoggingThreshold, - final String dataDirectory) { + final String dataDirectory, + final int numberOfBlocksToCache) { checkNotNull(genesisBlock); checkNotNull(blockchainStorage); checkNotNull(metricsSystem); @@ -144,6 +155,34 @@ private DefaultBlockchain( this.reorgLoggingThreshold = reorgLoggingThreshold; this.blockChoiceRule = heaviestChainBlockChoiceRule; + this.numberOfBlocksToCache = numberOfBlocksToCache; + + if (numberOfBlocksToCache != 0) { + blockHeadersCache = + Optional.of( + CacheBuilder.newBuilder().recordStats().maximumSize(numberOfBlocksToCache).build()); + blockBodiesCache = + Optional.of( + CacheBuilder.newBuilder().recordStats().maximumSize(numberOfBlocksToCache).build()); + transactionReceiptsCache = + Optional.of( + CacheBuilder.newBuilder().recordStats().maximumSize(numberOfBlocksToCache).build()); + totalDifficultyCache = + Optional.of( + CacheBuilder.newBuilder().recordStats().maximumSize(numberOfBlocksToCache).build()); + CacheMetricsCollector cacheMetrics = new CacheMetricsCollector(); + cacheMetrics.addCache("blockHeaders", blockHeadersCache.get()); + cacheMetrics.addCache("blockBodies", blockBodiesCache.get()); + cacheMetrics.addCache("transactionReceipts", transactionReceiptsCache.get()); + cacheMetrics.addCache("totalDifficulty", totalDifficultyCache.get()); + if (metricsSystem instanceof PrometheusMetricsSystem prometheusMetricsSystem) + prometheusMetricsSystem.addCollector(BesuMetricCategory.BLOCKCHAIN, () -> cacheMetrics); + } else { + blockHeadersCache = Optional.empty(); + blockBodiesCache = Optional.empty(); + transactionReceiptsCache = Optional.empty(); + totalDifficultyCache = Optional.empty(); + } } public static MutableBlockchain createMutable( @@ -153,7 +192,12 @@ public static MutableBlockchain createMutable( final long reorgLoggingThreshold) { checkNotNull(genesisBlock); return new DefaultBlockchain( - Optional.of(genesisBlock), blockchainStorage, metricsSystem, reorgLoggingThreshold); + Optional.of(genesisBlock), + blockchainStorage, + metricsSystem, + reorgLoggingThreshold, + null, + 0); } public static MutableBlockchain createMutable( @@ -168,7 +212,25 @@ public static MutableBlockchain createMutable( blockchainStorage, metricsSystem, reorgLoggingThreshold, - dataDirectory); + dataDirectory, + 0); + } + + public static MutableBlockchain createMutable( + final Block genesisBlock, + final BlockchainStorage blockchainStorage, + final MetricsSystem metricsSystem, + final long reorgLoggingThreshold, + final String dataDirectory, + final int numberOfBlocksToCache) { + checkNotNull(genesisBlock); + return new DefaultBlockchain( + Optional.of(genesisBlock), + blockchainStorage, + metricsSystem, + reorgLoggingThreshold, + dataDirectory, + numberOfBlocksToCache); } public static Blockchain create( @@ -227,22 +289,37 @@ public Block getChainHeadBlock() { @Override public Optional getBlockHeader(final long blockNumber) { - return blockchainStorage.getBlockHash(blockNumber).flatMap(blockchainStorage::getBlockHeader); + return blockchainStorage.getBlockHash(blockNumber).flatMap(this::getBlockHeader); } @Override public Optional getBlockHeader(final Hash blockHeaderHash) { - return blockchainStorage.getBlockHeader(blockHeaderHash); + return blockHeadersCache + .map( + cache -> + Optional.ofNullable(cache.getIfPresent(blockHeaderHash)) + .or(() -> blockchainStorage.getBlockHeader(blockHeaderHash))) + .orElseGet(() -> blockchainStorage.getBlockHeader(blockHeaderHash)); } @Override public Optional getBlockBody(final Hash blockHeaderHash) { - return blockchainStorage.getBlockBody(blockHeaderHash); + return blockBodiesCache + .map( + cache -> + Optional.ofNullable(cache.getIfPresent(blockHeaderHash)) + .or(() -> blockchainStorage.getBlockBody(blockHeaderHash))) + .orElseGet(() -> blockchainStorage.getBlockBody(blockHeaderHash)); } @Override public Optional> getTxReceipts(final Hash blockHeaderHash) { - return blockchainStorage.getTransactionReceipts(blockHeaderHash); + return transactionReceiptsCache + .map( + cache -> + Optional.ofNullable(cache.getIfPresent(blockHeaderHash)) + .or(() -> blockchainStorage.getTransactionReceipts(blockHeaderHash))) + .orElseGet(() -> blockchainStorage.getTransactionReceipts(blockHeaderHash)); } @Override @@ -252,7 +329,12 @@ public Optional getBlockHashByNumber(final long number) { @Override public Optional getTotalDifficultyByHash(final Hash blockHeaderHash) { - return blockchainStorage.getTotalDifficulty(blockHeaderHash); + return totalDifficultyCache + .map( + cache -> + Optional.ofNullable(cache.getIfPresent(blockHeaderHash)) + .or(() -> blockchainStorage.getTotalDifficulty(blockHeaderHash))) + .orElseGet(() -> blockchainStorage.getTotalDifficulty(blockHeaderHash)); } @Override @@ -283,14 +365,24 @@ public void setBlockChoiceRule(final Comparator blockChoiceRule) { @Override public synchronized void appendBlock(final Block block, final List receipts) { + if (numberOfBlocksToCache != 0) cacheBlockData(block, receipts); appendBlockHelper(new BlockWithReceipts(block, receipts), false); } @Override public synchronized void storeBlock(final Block block, final List receipts) { + if (numberOfBlocksToCache != 0) cacheBlockData(block, receipts); appendBlockHelper(new BlockWithReceipts(block, receipts), true); } + private void cacheBlockData(final Block block, final List receipts) { + blockHeadersCache.ifPresent(cache -> cache.put(block.getHash(), block.getHeader())); + blockBodiesCache.ifPresent(cache -> cache.put(block.getHash(), block.getBody())); + transactionReceiptsCache.ifPresent(cache -> cache.put(block.getHash(), receipts)); + totalDifficultyCache.ifPresent( + cache -> cache.put(block.getHash(), block.getHeader().getDifficulty())); + } + private boolean blockShouldBeProcessed( final Block block, final List receipts) { checkArgument( @@ -768,4 +860,20 @@ int observerCount() { private void notifyChainReorgBlockAdded(final BlockWithReceipts blockWithReceipts) { blockReorgObservers.forEach(observer -> observer.onBlockAdded(blockWithReceipts, this)); } + + public Optional> getBlockHeadersCache() { + return blockHeadersCache; + } + + public Optional> getBlockBodiesCache() { + return blockBodiesCache; + } + + public Optional>> getTransactionReceiptsCache() { + return transactionReceiptsCache; + } + + public Optional> getTotalDifficultyCache() { + return totalDifficultyCache; + } } diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchainTest.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchainTest.java index 80c8eee5d37..66c9ac5593b 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchainTest.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/chain/DefaultBlockchainTest.java @@ -942,6 +942,66 @@ public void blockAddedObserver_invokedMultiple() { assertThat(observer3Invoked.get()).isTrue(); } + @Test + public void testCacheEmptyWhenNumberOfBlocksToCacheIsZero() { + final BlockDataGenerator gen = new BlockDataGenerator(); + final KeyValueStorage kvStore = new InMemoryKeyValueStorage(); + final KeyValueStorage kvStoreVariables = new InMemoryKeyValueStorage(); + final Block genesisBlock = gen.genesisBlock(); + final DefaultBlockchain blockchain = + createMutableBlockchain(kvStore, kvStoreVariables, genesisBlock); + + assertThat(blockchain.getBlockHeadersCache()).isEmpty(); + assertThat(blockchain.getBlockBodiesCache()).isEmpty(); + assertThat(blockchain.getTransactionReceiptsCache()).isEmpty(); + assertThat(blockchain.getTotalDifficultyCache()).isEmpty(); + } + + @Test + public void testCacheUsedWhenNumberOfBlocksToCacheNotZero() { + final BlockDataGenerator gen = new BlockDataGenerator(); + final KeyValueStorage kvStore = new InMemoryKeyValueStorage(); + final KeyValueStorage kvStoreVariables = new InMemoryKeyValueStorage(); + final Block genesisBlock = gen.genesisBlock(); + final DefaultBlockchain blockchain = + createMutableBlockchain(kvStore, kvStoreVariables, genesisBlock, "/data/test", 512); + + final BlockDataGenerator.BlockOptions options = + new BlockDataGenerator.BlockOptions() + .setBlockNumber(1L) + .setParentHash(genesisBlock.getHash()); + final Block newBlock = gen.block(options); + final List receipts = gen.receipts(newBlock); + + assertThat(blockchain.getBlockHeadersCache()).isNotEmpty(); + assertThat(blockchain.getBlockBodiesCache()).isNotEmpty(); + assertThat(blockchain.getTransactionReceiptsCache()).isNotEmpty(); + assertThat(blockchain.getTotalDifficultyCache()).isNotEmpty(); + + assertThat(blockchain.getBlockHeadersCache().get().size()).isEqualTo(0); + assertThat(blockchain.getBlockBodiesCache().get().size()).isEqualTo(0); + assertThat(blockchain.getTransactionReceiptsCache().get().size()).isEqualTo(0); + assertThat(blockchain.getTotalDifficultyCache().get().size()).isEqualTo(0); + + blockchain.appendBlock(newBlock, receipts); + + assertThat(blockchain.getBlockHeadersCache().get().size()).isEqualTo(1); + assertThat(blockchain.getBlockHeadersCache().get().getIfPresent(newBlock.getHash())) + .isEqualTo(newBlock.getHeader()); + + assertThat(blockchain.getBlockBodiesCache().get().size()).isEqualTo(1); + assertThat(blockchain.getBlockBodiesCache().get().getIfPresent(newBlock.getHash())) + .isEqualTo(newBlock.getBody()); + + assertThat(blockchain.getTransactionReceiptsCache().get().size()).isEqualTo(1); + assertThat(blockchain.getTransactionReceiptsCache().get().getIfPresent(newBlock.getHash())) + .isEqualTo(receipts); + + assertThat(blockchain.getTotalDifficultyCache().get().size()).isEqualTo(1); + assertThat(blockchain.getTotalDifficultyCache().get().getIfPresent(newBlock.getHash())) + .isEqualTo(newBlock.getHeader().getDifficulty()); + } + /* * Check that block header, block body, block number, transaction locations, and receipts for this * block are all stored. @@ -1026,4 +1086,20 @@ private Blockchain createBlockchain( return DefaultBlockchain.create( createStorage(kvStore, kvStorageVariables), new NoOpMetricsSystem(), 0); } + + private DefaultBlockchain createMutableBlockchain( + final KeyValueStorage kvStore, + final KeyValueStorage kvStorageVariables, + final Block genesisBlock, + final String dataDirectory, + final int numberOfBlocksToCache) { + return (DefaultBlockchain) + DefaultBlockchain.createMutable( + genesisBlock, + createStorage(kvStore, kvStorageVariables), + new NoOpMetricsSystem(), + 0, + dataDirectory, + numberOfBlocksToCache); + } } From de2d3b4156a84701ba870d82f3dc475f51d769fa Mon Sep 17 00:00:00 2001 From: ahamlat Date: Tue, 17 Oct 2023 03:52:11 +0200 Subject: [PATCH 03/12] Sort only once rewardPercentiles instead of doing it for each block header (#6035) Signed-off-by: Ameziane H --- CHANGELOG.md | 1 + .../internal/methods/EthFeeHistory.java | 51 +++++++++---------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef334703528..65bd5a0d03b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Next release - Cache last n blocks by using a new Besu flag --cache-last-blocks=n +- Optimize performances of RPC method Eth_feeHistory ### Breaking Changes diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java index 240cdaeb655..ffe3b96b05a 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/EthFeeHistory.java @@ -149,33 +149,30 @@ public JsonRpcResponse response(final JsonRpcRequestContext request) { final Optional>> maybeRewards = maybeRewardPercentiles.map( - rewardPercentiles -> - blockHeaders.stream() - .parallel() - .map( - blockHeader -> { - final RewardCacheKey key = - new RewardCacheKey(blockHeader.getBlockHash(), rewardPercentiles); - return Optional.ofNullable(cache.getIfPresent(key)) - .or( - () -> { - Optional block = - blockchain.getBlockByHash(blockHeader.getBlockHash()); - return block.map( - b -> { - List rewards = - computeRewards( - rewardPercentiles.stream() - .sorted() - .collect(toUnmodifiableList()), - b); - cache.put(key, rewards); - return rewards; - }); - }); - }) - .flatMap(Optional::stream) - .collect(toUnmodifiableList())); + rewardPercentiles -> { + var sortedPercentiles = rewardPercentiles.stream().sorted().toList(); + return blockHeaders.stream() + .parallel() + .map( + blockHeader -> { + final RewardCacheKey key = + new RewardCacheKey(blockHeader.getBlockHash(), rewardPercentiles); + return Optional.ofNullable(cache.getIfPresent(key)) + .or( + () -> { + Optional block = + blockchain.getBlockByHash(blockHeader.getBlockHash()); + return block.map( + b -> { + List rewards = computeRewards(sortedPercentiles, b); + cache.put(key, rewards); + return rewards; + }); + }); + }) + .flatMap(Optional::stream) + .toList(); + }); return new JsonRpcSuccessResponse( requestId, From 0dc37ad81e930e393bdd4076ce066a3b6b6c918d Mon Sep 17 00:00:00 2001 From: Sally MacFarlane Date: Tue, 17 Oct 2023 12:24:05 +1000 Subject: [PATCH 04/12] link to issue 5772 (#6038) Signed-off-by: Sally MacFarlane --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 65bd5a0d03b..eda3c083b5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,14 +37,14 @@ By default, the txpool is tuned for mainnet usage, but if you are using private ### Breaking Changes - Removed support for Kotti network (ETC) [#5816](https://github.com/hyperledger/besu/pull/5816) -- Layered transaction pool implementation is now stable and enabled by default, so the following changes to experimental options have been done [#5772](https://github.com/hyperledger/besu): +- Layered transaction pool implementation is now stable and enabled by default, so the following changes to experimental options have been done [#5772](https://github.com/hyperledger/besu/pull/5772): - `--Xlayered-tx-pool` is gone, to select the implementation use the new `--tx-pool` option with values `layered` (default) or `legacy` - `--Xlayered-tx-pool-layer-max-capacity`, `--Xlayered-tx-pool-max-prioritized` and `--Xlayered-tx-pool-max-future-by-sender` just drop the `X` and keep the same behavior ### Additions and Improvements - Add access to an immutable world view to start/end transaction hooks in the tracing API[#5836](https://github.com/hyperledger/besu/pull/5836) - Layered transaction pool implementation is now stable and enabled by default. If you want still to use the legacy implementation, use `--tx-pool=legacy`. - By default, the new transaction pool is capped at using 25MB of memory, this limit can be raised using `--layered-tx-pool-layer-max-capacity` options [#5772](https://github.com/hyperledger/besu) + By default, the new transaction pool is capped at using 25MB of memory, this limit can be raised using `--layered-tx-pool-layer-max-capacity` options [#5772](https://github.com/hyperledger/besu/pull/5772) - Tune G1GC to reduce Besu memory footprint, and new `besu-untuned` start scripts to run without any specific G1GC flags [#5879](https://github.com/hyperledger/besu/pull/5879) - Reduce `engine_forkchoiceUpdatedV?` response time by asynchronously process block added events in the transaction pool [#5909](https://github.com/hyperledger/besu/pull/5909) From f0b2b560640bbd1d51481c2df22c173e0dfbe46a Mon Sep 17 00:00:00 2001 From: seongmin Date: Tue, 17 Oct 2023 12:03:47 +0900 Subject: [PATCH 05/12] fix incorrect argument passing in blockParameter of TraceCallMany class (#6034) Signed-off-by: seongmin --- .../ethereum/api/jsonrpc/internal/methods/TraceCallMany.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceCallMany.java b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceCallMany.java index e3dfb245bb8..4ee9ff04e33 100644 --- a/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceCallMany.java +++ b/ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/TraceCallMany.java @@ -64,7 +64,7 @@ public String getName() { @Override protected BlockParameter blockParameter(final JsonRpcRequestContext request) { final Optional maybeBlockParameter = - request.getOptionalParameter(2, BlockParameter.class); + request.getOptionalParameter(1, BlockParameter.class); if (maybeBlockParameter.isPresent()) { return maybeBlockParameter.get(); From 7d6fdced220be483093096bb22b5d6559b35871b Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 17 Oct 2023 12:38:27 +0200 Subject: [PATCH 06/12] Fix 23.10.0 Breaking Changes changelog entry (#6040) Signed-off-by: Fabio Di Fabio --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eda3c083b5d..f17f3c116c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,7 @@ By default, the txpool is tuned for mainnet usage, but if you are using private - Removed support for Kotti network (ETC) [#5816](https://github.com/hyperledger/besu/pull/5816) - Layered transaction pool implementation is now stable and enabled by default, so the following changes to experimental options have been done [#5772](https://github.com/hyperledger/besu/pull/5772): - `--Xlayered-tx-pool` is gone, to select the implementation use the new `--tx-pool` option with values `layered` (default) or `legacy` - - `--Xlayered-tx-pool-layer-max-capacity`, `--Xlayered-tx-pool-max-prioritized` and `--Xlayered-tx-pool-max-future-by-sender` just drop the `X` and keep the same behavior + - `--Xlayered-tx-pool-layer-max-capacity`, `--Xlayered-tx-pool-max-prioritized` and `--Xlayered-tx-pool-max-future-by-sender` just drop the `Xlayered-` and keep the same behavior ### Additions and Improvements - Add access to an immutable world view to start/end transaction hooks in the tracing API[#5836](https://github.com/hyperledger/besu/pull/5836) From 6d100f30b71af92e32ba00170dcdc7647a4d7ebc Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 17 Oct 2023 13:19:09 +0200 Subject: [PATCH 07/12] Priority senders (#5959) Signed-off-by: Fabio Di Fabio Co-authored-by: Sally MacFarlane --- CHANGELOG.md | 7 +- .../stable/TransactionPoolOptions.java | 28 +- .../stable/TransactionPoolOptionsTest.java | 62 +++- .../src/test/resources/everything_config.toml | 3 +- .../blockcreation/MergeCoordinatorTest.java | 46 +-- .../besu/datatypes/PendingTransaction.java | 7 + .../EthGetFilterChangesIntegrationTest.java | 11 +- .../EthGetFilterChangesIntegrationTest.java | 11 +- .../txselection/BlockTransactionSelector.java | 2 +- .../selectors/PriceTransactionSelector.java | 5 +- ...FeeMarketBlockTransactionSelectorTest.java | 2 +- .../DisabledPendingTransactions.java | 14 +- .../eth/transactions/PendingTransaction.java | 84 +++++ .../eth/transactions/PendingTransactions.java | 9 +- .../eth/transactions/TransactionPool.java | 165 +++++----- .../TransactionPoolConfiguration.java | 16 +- .../transactions/TransactionPoolMetrics.java | 43 ++- .../layered/AbstractTransactionsLayer.java | 26 +- .../BaseFeePrioritizedTransactions.java | 3 +- .../eth/transactions/layered/EndLayer.java | 8 +- .../GasPricePrioritizedTransactions.java | 2 +- .../layered/LayeredPendingTransactions.java | 25 +- .../layered/ReadyTransactions.java | 3 +- .../layered/SparseTransactions.java | 67 +++- .../layered/TransactionsLayer.java | 2 + .../AbstractPendingTransactionsSorter.java | 51 ++- .../BaseFeePendingTransactionsSorter.java | 4 +- .../GasPricePendingTransactionsSorter.java | 2 +- .../AbstractTransactionPoolTest.java | 302 +++++++++++------- .../layered/BaseTransactionPoolTest.java | 18 +- .../LayeredPendingTransactionsTest.java | 271 ++++++++++------ .../eth/transactions/layered/LayersTest.java | 160 +++++++++- .../eth/transactions/layered/ReplayTest.java | 34 +- .../AbstractPendingTransactionsTestBase.java | 225 ++++++++----- 34 files changed, 1172 insertions(+), 546 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f17f3c116c0..c849de7c608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,11 @@ ### Breaking Changes +### Deprecations +- `--tx-pool-disable-locals` has been deprecated for removal in favor of `--tx-pool-no-local-priority`, no semantic change, only a renaming [#5959](https://github.com/hyperledger/besu/pull/5959) + ### Additions and Improvements +- New option `--tx-pool-priority-senders` to specify a list of senders, that has the effect to prioritize any transactions sent by these senders from any source [#5959](https://github.com/hyperledger/besu/pull/5959) ### Bug Fixes @@ -46,7 +50,7 @@ By default, the txpool is tuned for mainnet usage, but if you are using private - Layered transaction pool implementation is now stable and enabled by default. If you want still to use the legacy implementation, use `--tx-pool=legacy`. By default, the new transaction pool is capped at using 25MB of memory, this limit can be raised using `--layered-tx-pool-layer-max-capacity` options [#5772](https://github.com/hyperledger/besu/pull/5772) - Tune G1GC to reduce Besu memory footprint, and new `besu-untuned` start scripts to run without any specific G1GC flags [#5879](https://github.com/hyperledger/besu/pull/5879) -- Reduce `engine_forkchoiceUpdatedV?` response time by asynchronously process block added events in the transaction pool [#5909](https://github.com/hyperledger/besu/pull/5909) +- Reduce `engine_forkchoiceUpdatedV?` response time by asynchronously process block added events in the transaction pool [#5909](https://github.com/hyperledger/besu/pull/5909) ### Bug Fixes - do not create ignorable storage on revert storage-variables subcommand [#5830](https://github.com/hyperledger/besu/pull/5830) @@ -55,7 +59,6 @@ By default, the txpool is tuned for mainnet usage, but if you are using private ### Download Links https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/23.10.0/besu-23.10.0.tar.gz / sha256: 3c75f3792bfdb0892705b378f0b8bfc14ef6cecf1d8afe711d8d8687ed6687cf - https://hyperledger.jfrog.io/artifactory/besu-binaries/besu/23.10.0/besu-23.10.0.zip / sha256: d5dafff4c3cbf104bf75b34a9f108dcdd7b08d2759de75ec65cd997f38f52866 ## 23.7.3 diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java index 6353618d24d..d481e0453a0 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptions.java @@ -24,6 +24,7 @@ import org.hyperledger.besu.cli.converter.PercentageConverter; import org.hyperledger.besu.cli.options.CLIOptions; import org.hyperledger.besu.cli.util.CommandLineUtils; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; @@ -32,19 +33,25 @@ import java.io.File; import java.util.List; +import java.util.Set; import picocli.CommandLine; /** The Transaction pool Cli stable options. */ public class TransactionPoolOptions implements CLIOptions { private static final String TX_POOL_IMPLEMENTATION = "--tx-pool"; + /** Use TX_POOL_NO_LOCAL_PRIORITY instead */ + @Deprecated(forRemoval = true) private static final String TX_POOL_DISABLE_LOCALS = "--tx-pool-disable-locals"; + + private static final String TX_POOL_NO_LOCAL_PRIORITY = "--tx-pool-no-local-priority"; private static final String TX_POOL_ENABLE_SAVE_RESTORE = "--tx-pool-enable-save-restore"; private static final String TX_POOL_SAVE_FILE = "--tx-pool-save-file"; private static final String TX_POOL_PRICE_BUMP = "--tx-pool-price-bump"; private static final String RPC_TX_FEECAP = "--rpc-tx-feecap"; private static final String STRICT_TX_REPLAY_PROTECTION_ENABLED_FLAG = "--strict-tx-replay-protection-enabled"; + private static final String TX_POOL_PRIORITY_SENDERS = "--tx-pool-priority-senders"; @CommandLine.Option( names = {TX_POOL_IMPLEMENTATION}, @@ -54,13 +61,13 @@ public class TransactionPoolOptions implements CLIOptions prioritySenders = TransactionPoolConfiguration.DEFAULT_PRIORITY_SENDERS; + @CommandLine.ArgGroup( validate = false, heading = "@|bold Tx Pool Layered Implementation Options|@%n") @@ -200,11 +216,12 @@ public static TransactionPoolOptions fromConfig(final TransactionPoolConfigurati final TransactionPoolOptions options = TransactionPoolOptions.create(); options.txPoolImplementation = config.getTxPoolImplementation(); options.saveRestoreEnabled = config.getEnableSaveRestore(); - options.disableLocalTxs = config.getDisableLocalTransactions(); + options.noLocalPriority = config.getNoLocalPriority(); options.priceBump = config.getPriceBump(); options.txFeeCap = config.getTxFeeCap(); options.saveFile = config.getSaveFile(); options.strictTxReplayProtectionEnabled = config.getStrictTransactionReplayProtectionEnabled(); + options.prioritySenders = config.getPrioritySenders(); options.layeredOptions.txPoolLayerMaxCapacity = config.getPendingTransactionsLayerMaxCapacityBytes(); options.layeredOptions.txPoolMaxPrioritized = config.getMaxPrioritizedTransactions(); @@ -242,11 +259,12 @@ public TransactionPoolConfiguration toDomainObject() { return ImmutableTransactionPoolConfiguration.builder() .txPoolImplementation(txPoolImplementation) .enableSaveRestore(saveRestoreEnabled) - .disableLocalTransactions(disableLocalTxs) + .noLocalPriority(noLocalPriority) .priceBump(priceBump) .txFeeCap(txFeeCap) .saveFile(saveFile) .strictTransactionReplayProtectionEnabled(strictTxReplayProtectionEnabled) + .prioritySenders(prioritySenders) .pendingTransactionsLayerMaxCapacityBytes(layeredOptions.txPoolLayerMaxCapacity) .maxPrioritizedTransactions(layeredOptions.txPoolMaxPrioritized) .maxFutureBySender(layeredOptions.txPoolMaxFutureBySender) diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptionsTest.java index 1f0705e9948..e7977a921e1 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptionsTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/options/stable/TransactionPoolOptionsTest.java @@ -20,6 +20,7 @@ import org.hyperledger.besu.cli.options.AbstractCLIOptionsTest; import org.hyperledger.besu.cli.options.OptionParser; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; @@ -73,20 +74,20 @@ public void pendingTransactionRetentionPeriod() { @Test public void disableLocalsDefault() { - internalTestSuccess(config -> assertThat(config.getDisableLocalTransactions()).isFalse()); + internalTestSuccess(config -> assertThat(config.getNoLocalPriority()).isFalse()); } @Test public void disableLocalsOn() { internalTestSuccess( - config -> assertThat(config.getDisableLocalTransactions()).isTrue(), + config -> assertThat(config.getNoLocalPriority()).isTrue(), "--tx-pool-disable-locals=true"); } @Test public void disableLocalsOff() { internalTestSuccess( - config -> assertThat(config.getDisableLocalTransactions()).isFalse(), + config -> assertThat(config.getNoLocalPriority()).isFalse(), "--tx-pool-disable-locals=false"); } @@ -214,6 +215,61 @@ public void failIfLayeredOptionsWhenLegacySelectedByArg() { "--tx-pool-max-prioritized=1000"); } + @Test + public void byDefaultNoPrioritySenders() { + internalTestSuccess(config -> assertThat(config.getPrioritySenders()).isEmpty()); + } + + @Test + public void onePrioritySenderWorks() { + final Address prioritySender = Address.fromHexString("0xABC123"); + internalTestSuccess( + config -> assertThat(config.getPrioritySenders()).containsExactly(prioritySender), + "--tx-pool-priority-senders", + prioritySender.toHexString()); + } + + @Test + public void morePrioritySendersWorks() { + final Address prioritySender1 = Address.fromHexString("0xABC123"); + final Address prioritySender2 = Address.fromHexString("0xDEF456"); + final Address prioritySender3 = Address.fromHexString("0x789000"); + internalTestSuccess( + config -> + assertThat(config.getPrioritySenders()) + .containsExactly(prioritySender1, prioritySender2, prioritySender3), + "--tx-pool-priority-senders", + prioritySender1.toHexString() + + "," + + prioritySender2.toHexString() + + "," + + prioritySender3.toHexString()); + } + + @Test + public void atLeastOnePrioritySenders() { + internalTestFailure( + "Missing required parameter for option '--tx-pool-priority-senders' at index 0 (Comma separated list of addresses)", + "--tx-pool-priority-senders"); + } + + @Test + public void malformedListOfPrioritySenders() { + final Address prioritySender1 = Address.fromHexString("0xABC123"); + final Address prioritySender2 = Address.fromHexString("0xDEF456"); + final Address prioritySender3 = Address.fromHexString("0x789000"); + internalTestFailure( + "Invalid value for option '--tx-pool-priority-senders' at index 0 (Comma separated list of addresses): " + + "cannot convert '0x0000000000000000000000000000000000abc123;0x0000000000000000000000000000000000def456' " + + "to Address (java.lang.IllegalArgumentException: Invalid odd-length hex binary representation)", + "--tx-pool-priority-senders", + prioritySender1.toHexString() + + ";" + + prioritySender2.toHexString() + + "," + + prioritySender3.toHexString()); + } + @Override protected TransactionPoolConfiguration createDefaultDomainObject() { return TransactionPoolConfiguration.DEFAULT; diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml index 185b443c941..f4c1d517d91 100644 --- a/besu/src/test/resources/everything_config.toml +++ b/besu/src/test/resources/everything_config.toml @@ -175,7 +175,8 @@ tx-pool="layered" tx-pool-price-bump=13 rpc-tx-feecap=2000000000000000000 strict-tx-replay-protection-enabled=true -tx-pool-disable-locals=false +tx-pool-no-local-priority=false +tx-pool-priority-senders=["0xABC0000000000000000000000000000000001234","0xDEF0000000000000000000000000000000001234"] tx-pool-enable-save-restore=true tx-pool-save-file="txpool.dump" ## Layered diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java index 232140fac57..bc7e424133d 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java @@ -58,11 +58,11 @@ import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.MiningParameters; -import org.hyperledger.besu.ethereum.core.Transaction; import org.hyperledger.besu.ethereum.core.TransactionTestFixture; import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; +import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction; import org.hyperledger.besu.ethereum.eth.transactions.TransactionBroadcaster; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration; @@ -314,8 +314,8 @@ public void exceptionDuringBuildingBlockShouldNotBeInvalid() invocation -> { if (retries.getAndIncrement() < txPerBlock) { // a new transaction every time a block is built - transactions.addLocalTransaction( - createTransaction(retries.get() - 1), Optional.empty()); + transactions.addTransaction( + createLocalTransaction(retries.get() - 1), Optional.empty()); } else { // when we have 5 transactions finalize block creation willThrow.finalizeProposalById( @@ -387,8 +387,8 @@ public void shouldContinueBuildingBlocksUntilFinalizeIsCalled() invocation -> { if (retries.getAndIncrement() < 5) { // a new transaction every time a block is built - transactions.addLocalTransaction( - createTransaction(retries.get() - 1), Optional.empty()); + transactions.addTransaction( + createLocalTransaction(retries.get() - 1), Optional.empty()); } else { // when we have 5 transactions finalize block creation coordinator.finalizeProposalById( @@ -506,7 +506,7 @@ public void shouldRetryBlockCreationOnRecoverableError() .when(mergeContext) .putPayloadById(any()); - transactions.addLocalTransaction(createTransaction(0), Optional.empty()); + transactions.addTransaction(createLocalTransaction(0), Optional.empty()); var payloadId = coordinator.preparePayload( @@ -643,8 +643,8 @@ public void shouldNotStartAnotherBlockCreationJobIfCalledAgainWithTheSamePayload invocation -> { if (retries.getAndIncrement() < 5) { // a new transaction every time a block is built - transactions.addLocalTransaction( - createTransaction(retries.get() - 1), Optional.empty()); + transactions.addTransaction( + createLocalTransaction(retries.get() - 1), Optional.empty()); } else { // when we have 5 transactions finalize block creation coordinator.finalizeProposalById( @@ -1022,20 +1022,24 @@ private BlockHeader nextBlockHeader( .buildHeader(); } - private Transaction createTransaction(final long transactionNumber) { - return new TransactionTestFixture() - .value(Wei.of(transactionNumber + 1)) - .to(Optional.of(Address.ZERO)) - .gasLimit(53000L) - .gasPrice( - Wei.fromHexString("0x00000000000000000000000000000000000000000000000000000013b9aca00")) - .maxFeePerGas( - Optional.of( + private PendingTransaction createLocalTransaction(final long transactionNumber) { + return PendingTransaction.newPendingTransaction( + new TransactionTestFixture() + .value(Wei.of(transactionNumber + 1)) + .to(Optional.of(Address.ZERO)) + .gasLimit(53000L) + .gasPrice( Wei.fromHexString( - "0x00000000000000000000000000000000000000000000000000000013b9aca00"))) - .maxPriorityFeePerGas(Optional.of(Wei.of(100_000))) - .nonce(transactionNumber) - .createTransaction(KEYS1); + "0x00000000000000000000000000000000000000000000000000000013b9aca00")) + .maxFeePerGas( + Optional.of( + Wei.fromHexString( + "0x00000000000000000000000000000000000000000000000000000013b9aca00"))) + .maxPriorityFeePerGas(Optional.of(Wei.of(100_000))) + .nonce(transactionNumber) + .createTransaction(KEYS1), + true, + true); } private static BlockHeader mockBlockHeader() { diff --git a/datatypes/src/main/java/org/hyperledger/besu/datatypes/PendingTransaction.java b/datatypes/src/main/java/org/hyperledger/besu/datatypes/PendingTransaction.java index 4a8f5795ff8..e19fb13f1d1 100644 --- a/datatypes/src/main/java/org/hyperledger/besu/datatypes/PendingTransaction.java +++ b/datatypes/src/main/java/org/hyperledger/besu/datatypes/PendingTransaction.java @@ -30,6 +30,13 @@ public interface PendingTransaction { */ boolean isReceivedFromLocalSource(); + /** + * Should this transaction be prioritized? + * + * @return true if it is a transaction with priority + */ + boolean hasPriority(); + /** * Timestamp in millisecond when this transaction has been added to the pool * diff --git a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java index bdb94af6e3d..555146ca7cb 100644 --- a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java +++ b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/frontier/EthGetFilterChangesIntegrationTest.java @@ -50,6 +50,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; +import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction; import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions; import org.hyperledger.besu.ethereum.eth.transactions.TransactionBroadcaster; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; @@ -90,7 +91,8 @@ public class EthGetFilterChangesIntegrationTest { private static final int MAX_TRANSACTIONS = 5; private static final KeyPair keyPair = SignatureAlgorithmFactory.getInstance().generateKeyPair(); - private final Transaction transaction = createTransaction(1); + private final PendingTransaction pendingTransaction = + new PendingTransaction.Local((createTransaction(1))); private FilterManager filterManager; private EthGetFilterChanges method; @@ -193,7 +195,7 @@ public void shouldReturnHashesIfNewBlocks() { JsonRpcResponse actual = method.response(request); assertThat(actual).usingRecursiveComparison().isEqualTo(expected); - final Block block = appendBlock(transaction); + final Block block = appendBlock(pendingTransaction.getTransaction()); // We've added one block, so there should be one new hash. expected = new JsonRpcSuccessResponse(null, Lists.newArrayList(block.getHash().toString())); @@ -223,11 +225,12 @@ public void shouldReturnHashesIfNewPendingTransactions() { JsonRpcResponse actual = method.response(request); assertThat(actual).usingRecursiveComparison().isEqualTo(expected); - transactions.addRemoteTransaction(transaction, Optional.empty()); + transactions.addTransaction(pendingTransaction, Optional.empty()); // We've added one transaction, so there should be one new hash. expected = - new JsonRpcSuccessResponse(null, Lists.newArrayList(String.valueOf(transaction.getHash()))); + new JsonRpcSuccessResponse( + null, Lists.newArrayList(String.valueOf(pendingTransaction.getHash()))); actual = method.response(request); assertThat(actual).usingRecursiveComparison().isEqualTo(expected); diff --git a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java index 3ea2ee83633..873956d34b1 100644 --- a/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java +++ b/ethereum/api/src/integration-test/java/org/hyperledger/besu/ethereum/api/jsonrpc/methods/fork/london/EthGetFilterChangesIntegrationTest.java @@ -50,6 +50,7 @@ import org.hyperledger.besu.ethereum.eth.manager.EthContext; import org.hyperledger.besu.ethereum.eth.manager.EthPeers; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; +import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction; import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions; import org.hyperledger.besu.ethereum.eth.transactions.TransactionBroadcaster; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; @@ -90,7 +91,8 @@ public class EthGetFilterChangesIntegrationTest { private static final int MAX_TRANSACTIONS = 5; private static final KeyPair keyPair = SignatureAlgorithmFactory.getInstance().generateKeyPair(); - private final Transaction transaction = createTransaction(1); + private final PendingTransaction pendingTransaction = + new PendingTransaction.Local(createTransaction(1)); private FilterManager filterManager; private EthGetFilterChanges method; @@ -193,7 +195,7 @@ public void shouldReturnHashesIfNewBlocks() { JsonRpcResponse actual = method.response(request); assertThat(actual).usingRecursiveComparison().isEqualTo(expected); - final Block block = appendBlock(transaction); + final Block block = appendBlock(pendingTransaction.getTransaction()); // We've added one block, so there should be one new hash. expected = new JsonRpcSuccessResponse(null, Lists.newArrayList(block.getHash().toString())); @@ -223,11 +225,12 @@ public void shouldReturnHashesIfNewPendingTransactions() { JsonRpcResponse actual = method.response(request); assertThat(actual).usingRecursiveComparison().isEqualTo(expected); - transactions.addRemoteTransaction(transaction, Optional.empty()); + transactions.addTransaction(pendingTransaction, Optional.empty()); // We've added one transaction, so there should be one new hash. expected = - new JsonRpcSuccessResponse(null, Lists.newArrayList(String.valueOf(transaction.getHash()))); + new JsonRpcSuccessResponse( + null, Lists.newArrayList(String.valueOf(pendingTransaction.getHash()))); actual = method.response(request); assertThat(actual).usingRecursiveComparison().isEqualTo(expected); 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 b89ce4d3a19..03c6ef28951 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 @@ -170,7 +170,7 @@ public TransactionSelectionResults buildTransactionListForBlock() { */ public TransactionSelectionResults evaluateTransactions(final List transactions) { transactions.forEach( - transaction -> evaluateTransaction(new PendingTransaction.Local(transaction))); + transaction -> evaluateTransaction(new PendingTransaction.Local.Priority(transaction))); return transactionSelectionResults; } 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 a6c1889e497..9828650b41b 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 @@ -73,8 +73,7 @@ private boolean transactionCurrentPriceBelowMin(final PendingTransaction pending final Transaction transaction = pendingTransaction.getTransaction(); // Here we only care about EIP1159 since for Frontier and local transactions the checks // that we do when accepting them in the pool are enough - if (transaction.getType().supports1559FeeMarket() - && !pendingTransaction.isReceivedFromLocalSource()) { + if (transaction.getType().supports1559FeeMarket() && !pendingTransaction.hasPriority()) { // For EIP1559 transactions, the price is dynamic and depends on network conditions, so we can // only calculate at this time the current minimum price the transaction is willing to pay @@ -90,7 +89,7 @@ private boolean transactionCurrentPriceBelowMin(final PendingTransaction pending if (context.minTransactionGasPrice().compareTo(currentMinTransactionGasPriceInBlock) > 0) { LOG.trace( "Current gas fee of {} is lower than configured minimum {}, skipping", - transaction, + pendingTransaction, context.minTransactionGasPrice()); return true; } diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java index 29860d555a6..57b89f8a1d7 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/LondonFeeMarketBlockTransactionSelectorTest.java @@ -155,7 +155,7 @@ public void eip1559TransactionCurrentGasPriceGreaterThanMinimumIsSelected() { } @Test - public void eip1559LocalTransactionCurrentGasPriceLessThanMinimumIsSelected() { + public void eip1559PriorityTransactionCurrentGasPriceLessThanMinimumIsSelected() { final ProcessableBlockHeader blockHeader = createBlock(301_000, Wei.ONE); final Address miningBeneficiary = AddressHelpers.ofValue(1); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/DisabledPendingTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/DisabledPendingTransactions.java index da77f6f3196..de9d6a07e8e 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/DisabledPendingTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/DisabledPendingTransactions.java @@ -41,14 +41,13 @@ public List getLocalTransactions() { } @Override - public TransactionAddedResult addRemoteTransaction( - final Transaction transaction, final Optional maybeSenderAccount) { - return TransactionAddedResult.DISABLED; + public List getPriorityTransactions() { + return List.of(); } @Override - public TransactionAddedResult addLocalTransaction( - final Transaction transaction, final Optional maybeSenderAccount) { + public TransactionAddedResult addTransaction( + final PendingTransaction transaction, final Optional maybeSenderAccount) { return TransactionAddedResult.DISABLED; } @@ -117,9 +116,4 @@ public String toTraceLog() { public String logStats() { return "Disabled"; } - - @Override - public boolean isLocalSender(final Address sender) { - return false; - } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransaction.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransaction.java index 03e6ed69009..dbc461ecc38 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransaction.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransaction.java @@ -64,6 +64,28 @@ private PendingTransaction(final Transaction transaction, final long addedAt) { this(transaction, addedAt, TRANSACTIONS_ADDED.getAndIncrement()); } + public static PendingTransaction newPendingTransaction( + final Transaction transaction, final boolean isLocal, final boolean hasPriority) { + return newPendingTransaction(transaction, isLocal, hasPriority, System.currentTimeMillis()); + } + + public static PendingTransaction newPendingTransaction( + final Transaction transaction, + final boolean isLocal, + final boolean hasPriority, + final long addedAt) { + if (isLocal) { + if (hasPriority) { + return new Local.Priority(transaction, addedAt); + } + return new Local(transaction, addedAt); + } + if (hasPriority) { + return new Remote.Priority(transaction, addedAt); + } + return new Remote(transaction, addedAt); + } + @Override public Transaction getTransaction() { return transaction; @@ -226,6 +248,8 @@ public String toString() { + sequence + ", isLocal=" + isReceivedFromLocalSource() + + ", hasPriority=" + + hasPriority() + '}'; } @@ -236,6 +260,8 @@ public String toTraceLog() { + addedAt + ", isLocal=" + isReceivedFromLocalSource() + + ", hasPriority=" + + hasPriority() + ", " + transaction.toTraceLog() + "}"; @@ -264,6 +290,35 @@ public PendingTransaction detachedCopy() { public boolean isReceivedFromLocalSource() { return true; } + + @Override + public boolean hasPriority() { + return false; + } + + public static class Priority extends Local { + public Priority(final Transaction transaction) { + this(transaction, System.currentTimeMillis()); + } + + public Priority(final Transaction transaction, final long addedAt) { + super(transaction, addedAt); + } + + public Priority(final long sequence, final Transaction transaction) { + super(sequence, transaction); + } + + @Override + public PendingTransaction detachedCopy() { + return new Priority(getSequence(), getTransaction().detachedCopy()); + } + + @Override + public boolean hasPriority() { + return true; + } + } } public static class Remote extends PendingTransaction { @@ -289,5 +344,34 @@ public PendingTransaction detachedCopy() { public boolean isReceivedFromLocalSource() { return false; } + + @Override + public boolean hasPriority() { + return false; + } + + public static class Priority extends Remote { + public Priority(final Transaction transaction) { + this(transaction, System.currentTimeMillis()); + } + + public Priority(final Transaction transaction, final long addedAt) { + super(transaction, addedAt); + } + + public Priority(final long sequence, final Transaction transaction) { + super(sequence, transaction); + } + + @Override + public PendingTransaction detachedCopy() { + return new Priority(getSequence(), getTransaction().detachedCopy()); + } + + @Override + public boolean hasPriority() { + return true; + } + } } } 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 199e88675e0..66f3bc0f4b0 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 @@ -35,11 +35,10 @@ public interface PendingTransactions { List getLocalTransactions(); - TransactionAddedResult addRemoteTransaction( - Transaction transaction, Optional maybeSenderAccount); + List getPriorityTransactions(); - TransactionAddedResult addLocalTransaction( - Transaction transaction, Optional maybeSenderAccount); + TransactionAddedResult addTransaction( + PendingTransaction transaction, Optional maybeSenderAccount); void selectTransactions(TransactionSelector selector); @@ -84,8 +83,6 @@ default void signalInvalidAndRemoveDependentTransactions(final Transaction trans // no-op } - boolean isLocalSender(Address sender); - @FunctionalInterface interface TransactionSelector { TransactionSelectionResult evaluateTransaction(PendingTransaction pendingTransaction); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java index f2aec4e7e33..c41027deb1b 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPool.java @@ -63,7 +63,9 @@ import java.util.Optional; import java.util.OptionalLong; import java.util.Queue; +import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -106,6 +108,7 @@ public class TransactionPool implements BlockAddedObserver { new PendingTransactionsListenersProxy(); private volatile OptionalLong subscribeConnectId = OptionalLong.empty(); private final SaveRestoreManager saveRestoreManager = new SaveRestoreManager(); + private final Set
localSenders = ConcurrentHashMap.newKeySet(); private final Lock blockAddedLock = new ReentrantLock(); private final Queue blockAddedQueue = new ConcurrentLinkedQueue<>(); @@ -135,6 +138,7 @@ public TransactionPool( } private void initLogForReplay() { + // log the initial block header data LOG_FOR_REPLAY .atTrace() .setMessage("{},{},{},{}") @@ -148,6 +152,16 @@ private void initLogForReplay() { .addArgument(() -> getChainHeadBlockHeader().map(BlockHeader::getGasUsed).orElse(0L)) .addArgument(() -> getChainHeadBlockHeader().map(BlockHeader::getGasLimit).orElse(0L)) .log(); + // log the priority senders + LOG_FOR_REPLAY + .atTrace() + .setMessage("{}") + .addArgument( + () -> + configuration.getPrioritySenders().stream() + .map(Address::toHexString) + .collect(Collectors.joining(","))) + .log(); } @VisibleForTesting @@ -159,57 +173,12 @@ void handleConnect(final EthPeer peer) { public ValidationResult addTransactionViaApi( final Transaction transaction) { - if (configuration.getDisableLocalTransactions()) { - final var result = addRemoteTransaction(transaction); - if (result.isValid()) { - transactionBroadcaster.onTransactionsAdded(List.of(transaction)); - } - return result; - } - return addLocalTransaction(transaction); - } - - ValidationResult addLocalTransaction(final Transaction transaction) { - final ValidationResultAndAccount validationResult = validateLocalTransaction(transaction); - - if (validationResult.result.isValid()) { - - final TransactionAddedResult transactionAddedResult = - pendingTransactions.addLocalTransaction(transaction, validationResult.maybeAccount); - - if (transactionAddedResult.isRejected()) { - final var rejectReason = - transactionAddedResult - .maybeInvalidReason() - .orElseGet( - () -> { - LOG.warn("Missing invalid reason for status {}", transactionAddedResult); - return INTERNAL_ERROR; - }); - return ValidationResult.invalid(rejectReason); - } - + final var result = addTransaction(transaction, true); + if (result.isValid()) { + localSenders.add(transaction.getSender()); transactionBroadcaster.onTransactionsAdded(List.of(transaction)); - } else { - metrics.incrementRejected(true, validationResult.result.getInvalidReason(), "txpool"); } - - return validationResult.result; - } - - private Optional getMaxGasPrice(final Transaction transaction) { - return transaction.getGasPrice().map(Optional::of).orElse(transaction.getMaxFeePerGas()); - } - - private boolean isMaxGasPriceBelowConfiguredMinGasPrice(final Transaction transaction) { - return getMaxGasPrice(transaction) - .map(g -> g.lessThan(miningParameters.getMinTransactionGasPrice())) - .orElse(true); - } - - private Stream sortedBySenderAndNonce(final Collection transactions) { - return transactions.stream() - .sorted(Comparator.comparing(Transaction::getSender).thenComparing(Transaction::getNonce)); + return result; } public Map> addRemoteTransactions( @@ -225,7 +194,7 @@ public Map> addRemoteTransactio Collectors.toMap( Transaction::getHash, transaction -> { - final var result = addRemoteTransaction(transaction); + final var result = addTransaction(transaction, false); if (result.isValid()) { addedTransactions.add(transaction); } @@ -254,26 +223,33 @@ public Map> addRemoteTransactio return validationResults; } - private ValidationResult addRemoteTransaction( - final Transaction transaction) { + private ValidationResult addTransaction( + final Transaction transaction, final boolean isLocal) { + + final boolean hasPriority = isPriorityTransaction(transaction, isLocal); + if (pendingTransactions.containsTransaction(transaction)) { LOG.atTrace() .setMessage("Discard already present transaction {}") .addArgument(transaction::toTraceLog) .log(); // We already have this transaction, don't even validate it. - metrics.incrementRejected(false, TRANSACTION_ALREADY_KNOWN, "txpool"); + metrics.incrementRejected(isLocal, hasPriority, TRANSACTION_ALREADY_KNOWN, "txpool"); return ValidationResult.invalid(TRANSACTION_ALREADY_KNOWN); } - final ValidationResultAndAccount validationResult = validateRemoteTransaction(transaction); + final ValidationResultAndAccount validationResult = + validateTransaction(transaction, isLocal, hasPriority); if (validationResult.result.isValid()) { final TransactionAddedResult status = - pendingTransactions.addRemoteTransaction(transaction, validationResult.maybeAccount); + pendingTransactions.addTransaction( + PendingTransaction.newPendingTransaction(transaction, isLocal, hasPriority), + validationResult.maybeAccount); if (status.isSuccess()) { LOG.atTrace() - .setMessage("Added remote transaction {}") + .setMessage("Added {} transaction {}") + .addArgument(() -> isLocal ? "local" : "remote") .addArgument(transaction::toTraceLog) .log(); } else { @@ -290,7 +266,7 @@ private ValidationResult addRemoteTransaction( .addArgument(transaction::toTraceLog) .addArgument(rejectReason) .log(); - metrics.incrementRejected(false, rejectReason, "txpool"); + metrics.incrementRejected(isLocal, hasPriority, rejectReason, "txpool"); return ValidationResult.invalid(rejectReason); } } else { @@ -299,13 +275,40 @@ private ValidationResult addRemoteTransaction( .addArgument(transaction::toTraceLog) .addArgument(validationResult.result::getInvalidReason) .log(); - metrics.incrementRejected(false, validationResult.result.getInvalidReason(), "txpool"); - pendingTransactions.signalInvalidAndRemoveDependentTransactions(transaction); + metrics.incrementRejected( + isLocal, hasPriority, validationResult.result.getInvalidReason(), "txpool"); + if (!isLocal) { + pendingTransactions.signalInvalidAndRemoveDependentTransactions(transaction); + } } return validationResult.result; } + private Optional getMaxGasPrice(final Transaction transaction) { + return transaction.getGasPrice().map(Optional::of).orElse(transaction.getMaxFeePerGas()); + } + + private boolean isMaxGasPriceBelowConfiguredMinGasPrice(final Transaction transaction) { + return getMaxGasPrice(transaction) + .map(g -> g.lessThan(miningParameters.getMinTransactionGasPrice())) + .orElse(true); + } + + private Stream sortedBySenderAndNonce(final Collection transactions) { + return transactions.stream() + .sorted(Comparator.comparing(Transaction::getSender).thenComparing(Transaction::getNonce)); + } + + private boolean isPriorityTransaction(final Transaction transaction, final boolean isLocal) { + if (isLocal && !configuration.getNoLocalPriority()) { + // unless no-local-priority option is specified, senders of local sent txs are prioritized + return true; + } + // otherwise check if the sender belongs to the priority list + return configuration.getPrioritySenders().contains(transaction.getSender()); + } + public long subscribePendingTransactions(final PendingTransactionAddedListener listener) { return pendingTransactionsListenersProxy.onAddedListeners.subscribe(listener); } @@ -374,17 +377,12 @@ private void reAddTransactions(final List reAddTransactions) { if (!reAddTransactions.isEmpty()) { var txsByOrigin = reAddTransactions.stream() - .collect( - Collectors.partitioningBy( - tx -> - configuration.getDisableLocalTransactions() - ? false - : pendingTransactions.isLocalSender(tx.getSender()))); + .collect(Collectors.partitioningBy(tx -> isLocalSender(tx.getSender()))); var reAddLocalTxs = txsByOrigin.get(true); var reAddRemoteTxs = txsByOrigin.get(false); if (!reAddLocalTxs.isEmpty()) { logReAddedTransactions(reAddLocalTxs, "local"); - sortedBySenderAndNonce(reAddLocalTxs).forEach(this::addLocalTransaction); + sortedBySenderAndNonce(reAddLocalTxs).forEach(this::addTransactionViaApi); } if (!reAddRemoteTxs.isEmpty()) { logReAddedTransactions(reAddRemoteTxs, "remote"); @@ -412,16 +410,8 @@ private TransactionValidator getTransactionValidator() { .get(); } - private ValidationResultAndAccount validateLocalTransaction(final Transaction transaction) { - return validateTransaction(transaction, true); - } - - private ValidationResultAndAccount validateRemoteTransaction(final Transaction transaction) { - return validateTransaction(transaction, false); - } - private ValidationResultAndAccount validateTransaction( - final Transaction transaction, final boolean isLocal) { + final Transaction transaction, final boolean isLocal, final boolean hasPriority) { final BlockHeader chainHeadBlockHeader = getChainHeadBlockHeader().orElse(null); if (chainHeadBlockHeader == null) { @@ -436,7 +426,7 @@ private ValidationResultAndAccount validateTransaction( protocolSchedule.getByBlockHeader(chainHeadBlockHeader).getFeeMarket(); final TransactionInvalidReason priceInvalidReason = - validatePrice(transaction, isLocal, feeMarket); + validatePrice(transaction, isLocal, hasPriority, feeMarket); if (priceInvalidReason != null) { return ValidationResultAndAccount.invalid(priceInvalidReason); } @@ -451,7 +441,7 @@ private ValidationResultAndAccount validateTransaction( return new ValidationResultAndAccount(basicValidationResult); } - if (isLocal + if (hasPriority && strictReplayProtectionShouldBeEnforcedLocally(chainHeadBlockHeader) && transaction.getChainId().isEmpty()) { // Strict replay protection is enabled but the tx is not replay-protected @@ -507,14 +497,19 @@ && strictReplayProtectionShouldBeEnforcedLocally(chainHeadBlockHeader) } private TransactionInvalidReason validatePrice( - final Transaction transaction, final boolean isLocal, final FeeMarket feeMarket) { + final Transaction transaction, + final boolean isLocal, + final boolean hasPriority, + final FeeMarket feeMarket) { if (isLocal) { if (!configuration.getTxFeeCap().isZero() && getMaxGasPrice(transaction).get().greaterThan(configuration.getTxFeeCap())) { return TransactionInvalidReason.TX_FEECAP_EXCEEDED; } - // allow local transactions to be below minGas as long as we are mining + } + if (hasPriority) { + // allow priority transactions to be below minGas as long as we are mining // or at least gas price is above the configured floor if ((!miningParameters.isMiningEnabled() && isMaxGasPriceBelowConfiguredMinGasPrice(transaction)) @@ -554,8 +549,8 @@ private Optional getChainHeadBlockHeader() { return blockchain.getBlockHeader(blockchain.getChainHeadHash()); } - public boolean isLocalSender(final Address sender) { - return pendingTransactions.isLocalSender(sender); + private boolean isLocalSender(final Address sender) { + return localSenders.contains(sender); } public int count() { @@ -804,12 +799,8 @@ private void executeLoadFromDisk() { final Transaction tx = Transaction.readFrom(Bytes.fromBase64String(line.substring(1))); - final ValidationResult result; - if (isLocal && !configuration.getDisableLocalTransactions()) { - result = addLocalTransaction(tx); - } else { - result = addRemoteTransaction(tx); - } + final ValidationResult result = + addTransaction(tx, isLocal); return result.isValid() ? 1 : 0; }) diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java index aaf8c326c0a..874446b7671 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolConfiguration.java @@ -14,12 +14,14 @@ */ package org.hyperledger.besu.ethereum.eth.transactions; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.util.number.Fraction; import org.hyperledger.besu.util.number.Percentage; import java.io.File; import java.time.Duration; +import java.util.Set; import org.immutables.value.Value; @@ -53,21 +55,20 @@ enum Implementation { } String DEFAULT_SAVE_FILE_NAME = "txpool.dump"; - int DEFAULT_MAX_PENDING_TRANSACTIONS = 4096; Fraction DEFAULT_LIMIT_TX_POOL_BY_ACCOUNT_PERCENTAGE = Fraction.fromFloat(0.001f); // 0.1% int DEFAULT_TX_RETENTION_HOURS = 13; boolean DEFAULT_STRICT_TX_REPLAY_PROTECTION_ENABLED = false; Percentage DEFAULT_PRICE_BUMP = Percentage.fromInt(10); Wei DEFAULT_RPC_TX_FEE_CAP = Wei.fromEth(1); - boolean DEFAULT_DISABLE_LOCAL_TXS = false; + boolean DEFAULT_NO_LOCAL_PRIORITY = false; boolean DEFAULT_ENABLE_SAVE_RESTORE = false; - File DEFAULT_SAVE_FILE = new File(DEFAULT_SAVE_FILE_NAME); long DEFAULT_PENDING_TRANSACTIONS_LAYER_MAX_CAPACITY_BYTES = 12_500_000L; int DEFAULT_MAX_PRIORITIZED_TRANSACTIONS = 2000; int DEFAULT_MAX_FUTURE_BY_SENDER = 200; Implementation DEFAULT_TX_POOL_IMPLEMENTATION = Implementation.LAYERED; + Set
DEFAULT_PRIORITY_SENDERS = Set.of(); TransactionPoolConfiguration DEFAULT = ImmutableTransactionPoolConfiguration.builder().build(); @@ -107,8 +108,8 @@ default Boolean getStrictTransactionReplayProtectionEnabled() { } @Value.Default - default Boolean getDisableLocalTransactions() { - return DEFAULT_DISABLE_LOCAL_TXS; + default Boolean getNoLocalPriority() { + return DEFAULT_NO_LOCAL_PRIORITY; } @Value.Default @@ -141,6 +142,11 @@ default int getMaxFutureBySender() { return DEFAULT_MAX_FUTURE_BY_SENDER; } + @Value.Default + default Set
getPrioritySenders() { + return DEFAULT_PRIORITY_SENDERS; + } + @Value.Default default Unstable getUnstable() { return Unstable.DEFAULT; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolMetrics.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolMetrics.java index 30ddb9db3db..9ace1819cc1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolMetrics.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolMetrics.java @@ -56,6 +56,7 @@ public TransactionPoolMetrics(final MetricsSystem metricsSystem) { ADDED_COUNTER_NAME, "Count of transactions added to the transaction pool", "source", + "priority", "layer"); removedCounter = @@ -64,6 +65,7 @@ public TransactionPoolMetrics(final MetricsSystem metricsSystem) { REMOVED_COUNTER_NAME, "Count of transactions removed from the transaction pool", "source", + "priority", "operation", "layer"); @@ -73,6 +75,7 @@ public TransactionPoolMetrics(final MetricsSystem metricsSystem) { REJECTED_COUNTER_NAME, "Count of transactions not accepted to the transaction pool", "source", + "priority", "reason", "layer"); @@ -143,20 +146,46 @@ public void initExpiredMessagesCounter(final String message) { SKIPPED_MESSAGES_LOGGING_THRESHOLD)); } - public void incrementAdded(final boolean receivedFromLocalSource, final String layer) { - addedCounter.labels(location(receivedFromLocalSource), layer).inc(); + public void incrementAdded(final PendingTransaction pendingTransaction, final String layer) { + addedCounter + .labels( + location(pendingTransaction.isReceivedFromLocalSource()), + priority(pendingTransaction.hasPriority()), + layer) + .inc(); } public void incrementRemoved( - final boolean receivedFromLocalSource, final String operation, final String layer) { - removedCounter.labels(location(receivedFromLocalSource), operation, layer).inc(); + final PendingTransaction pendingTransaction, final String operation, final String layer) { + removedCounter + .labels( + location(pendingTransaction.isReceivedFromLocalSource()), + priority(pendingTransaction.hasPriority()), + operation, + layer) + .inc(); + } + + public void incrementRejected( + final PendingTransaction pendingTransaction, + final TransactionInvalidReason rejectReason, + final String layer) { + incrementRejected( + pendingTransaction.isReceivedFromLocalSource(), + pendingTransaction.hasPriority(), + rejectReason, + layer); } public void incrementRejected( final boolean receivedFromLocalSource, + final boolean hasPriority, final TransactionInvalidReason rejectReason, final String layer) { - rejectedCounter.labels(location(receivedFromLocalSource), rejectReason.name(), layer).inc(); + rejectedCounter + .labels( + location(receivedFromLocalSource), priority(hasPriority), rejectReason.name(), layer) + .inc(); } public void incrementExpiredMessages(final String message) { @@ -170,4 +199,8 @@ public void incrementAlreadySeenTransactions(final String message, final long co private String location(final boolean receivedFromLocalSource) { return receivedFromLocalSource ? "local" : "remote"; } + + private String priority(final boolean hasPriority) { + return hasPriority ? "yes" : "no"; + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java index 12ffd91f590..136f80499c1 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java @@ -163,7 +163,7 @@ public TransactionAddedResult add(final PendingTransaction pendingTransaction, f notifyTransactionAdded(pendingTransaction); } else { final var rejectReason = addStatus.maybeInvalidReason().orElseThrow(); - metrics.incrementRejected(false, rejectReason, name()); + metrics.incrementRejected(pendingTransaction, rejectReason, name()); LOG.atTrace() .setMessage("Transaction {} rejected reason {}") .addArgument(pendingTransaction::toTraceLog) @@ -245,7 +245,7 @@ public PendingTransaction promoteFor(final Address sender, final long nonce) { if (senderTxs.firstKey() == expectedNonce) { final PendingTransaction promotedTx = senderTxs.pollFirstEntry().getValue(); processRemove(senderTxs, promotedTx.getTransaction(), PROMOTED); - metrics.incrementRemoved(promotedTx.isReceivedFromLocalSource(), "promoted", name()); + metrics.incrementRemoved(promotedTx, "promoted", name()); if (senderTxs.isEmpty()) { txsBySender.remove(sender); @@ -282,7 +282,7 @@ private void processAdded(final PendingTransaction addedTx) { final var senderTxs = txsBySender.computeIfAbsent(addedTx.getSender(), s -> new TreeMap<>()); senderTxs.put(addedTx.getNonce(), addedTx); increaseSpaceUsed(addedTx); - metrics.incrementAdded(addedTx.isReceivedFromLocalSource(), name()); + metrics.incrementAdded(addedTx, name()); internalAdd(senderTxs, addedTx); } @@ -328,7 +328,7 @@ private void evict(final long spaceToFree, final int txsToEvict) { protected void replaced(final PendingTransaction replacedTx) { pendingTransactions.remove(replacedTx.getHash()); decreaseSpaceUsed(replacedTx); - metrics.incrementRemoved(replacedTx.isReceivedFromLocalSource(), REPLACED.label(), name()); + metrics.incrementRemoved(replacedTx, REPLACED.label(), name()); internalReplaced(replacedTx); notifyTransactionDropped(replacedTx); } @@ -363,8 +363,7 @@ protected PendingTransaction processRemove( final PendingTransaction removedTx = pendingTransactions.remove(transaction.getHash()); if (removedTx != null) { decreaseSpaceUsed(removedTx); - metrics.incrementRemoved( - removedTx.isReceivedFromLocalSource(), removalReason.label(), name()); + metrics.incrementRemoved(removedTx, removalReason.label(), name()); internalRemove(senderTxs, removedTx, removalReason); } return removedTx; @@ -377,7 +376,7 @@ protected PendingTransaction processEvict( final PendingTransaction removedTx = pendingTransactions.remove(evictedTx.getHash()); if (removedTx != null) { decreaseSpaceUsed(evictedTx); - metrics.incrementRemoved(evictedTx.isReceivedFromLocalSource(), reason.label(), name()); + metrics.incrementRemoved(evictedTx, reason.label(), name()); internalEvict(senderTxs, removedTx); } return removedTx; @@ -431,7 +430,7 @@ private void confirmed(final Address sender, final long maxConfirmedNonce) { itConfirmedTxs.remove(); processRemove(senderTxs, confirmedTx.getTransaction(), CONFIRMED); - metrics.incrementRemoved(confirmedTx.isReceivedFromLocalSource(), "confirmed", name()); + metrics.incrementRemoved(confirmedTx, "confirmed", name()); LOG.atTrace() .setMessage("Removed confirmed pending transactions {}") .addArgument(confirmedTx::toTraceLog) @@ -482,6 +481,17 @@ public List getAllLocal() { return localTxs; } + @Override + public List getAllPriority() { + final var priorityTxs = + pendingTransactions.values().stream() + .filter(PendingTransaction::hasPriority) + .map(PendingTransaction::getTransaction) + .collect(Collectors.toCollection(ArrayList::new)); + priorityTxs.addAll(nextLayer.getAllPriority()); + return priorityTxs; + } + Stream stream(final Address sender) { return txsBySender.getOrDefault(sender, EMPTY_SENDER_TXS).values().stream(); } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseFeePrioritizedTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseFeePrioritizedTransactions.java index 3dcdfd734f8..f5ee0de2ae4 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseFeePrioritizedTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseFeePrioritizedTransactions.java @@ -53,7 +53,8 @@ public BaseFeePrioritizedTransactions( @Override protected int compareByFee(final PendingTransaction pt1, final PendingTransaction pt2) { - return Comparator.comparing( + return Comparator.comparing(PendingTransaction::hasPriority) + .thenComparing( (PendingTransaction pendingTransaction) -> pendingTransaction.getTransaction().getEffectivePriorityFeePerGas(nextBlockBaseFee)) .thenComparing( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/EndLayer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/EndLayer.java index 12d3147b326..e79ce1d8a71 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/EndLayer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/EndLayer.java @@ -77,8 +77,7 @@ public List getAll() { @Override public TransactionAddedResult add(final PendingTransaction pendingTransaction, final int gap) { notifyTransactionDropped(pendingTransaction); - metrics.incrementRemoved( - pendingTransaction.isReceivedFromLocalSource(), DROPPED.label(), name()); + metrics.incrementRemoved(pendingTransaction, DROPPED.label(), name()); ++droppedCount; return TransactionAddedResult.DROPPED; } @@ -99,6 +98,11 @@ public List getAllLocal() { return List.of(); } + @Override + public List getAllPriority() { + return List.of(); + } + @Override public int count() { return 0; diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/GasPricePrioritizedTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/GasPricePrioritizedTransactions.java index 725b5138d34..762e7469f45 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/GasPricePrioritizedTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/GasPricePrioritizedTransactions.java @@ -43,7 +43,7 @@ public GasPricePrioritizedTransactions( @Override protected int compareByFee(final PendingTransaction pt1, final PendingTransaction pt2) { - return comparing(PendingTransaction::isReceivedFromLocalSource) + return comparing(PendingTransaction::hasPriority) .thenComparing(PendingTransaction::getGasPrice) .thenComparing(PendingTransaction::getSequence) .compare(pt1, pt2); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java index 5502e9a6f1a..bd93f0d9fa0 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactions.java @@ -58,7 +58,6 @@ public class LayeredPendingTransactions implements PendingTransactions { private static final Logger LOG = LoggerFactory.getLogger(LayeredPendingTransactions.class); private static final Logger LOG_FOR_REPLAY = LoggerFactory.getLogger("LOG_FOR_REPLAY"); private final TransactionPoolConfiguration poolConfig; - private final Set
localSenders = new HashSet<>(); private final AbstractPrioritizedTransactions prioritizedTransactions; public LayeredPendingTransactions( @@ -74,25 +73,7 @@ public synchronized void reset() { } @Override - public synchronized TransactionAddedResult addRemoteTransaction( - final Transaction transaction, final Optional maybeSenderAccount) { - - return addTransaction(new PendingTransaction.Remote(transaction), maybeSenderAccount); - } - - @Override - public synchronized TransactionAddedResult addLocalTransaction( - final Transaction transaction, final Optional maybeSenderAccount) { - - final TransactionAddedResult addedResult = - addTransaction(new PendingTransaction.Local(transaction), maybeSenderAccount); - if (addedResult.isSuccess()) { - localSenders.add(transaction.getSender()); - } - return addedResult; - } - - TransactionAddedResult addTransaction( + public synchronized TransactionAddedResult addTransaction( final PendingTransaction pendingTransaction, final Optional maybeSenderAccount) { final long stateSenderNonce = maybeSenderAccount.map(AccountState::getNonce).orElse(0L); @@ -305,8 +286,8 @@ public synchronized List getLocalTransactions() { } @Override - public synchronized boolean isLocalSender(final Address sender) { - return localSenders.contains(sender); + public synchronized List getPriorityTransactions() { + return prioritizedTransactions.getAllPriority(); } @Override diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReadyTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReadyTransactions.java index 227d0e4daa3..484f131b0b9 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReadyTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReadyTransactions.java @@ -43,7 +43,8 @@ public class ReadyTransactions extends AbstractSequentialTransactionsLayer { private final NavigableSet orderByMaxFee = new TreeSet<>( - Comparator.comparing((PendingTransaction pt) -> pt.getTransaction().getMaxGasPrice()) + Comparator.comparing(PendingTransaction::hasPriority) + .thenComparing((PendingTransaction pt) -> pt.getTransaction().getMaxGasPrice()) .thenComparing(PendingTransaction::getSequence)); public ReadyTransactions( diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java index 64a4142bccb..3567fb84266 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/SparseTransactions.java @@ -30,6 +30,7 @@ import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NavigableMap; @@ -44,11 +45,15 @@ import java.util.stream.IntStream; import java.util.stream.Stream; +import com.google.common.collect.Iterables; + public class SparseTransactions extends AbstractTransactionsLayer { private final NavigableSet sparseEvictionOrder = - new TreeSet<>(Comparator.comparing(PendingTransaction::getSequence)); + new TreeSet<>( + Comparator.comparing(PendingTransaction::hasPriority) + .thenComparing(PendingTransaction::getSequence)); private final Map gapBySender = new HashMap<>(); - private final List> orderByGap; + private final List orderByGap; public SparseTransactions( final TransactionPoolConfiguration poolConfig, @@ -59,7 +64,7 @@ public SparseTransactions( super(poolConfig, nextLayer, transactionReplacementTester, metrics); orderByGap = new ArrayList<>(poolConfig.getMaxFutureBySender()); IntStream.range(0, poolConfig.getMaxFutureBySender()) - .forEach(i -> orderByGap.add(new HashSet<>())); + .forEach(i -> orderByGap.add(new SendersByPriority())); } @Override @@ -82,7 +87,7 @@ public void reset() { super.reset(); sparseEvictionOrder.clear(); gapBySender.clear(); - orderByGap.forEach(Set::clear); + orderByGap.forEach(SendersByPriority::clear); } @Override @@ -92,12 +97,12 @@ protected TransactionAddedResult canAdd( pendingTransaction.getSender(), (sender, currGap) -> { if (currGap == null) { - orderByGap.get(gap).add(sender); + orderByGap.get(gap).add(pendingTransaction); return gap; } if (pendingTransaction.getNonce() < txsBySender.get(sender).firstKey()) { orderByGap.get(currGap).remove(sender); - orderByGap.get(gap).add(sender); + orderByGap.get(gap).add(pendingTransaction); return gap; } return currGap; @@ -390,8 +395,8 @@ public String internalLogStats() { } private void updateGap(final Address sender, final int currGap, final int newGap) { - orderByGap.get(currGap).remove(sender); - orderByGap.get(newGap).add(sender); + final boolean hasPriority = orderByGap.get(currGap).remove(sender); + orderByGap.get(newGap).add(sender, hasPriority); gapBySender.put(sender, newGap); } @@ -430,4 +435,50 @@ protected void internalConsistencyCheck( } }); } + + private static class SendersByPriority implements Iterable
{ + final Set
prioritySenders = new HashSet<>(); + final Set
standardSenders = new HashSet<>(); + + void clear() { + prioritySenders.clear(); + standardSenders.clear(); + } + + public void add(final Address sender, final boolean hasPriority) { + if (hasPriority) { + if (standardSenders.contains(sender)) { + throw new IllegalStateException( + "Sender " + sender + " cannot simultaneously have and not have priority"); + } + prioritySenders.add(sender); + } else { + if (prioritySenders.contains(sender)) { + throw new IllegalStateException( + "Sender " + sender + " cannot simultaneously have and not have priority"); + } + standardSenders.add(sender); + } + } + + void add(final PendingTransaction pendingTransaction) { + add(pendingTransaction.getSender(), pendingTransaction.hasPriority()); + } + + boolean remove(final Address sender) { + if (standardSenders.remove(sender)) { + return false; + } + return prioritySenders.remove(sender); + } + + public boolean contains(final Address sender) { + return standardSenders.contains(sender) || prioritySenders.contains(sender); + } + + @Override + public Iterator
iterator() { + return Iterables.concat(prioritySenders, standardSenders).iterator(); + } + } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/TransactionsLayer.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/TransactionsLayer.java index c6fcc5e7cee..85227766b40 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/TransactionsLayer.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/layered/TransactionsLayer.java @@ -53,6 +53,8 @@ void blockAdded( List getAllLocal(); + List getAllPriority(); + int count(); OptionalLong getNextNonceFor(Address sender); diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java index ea12b3c4f6a..ad2595c2658 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsSorter.java @@ -97,8 +97,6 @@ public abstract class AbstractPendingTransactionsSorter implements PendingTransa protected final TransactionPoolReplacementHandler transactionReplacementHandler; protected final Supplier chainHeadHeaderSupplier; - private final Set
localSenders; - public AbstractPendingTransactionsSorter( final TransactionPoolConfiguration poolConfig, final Clock clock, @@ -106,8 +104,6 @@ public AbstractPendingTransactionsSorter( final Supplier chainHeadHeaderSupplier) { this.poolConfig = poolConfig; this.pendingTransactions = new ConcurrentHashMap<>(poolConfig.getTxPoolMaxSize()); - this.localSenders = - poolConfig.getDisableLocalTransactions() ? Set.of() : ConcurrentHashMap.newKeySet(); this.clock = clock; this.chainHeadHeaderSupplier = chainHeadHeaderSupplier; this.transactionReplacementHandler = @@ -172,10 +168,19 @@ public List getLocalTransactions() { } @Override - public TransactionAddedResult addRemoteTransaction( - final Transaction transaction, final Optional maybeSenderAccount) { + public List getPriorityTransactions() { + return pendingTransactions.values().stream() + .filter(PendingTransaction::hasPriority) + .map(PendingTransaction::getTransaction) + .collect(Collectors.toList()); + } + + @Override + public TransactionAddedResult addTransaction( + final PendingTransaction transaction, final Optional maybeSenderAccount) { - if (lowestInvalidKnownNonceCache.hasInvalidLowerNonce(transaction)) { + if (!transaction.isReceivedFromLocalSource() + && lowestInvalidKnownNonceCache.hasInvalidLowerNonce(transaction.getTransaction())) { LOG.atDebug() .setMessage( "Dropping transaction {} since the sender has an invalid transaction with lower nonce") @@ -184,30 +189,19 @@ public TransactionAddedResult addRemoteTransaction( return LOWER_NONCE_INVALID_TRANSACTION_KNOWN; } - final PendingTransaction pendingTransaction = - new PendingTransaction.Remote(transaction, clock.millis()); final TransactionAddedResult transactionAddedStatus = - addTransaction(pendingTransaction, maybeSenderAccount); + internalAddTransaction(transaction, maybeSenderAccount); if (transactionAddedStatus.equals(ADDED)) { - lowestInvalidKnownNonceCache.registerValidTransaction(transaction); - remoteTransactionAddedCounter.inc(); + if (!transaction.isReceivedFromLocalSource()) { + lowestInvalidKnownNonceCache.registerValidTransaction(transaction.getTransaction()); + remoteTransactionAddedCounter.inc(); + } else { + localTransactionAddedCounter.inc(); + } } return transactionAddedStatus; } - @Override - public TransactionAddedResult addLocalTransaction( - final Transaction transaction, final Optional maybeSenderAccount) { - final TransactionAddedResult transactionAdded = - addTransaction( - new PendingTransaction.Local(transaction, clock.millis()), maybeSenderAccount); - if (transactionAdded.equals(ADDED)) { - localSenders.add(transaction.getSender()); - localTransactionAddedCounter.inc(); - } - return transactionAdded; - } - void removeTransaction(final Transaction transaction) { removeTransaction(transaction, false); notifyTransactionDropped(transaction); @@ -428,7 +422,7 @@ private void removeTransaction(final Transaction transaction, final boolean adde protected abstract void prioritizeTransaction(final PendingTransaction pendingTransaction); - private TransactionAddedResult addTransaction( + private TransactionAddedResult internalAddTransaction( final PendingTransaction pendingTransaction, final Optional maybeSenderAccount) { final Transaction transaction = pendingTransaction.getTransaction(); synchronized (lock) { @@ -540,9 +534,4 @@ public void signalInvalidAndRemoveDependentTransactions(final Transaction transa signalInvalidAndGetDependentTransactions(transaction).forEach(this::removeTransaction); } } - - @Override - public boolean isLocalSender(final Address sender) { - return poolConfig.getDisableLocalTransactions() ? false : localSenders.contains(sender); - } } diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/BaseFeePendingTransactionsSorter.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/BaseFeePendingTransactionsSorter.java index aeeb2b3e469..2ccb4d476ee 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/BaseFeePendingTransactionsSorter.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/BaseFeePendingTransactionsSorter.java @@ -63,7 +63,7 @@ public BaseFeePendingTransactionsSorter( */ private final NavigableSet prioritizedTransactionsStaticRange = new TreeSet<>( - comparing(PendingTransaction::isReceivedFromLocalSource) + comparing(PendingTransaction::hasPriority) .thenComparing( pendingTx -> pendingTx @@ -79,7 +79,7 @@ public BaseFeePendingTransactionsSorter( private final NavigableSet prioritizedTransactionsDynamicRange = new TreeSet<>( - comparing(PendingTransaction::isReceivedFromLocalSource) + comparing(PendingTransaction::hasPriority) .thenComparing( pendingTx -> pendingTx diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/GasPricePendingTransactionsSorter.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/GasPricePendingTransactionsSorter.java index 29b3406b059..ce8f8802592 100644 --- a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/GasPricePendingTransactionsSorter.java +++ b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/GasPricePendingTransactionsSorter.java @@ -37,7 +37,7 @@ public class GasPricePendingTransactionsSorter extends AbstractPendingTransactio private final NavigableSet prioritizedTransactions = new TreeSet<>( - comparing(PendingTransaction::isReceivedFromLocalSource) + comparing(PendingTransaction::hasPriority) .thenComparing(PendingTransaction::getGasPrice) .thenComparing(PendingTransaction::getAddedAt) .thenComparing(PendingTransaction::getSequence) diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java index b5843001053..d9c98f3a92b 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/AbstractTransactionPoolTest.java @@ -48,6 +48,7 @@ import org.hyperledger.besu.config.StubGenesisConfigOptions; import org.hyperledger.besu.crypto.KeyPair; import org.hyperledger.besu.crypto.SignatureAlgorithmFactory; +import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.TransactionType; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.ProtocolContext; @@ -296,39 +297,39 @@ private TransactionPool createTransactionPool( @ParameterizedTest @ValueSource(booleans = {true, false}) - public void localTransactionHappyPath(final boolean disableLocalTxs) { - this.transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + public void localTransactionHappyPath(final boolean noLocalPriority) { + this.transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); final Transaction transaction = createTransaction(0); givenTransactionIsValid(transaction); - addAndAssertTransactionViaApiValid(transaction, disableLocalTxs); + addAndAssertTransactionViaApiValid(transaction, noLocalPriority); } @ParameterizedTest @ValueSource(booleans = {true, false}) - public void shouldReturnLocalTransactionsWhenAppropriate(final boolean disableLocalTxs) { - this.transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + public void shouldReturnLocalTransactionsWhenAppropriate(final boolean noLocalPriority) { + this.transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); final Transaction localTransaction2 = createTransaction(2); givenTransactionIsValid(localTransaction2); givenTransactionIsValid(transaction0); givenTransactionIsValid(transaction1); - addAndAssertTransactionViaApiValid(localTransaction2, disableLocalTxs); - addAndAssertRemoteTransactionValid(transaction0); - addAndAssertRemoteTransactionValid(transaction1); + addAndAssertTransactionViaApiValid(localTransaction2, noLocalPriority); + addAndAssertRemoteTransactionsValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction1); assertThat(transactions.size()).isEqualTo(3); - List localTransactions = transactions.getLocalTransactions(); - assertThat(localTransactions.size()).isEqualTo(disableLocalTxs ? 0 : 1); + assertThat(transactions.getLocalTransactions()).contains(localTransaction2); + assertThat(transactions.getPriorityTransactions().size()).isEqualTo(noLocalPriority ? 0 : 1); } @Test public void shouldRemoveTransactionsFromPendingListWhenIncludedInBlockOnchain() { givenTransactionIsValid(transaction0); - addAndAssertRemoteTransactionValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction0); appendBlock(transaction0); @@ -340,8 +341,8 @@ public void shouldRemoveMultipleTransactionsAddedInOneBlock() { givenTransactionIsValid(transaction0); givenTransactionIsValid(transaction1); - addAndAssertRemoteTransactionValid(transaction0); - addAndAssertRemoteTransactionValid(transaction1); + addAndAssertRemoteTransactionsValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction1); appendBlock(transaction0, transaction1); @@ -354,7 +355,7 @@ public void shouldRemoveMultipleTransactionsAddedInOneBlock() { public void shouldIgnoreUnknownTransactionsThatAreAddedInABlock() { givenTransactionIsValid(transaction0); - addAndAssertRemoteTransactionValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction0); appendBlock(transaction0, transaction1); @@ -367,7 +368,7 @@ public void shouldIgnoreUnknownTransactionsThatAreAddedInABlock() { public void shouldNotRemovePendingTransactionsWhenABlockAddedToAFork() { givenTransactionIsValid(transaction0); - addAndAssertRemoteTransactionValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction0); final BlockHeader commonParent = getHeaderForCurrentChainHead(); final Block canonicalHead = appendBlock(Difficulty.of(1000), commonParent); @@ -383,8 +384,8 @@ public void shouldRemovePendingTransactionsFromAllBlocksOnAForkWhenItBecomesTheC givenTransactionIsValid(transaction0); givenTransactionIsValid(transaction1); - addAndAssertRemoteTransactionValid(transaction0); - addAndAssertRemoteTransactionValid(transaction1); + addAndAssertRemoteTransactionsValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction1); final BlockHeader commonParent = getHeaderForCurrentChainHead(); final Block originalChainHead = appendBlock(Difficulty.of(1000), commonParent); @@ -436,8 +437,8 @@ public void shouldNotReAddTransactionsThatAreInBothForksWhenReorgHappens() { givenTransactionIsValid(transaction0); givenTransactionIsValid(transaction1); - addAndAssertRemoteTransactionValid(transaction0); - addAndAssertRemoteTransactionValid(transaction1); + addAndAssertRemoteTransactionsValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction1); final BlockHeader commonParent = getHeaderForCurrentChainHead(); final Block originalFork1 = appendBlock(Difficulty.of(1000), commonParent, transaction0); @@ -462,8 +463,8 @@ public void shouldNotReAddBlobTxsWhenReorgHappens() { givenTransactionIsValid(transaction1); givenTransactionIsValid(transactionBlob); - addAndAssertRemoteTransactionValid(transaction0); - addAndAssertRemoteTransactionValid(transaction1); + addAndAssertRemoteTransactionsValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction1); addAndAssertRemoteTransactionInvalid(transactionBlob); final BlockHeader commonParent = getHeaderForCurrentChainHead(); @@ -493,17 +494,15 @@ public void shouldNotReAddBlobTxsWhenReorgHappens() { @ParameterizedTest @ValueSource(booleans = {true, false}) public void addLocalTransaction_strictReplayProtectionOn_txWithChainId_chainIdIsConfigured( - final boolean disableLocalTxs) { + final boolean noLocalPriority) { protocolSupportsTxReplayProtection(1337, true); transactionPool = createTransactionPool( - b -> - b.strictTransactionReplayProtectionEnabled(true) - .disableLocalTransactions(disableLocalTxs)); + b -> b.strictTransactionReplayProtectionEnabled(true).noLocalPriority(noLocalPriority)); final Transaction tx = createTransaction(1); givenTransactionIsValid(tx); - addAndAssertTransactionViaApiValid(tx, disableLocalTxs); + addAndAssertTransactionViaApiValid(tx, noLocalPriority); } @Test @@ -513,7 +512,7 @@ public void addRemoteTransactions_strictReplayProtectionOn_txWithChainId_chainId final Transaction tx = createTransaction(1); givenTransactionIsValid(tx); - addAndAssertRemoteTransactionValid(tx); + addAndAssertRemoteTransactionsValid(tx); } @Test @@ -525,6 +524,17 @@ public void shouldNotAddRemoteTransactionsWhenGasPriceBelowMinimum() { verifyNoMoreInteractions(transactionValidatorFactory); } + @Test + public void shouldAddRemotePriorityTransactionsWhenGasPriceBelowMinimum() { + final Transaction transaction = createTransaction(1, Wei.of(7)); + transactionPool = + createTransactionPool(b -> b.prioritySenders(Set.of(transaction.getSender()))); + + givenTransactionIsValid(transaction); + + addAndAssertRemotePriorityTransactionsValid(transaction); + } + @Test public void shouldNotAddRemoteTransactionsThatAreInvalidAccordingToStateDependentChecks() { givenTransactionIsValid(transaction0); @@ -551,8 +561,8 @@ public void shouldNotAddRemoteTransactionsThatAreInvalidAccordingToStateDependen @ParameterizedTest @ValueSource(booleans = {true, false}) public void shouldAllowSequenceOfTransactionsWithIncreasingNonceFromSameSender( - final boolean disableLocalTxs) { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + final boolean noLocalPriority) { + transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); final Transaction transaction1 = createTransaction(1); final Transaction transaction2 = createTransaction(2); final Transaction transaction3 = createTransaction(3); @@ -561,9 +571,9 @@ public void shouldAllowSequenceOfTransactionsWithIncreasingNonceFromSameSender( givenTransactionIsValid(transaction2); givenTransactionIsValid(transaction3); - addAndAssertTransactionViaApiValid(transaction1, disableLocalTxs); - addAndAssertTransactionViaApiValid(transaction2, disableLocalTxs); - addAndAssertTransactionViaApiValid(transaction3, disableLocalTxs); + addAndAssertTransactionViaApiValid(transaction1, noLocalPriority); + addAndAssertTransactionViaApiValid(transaction2, noLocalPriority); + addAndAssertTransactionViaApiValid(transaction3, noLocalPriority); } @Test @@ -575,9 +585,9 @@ public void shouldAllowSequenceOfTransactionsWithIncreasingNonceFromSameSender( givenTransactionIsValid(transaction1); givenTransactionIsValid(transaction2); - addAndAssertRemoteTransactionValid(transaction2); - addAndAssertRemoteTransactionValid(transaction0); - addAndAssertRemoteTransactionValid(transaction1); + addAndAssertRemoteTransactionsValid(transaction2); + addAndAssertRemoteTransactionsValid(transaction0); + addAndAssertRemoteTransactionsValid(transaction1); } @Test @@ -597,22 +607,22 @@ public void shouldNotNotifyBatchListenerWhenRemoteTransactionDoesNotReplaceExist givenTransactionIsValid(transaction0a); givenTransactionIsValid(transaction0b); - addAndAssertRemoteTransactionValid(transaction0a); + addAndAssertRemoteTransactionsValid(transaction0a); addAndAssertRemoteTransactionInvalid(transaction0b); } @ParameterizedTest @ValueSource(booleans = {true, false}) public void shouldNotNotifyBatchListenerWhenLocalTransactionDoesNotReplaceExisting( - final boolean disableLocalTxs) { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + final boolean noLocalPriority) { + transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); final Transaction transaction0a = createTransaction(0, Wei.of(10)); final Transaction transaction0b = createTransaction(0, Wei.of(9)); givenTransactionIsValid(transaction0a); givenTransactionIsValid(transaction0b); - addAndAssertTransactionViaApiValid(transaction0a, disableLocalTxs); + addAndAssertTransactionViaApiValid(transaction0a, noLocalPriority); addAndAssertTransactionViaApiInvalid(transaction0b, TRANSACTION_REPLACEMENT_UNDERPRICED); } @@ -638,7 +648,7 @@ public void shouldRejectRemoteTransactionsWhereGasLimitExceedBlockGasLimit() { @Test public void shouldAcceptLocalTransactionsEvenIfAnInvalidTransactionWithLowerNonceExists() { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(false)); + transactionPool = createTransactionPool(b -> b.noLocalPriority(false)); final Transaction invalidTx = createBaseTransaction(0).gasLimit(blockGasLimit + 1).createTransaction(KEY_PAIR1); @@ -653,8 +663,8 @@ public void shouldAcceptLocalTransactionsEvenIfAnInvalidTransactionWithLowerNonc @ParameterizedTest @ValueSource(booleans = {true, false}) - public void shouldRejectLocalTransactionsWhenNonceTooFarInFuture(final boolean disableLocalTxs) { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + public void shouldRejectLocalTransactionsWhenNonceTooFarInFuture(final boolean noLocalPriority) { + transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); final Transaction transactionFarFuture = createTransaction(Integer.MAX_VALUE); givenTransactionIsValid(transactionFarFuture); @@ -732,22 +742,23 @@ public void shouldCallValidatorWithExpectedValidationParameters() { @ParameterizedTest @ValueSource(booleans = {true, false}) - public void shouldIgnoreFeeCapIfSetZero(final boolean disableLocalTxs) { + public void shouldIgnoreFeeCapIfSetZero(final boolean noLocalPriority) { final Wei twoEthers = Wei.fromEth(2); transactionPool = - createTransactionPool(b -> b.txFeeCap(Wei.ZERO).disableLocalTransactions(disableLocalTxs)); + createTransactionPool(b -> b.txFeeCap(Wei.ZERO).noLocalPriority(noLocalPriority)); final Transaction transaction = createTransaction(0, twoEthers.add(Wei.of(1))); givenTransactionIsValid(transaction); - addAndAssertTransactionViaApiValid(transaction, disableLocalTxs); + addAndAssertTransactionViaApiValid(transaction, noLocalPriority); } - @Test - public void shouldRejectLocalTransactionIfFeeCapExceeded() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void shouldRejectLocalTransactionIfFeeCapExceeded(final boolean noLocalPriority) { final Wei twoEthers = Wei.fromEth(2); transactionPool = - createTransactionPool(b -> b.txFeeCap(twoEthers).disableLocalTransactions(false)); + createTransactionPool(b -> b.txFeeCap(twoEthers).noLocalPriority(noLocalPriority)); final Transaction transactionLocal = createTransaction(0, twoEthers.add(1)); @@ -758,8 +769,23 @@ public void shouldRejectLocalTransactionIfFeeCapExceeded() { @ParameterizedTest @ValueSource(booleans = {true, false}) - public void shouldRejectZeroGasPriceLocalTransactionWhenNotMining(final boolean disableLocalTxs) { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + public void shouldAcceptRemoteTransactionEvenIfFeeCapExceeded(final boolean hasPriority) { + final Wei twoEthers = Wei.fromEth(2); + final Transaction remoteTransaction = createTransaction(0, twoEthers.add(1)); + final Set
prioritySenders = + hasPriority ? Set.of(remoteTransaction.getSender()) : Set.of(); + transactionPool = + createTransactionPool(b -> b.txFeeCap(twoEthers).prioritySenders(prioritySenders)); + + givenTransactionIsValid(remoteTransaction); + + addAndAssertRemoteTransactionsValid(hasPriority, remoteTransaction); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void shouldRejectZeroGasPriceLocalTransactionWhenNotMining(final boolean noLocalPriority) { + transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); when(miningParameters.isMiningEnabled()).thenReturn(false); final Transaction transaction = createTransaction(0, Wei.ZERO); @@ -769,28 +795,67 @@ public void shouldRejectZeroGasPriceLocalTransactionWhenNotMining(final boolean addAndAssertTransactionViaApiInvalid(transaction, GAS_PRICE_TOO_LOW); } + @Test + @DisabledIf("isBaseFeeMarket") + public void shouldAcceptZeroGasPriceFrontierLocalPriorityTransactionsWhenMining() { + transactionPool = createTransactionPool(b -> b.noLocalPriority(false)); + when(miningParameters.isMiningEnabled()).thenReturn(true); + + final Transaction transaction = createTransaction(0, Wei.ZERO); + + givenTransactionIsValid(transaction); + + addAndAssertTransactionViaApiValid(transaction, false); + } + @ParameterizedTest @ValueSource(booleans = {true, false}) - public void transactionNotRejectedByPluginShouldBeAdded(final boolean disableLocalTxs) { + public void shouldRejectZeroGasPriceRemoteTransactionWhenNotMining(final boolean hasPriority) { + final Transaction transaction = createTransaction(0, Wei.ZERO); + final Set
prioritySenders = hasPriority ? Set.of(transaction.getSender()) : Set.of(); + transactionPool = createTransactionPool(b -> b.prioritySenders(prioritySenders)); + when(miningParameters.isMiningEnabled()).thenReturn(false); + + givenTransactionIsValid(transaction); + + addAndAssertRemoteTransactionInvalid(transaction); + } + + @Test + @DisabledIf("isBaseFeeMarket") + public void shouldAcceptZeroGasPriceFrontierRemotePriorityTransactionsWhenMining() { + final Transaction transaction = createTransaction(0, Wei.ZERO); + transactionPool = + createTransactionPool(b -> b.prioritySenders(Set.of(transaction.getSender()))); + when(miningParameters.isMiningEnabled()).thenReturn(true); + + givenTransactionIsValid(transaction); + + addAndAssertRemoteTransactionsValid(true, transaction); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void transactionNotRejectedByPluginShouldBeAdded(final boolean noLocalPriority) { final PluginTransactionValidatorFactory pluginTransactionValidatorFactory = getPluginTransactionValidatorFactoryReturning(null); // null -> not rejecting !! this.transactionPool = createTransactionPool( - b -> b.disableLocalTransactions(disableLocalTxs), pluginTransactionValidatorFactory); + b -> b.noLocalPriority(noLocalPriority), pluginTransactionValidatorFactory); givenTransactionIsValid(transaction0); - addAndAssertTransactionViaApiValid(transaction0, disableLocalTxs); + addAndAssertTransactionViaApiValid(transaction0, noLocalPriority); } @ParameterizedTest @ValueSource(booleans = {true, false}) - public void transactionRejectedByPluginShouldNotBeAdded(final boolean disableLocalTxs) { + public void transactionRejectedByPluginShouldNotBeAdded(final boolean noLocalPriority) { final PluginTransactionValidatorFactory pluginTransactionValidatorFactory = getPluginTransactionValidatorFactoryReturning("false"); this.transactionPool = createTransactionPool( - b -> b.disableLocalTransactions(disableLocalTxs), pluginTransactionValidatorFactory); + b -> b.noLocalPriority(noLocalPriority), pluginTransactionValidatorFactory); givenTransactionIsValid(transaction0); @@ -814,17 +879,15 @@ public void remoteTransactionRejectedByPluginShouldNotBeAdded() { @DisabledIf("isBaseFeeMarket") public void addLocalTransaction_strictReplayProtectionOn_txWithoutChainId_chainIdIsConfigured_protectionNotSupportedAtCurrentBlock( - final boolean disableLocalTxs) { + final boolean noLocalPriority) { protocolSupportsTxReplayProtection(1337, false); transactionPool = createTransactionPool( - b -> - b.strictTransactionReplayProtectionEnabled(true) - .disableLocalTransactions(disableLocalTxs)); + b -> b.strictTransactionReplayProtectionEnabled(true).noLocalPriority(noLocalPriority)); final Transaction tx = createTransactionWithoutChainId(1); givenTransactionIsValid(tx); - addAndAssertTransactionViaApiValid(tx, disableLocalTxs); + addAndAssertTransactionViaApiValid(tx, noLocalPriority); } @Test @@ -836,24 +899,23 @@ public void remoteTransactionRejectedByPluginShouldNotBeAdded() { final Transaction tx = createTransactionWithoutChainId(1); givenTransactionIsValid(tx); - addAndAssertRemoteTransactionValid(tx); + addAndAssertRemoteTransactionsValid(tx); } @ParameterizedTest @ValueSource(booleans = {true, false}) @DisabledIf("isBaseFeeMarket") public void addLocalTransaction_strictReplayProtectionOff_txWithoutChainId_chainIdIsConfigured( - final boolean disableLocalTxs) { + final boolean noLocalPriority) { protocolSupportsTxReplayProtection(1337, true); transactionPool = createTransactionPool( b -> - b.strictTransactionReplayProtectionEnabled(false) - .disableLocalTransactions(disableLocalTxs)); + b.strictTransactionReplayProtectionEnabled(false).noLocalPriority(noLocalPriority)); final Transaction tx = createTransactionWithoutChainId(1); givenTransactionIsValid(tx); - addAndAssertTransactionViaApiValid(tx, disableLocalTxs); + addAndAssertTransactionViaApiValid(tx, noLocalPriority); } @Test @@ -876,24 +938,22 @@ public void addLocalTransaction_strictReplayProtectionOn_txWithoutChainId_chainI final Transaction tx = createTransactionWithoutChainId(1); givenTransactionIsValid(tx); - addAndAssertRemoteTransactionValid(tx); + addAndAssertRemoteTransactionsValid(tx); } @ParameterizedTest @ValueSource(booleans = {true, false}) @DisabledIf("isBaseFeeMarket") public void addLocalTransaction_strictReplayProtectionOn_txWithoutChainId_chainIdIsNotConfigured( - final boolean disableLocalTxs) { + final boolean noLocalPriority) { protocolDoesNotSupportTxReplayProtection(); transactionPool = createTransactionPool( - b -> - b.strictTransactionReplayProtectionEnabled(true) - .disableLocalTransactions(disableLocalTxs)); + b -> b.strictTransactionReplayProtectionEnabled(true).noLocalPriority(noLocalPriority)); final Transaction tx = createTransactionWithoutChainId(1); givenTransactionIsValid(tx); - addAndAssertTransactionViaApiValid(tx, disableLocalTxs); + addAndAssertTransactionViaApiValid(tx, noLocalPriority); } @Test @@ -905,7 +965,7 @@ public void addLocalTransaction_strictReplayProtectionOn_txWithoutChainId_chainI final Transaction tx = createTransactionWithoutChainId(1); givenTransactionIsValid(tx); - addAndAssertRemoteTransactionValid(tx); + addAndAssertRemoteTransactionsValid(tx); } @Test @@ -925,39 +985,26 @@ public void shouldIgnoreEIP1559TransactionWhenNotAllowed() { addAndAssertTransactionViaApiInvalid(transaction, INVALID_TRANSACTION_FORMAT); } - @Test - @DisabledIf("isBaseFeeMarket") - public void shouldAcceptZeroGasPriceFrontierLocalTransactionsWhenMining() { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(false)); - when(miningParameters.isMiningEnabled()).thenReturn(true); - - final Transaction transaction = createTransaction(0, Wei.ZERO); - - givenTransactionIsValid(transaction); - - addAndAssertTransactionViaApiValid(transaction, false); - } - @ParameterizedTest @ValueSource(booleans = {true, false}) @DisabledIf("isBaseFeeMarket") public void shouldAcceptZeroGasPriceTransactionWhenMinGasPriceIsZero( - final boolean disableLocalTxs) { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + final boolean noLocalPriority) { + transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); final Transaction transaction = createTransaction(0, Wei.ZERO); givenTransactionIsValid(transaction); - addAndAssertTransactionViaApiValid(transaction, disableLocalTxs); + addAndAssertTransactionViaApiValid(transaction, noLocalPriority); } @ParameterizedTest @ValueSource(booleans = {true, false}) public void shouldAcceptZeroGasPriceFrontierTxsWhenMinGasPriceIsZeroAndLondonWithZeroBaseFee( - final boolean disableLocalTxs) { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + final boolean noLocalPriority) { + transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); when(protocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0, Optional.of(Wei.ZERO))); whenBlockBaseFeeIs(Wei.ZERO); @@ -965,14 +1012,14 @@ public void shouldAcceptZeroGasPriceFrontierTxsWhenMinGasPriceIsZeroAndLondonWit final Transaction frontierTransaction = createFrontierTransaction(0, Wei.ZERO); givenTransactionIsValid(frontierTransaction); - addAndAssertTransactionViaApiValid(frontierTransaction, disableLocalTxs); + addAndAssertTransactionViaApiValid(frontierTransaction, noLocalPriority); } @ParameterizedTest @ValueSource(booleans = {true, false}) public void shouldAcceptZeroGasPrice1559TxsWhenMinGasPriceIsZeroAndLondonWithZeroBaseFee( - final boolean disableLocalTxs) { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(disableLocalTxs)); + final boolean noLocalPriority) { + transactionPool = createTransactionPool(b -> b.noLocalPriority(noLocalPriority)); when(miningParameters.getMinTransactionGasPrice()).thenReturn(Wei.ZERO); when(protocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0, Optional.of(Wei.ZERO))); whenBlockBaseFeeIs(Wei.ZERO); @@ -980,12 +1027,12 @@ public void shouldAcceptZeroGasPrice1559TxsWhenMinGasPriceIsZeroAndLondonWithZer final Transaction transaction = createTransaction(0, Wei.ZERO); givenTransactionIsValid(transaction); - addAndAssertTransactionViaApiValid(transaction, disableLocalTxs); + addAndAssertTransactionViaApiValid(transaction, noLocalPriority); } @Test - public void shouldAcceptBaseFeeFloorGasPriceFrontierLocalTransactionsWhenMining() { - transactionPool = createTransactionPool(b -> b.disableLocalTransactions(false)); + public void shouldAcceptBaseFeeFloorGasPriceFrontierLocalPriorityTransactionsWhenMining() { + transactionPool = createTransactionPool(b -> b.noLocalPriority(false)); final Transaction frontierTransaction = createFrontierTransaction(0, BASE_FEE_FLOOR); givenTransactionIsValid(frontierTransaction); @@ -994,7 +1041,19 @@ public void shouldAcceptBaseFeeFloorGasPriceFrontierLocalTransactionsWhenMining( } @Test - public void shouldRejectRemote1559TxsWhenMaxFeePerGasBelowMinGasPrice() { + public void shouldAcceptBaseFeeFloorGasPriceFrontierRemotePriorityTransactionsWhenMining() { + final Transaction frontierTransaction = createFrontierTransaction(0, BASE_FEE_FLOOR); + transactionPool = + createTransactionPool(b -> b.prioritySenders(Set.of(frontierTransaction.getSender()))); + + givenTransactionIsValid(frontierTransaction); + + addAndAssertRemoteTransactionsValid(frontierTransaction); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void shouldRejectRemote1559TxsWhenMaxFeePerGasBelowMinGasPrice(final boolean hasPriority) { final Wei genesisBaseFee = Wei.of(100L); final Wei minGasPrice = Wei.of(200L); final Wei lastBlockBaseFee = minGasPrice.add(50L); @@ -1002,12 +1061,14 @@ public void shouldRejectRemote1559TxsWhenMaxFeePerGasBelowMinGasPrice() { assertThat( add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, false)) + genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, false, hasPriority)) .isEqualTo(0); } - @Test - public void shouldAcceptRemote1559TxsWhenMaxFeePerGasIsAtLeastEqualToMinGasPrice() { + @ParameterizedTest + @ValueSource(booleans = {true, false}) + public void shouldAcceptRemote1559TxsWhenMaxFeePerGasIsAtLeastEqualToMinGasPrice( + final boolean hasPriority) { final Wei genesisBaseFee = Wei.of(100L); final Wei minGasPrice = Wei.of(200L); final Wei lastBlockBaseFee = minGasPrice.add(50L); @@ -1015,7 +1076,7 @@ public void shouldAcceptRemote1559TxsWhenMaxFeePerGasIsAtLeastEqualToMinGasPrice assertThat( add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, false)) + genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, false, hasPriority)) .isEqualTo(1); } @@ -1028,7 +1089,7 @@ public void shouldRejectLocal1559TxsWhenMaxFeePerGasBelowMinGasPrice() { assertThat( add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, true)) + genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, true, true)) .isEqualTo(0); } @@ -1041,7 +1102,7 @@ public void shouldAcceptLocal1559TxsWhenMaxFeePerGasIsAtLeastEqualToMinMinGasPri assertThat( add1559TxAndGetPendingTxsCount( - genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, true)) + genesisBaseFee, minGasPrice, lastBlockBaseFee, txMaxFeePerGas, true, true)) .isEqualTo(1); } @@ -1094,7 +1155,16 @@ protected void assertTransactionPending(final Transaction t) { assertThat(transactions.getTransactionByHash(t.getHash())).contains(t); } - protected void addAndAssertRemoteTransactionValid(final Transaction... txs) { + protected void addAndAssertRemoteTransactionsValid(final Transaction... txs) { + addAndAssertRemoteTransactionsValid(false, txs); + } + + protected void addAndAssertRemotePriorityTransactionsValid(final Transaction... txs) { + addAndAssertRemoteTransactionsValid(true, txs); + } + + protected void addAndAssertRemoteTransactionsValid( + final boolean hasPriority, final Transaction... txs) { transactionPool.addRemoteTransactions(List.of(txs)); verify(transactionBroadcaster) @@ -1102,24 +1172,24 @@ protected void addAndAssertRemoteTransactionValid(final Transaction... txs) { argThat(btxs -> btxs.size() == txs.length && btxs.containsAll(List.of(txs)))); Arrays.stream(txs).forEach(this::assertTransactionPending); assertThat(transactions.getLocalTransactions()).doesNotContain(txs); - } - - protected void addAndAssertTransactionViaApiValid(final Transaction tx) { - addAndAssertTransactionViaApiValid(tx, false); + if (hasPriority) { + assertThat(transactions.getPriorityTransactions()).contains(txs); + } } protected void addAndAssertTransactionViaApiValid( - final Transaction tx, final boolean disableLocals) { + final Transaction tx, final boolean disableLocalPriority) { final ValidationResult result = transactionPool.addTransactionViaApi(tx); assertThat(result.isValid()).isTrue(); assertTransactionPending(tx); verify(transactionBroadcaster).onTransactionsAdded(singletonList(tx)); - if (disableLocals) { - assertThat(transactions.getLocalTransactions()).doesNotContain(tx); + assertThat(transactions.getLocalTransactions()).contains(tx); + if (disableLocalPriority) { + assertThat(transactions.getPriorityTransactions()).doesNotContain(tx); } else { - assertThat(transactions.getLocalTransactions()).contains(tx); + assertThat(transactions.getPriorityTransactions()).contains(tx); } } @@ -1243,13 +1313,17 @@ protected int add1559TxAndGetPendingTxsCount( final Wei minGasPrice, final Wei lastBlockBaseFee, final Wei txMaxFeePerGas, - final boolean isLocal) { + final boolean isLocal, + final boolean hasPriority) { when(miningParameters.getMinTransactionGasPrice()).thenReturn(minGasPrice); when(protocolSpec.getFeeMarket()).thenReturn(FeeMarket.london(0, Optional.of(genesisBaseFee))); whenBlockBaseFeeIs(lastBlockBaseFee); final Transaction transaction = createTransaction(0, txMaxFeePerGas); - + if (hasPriority) { + transactionPool = + createTransactionPool(b -> b.prioritySenders(Set.of(transaction.getSender()))); + } givenTransactionIsValid(transaction); if (isLocal) { diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseTransactionPoolTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseTransactionPoolTest.java index 4cf6a3bd264..0d69dac4faa 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseTransactionPoolTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/BaseTransactionPoolTest.java @@ -183,6 +183,11 @@ protected PendingTransaction createRemotePendingTransaction(final Transaction tr return new PendingTransaction.Remote(transaction); } + protected PendingTransaction createRemotePendingTransaction( + final Transaction transaction, final boolean hasPriority) { + return PendingTransaction.newPendingTransaction(transaction, false, hasPriority); + } + protected PendingTransaction createLocalPendingTransaction(final Transaction transaction) { return new PendingTransaction.Local(transaction); } @@ -210,16 +215,19 @@ protected void assertNextNonceForSender( protected void addLocalTransactions( final PendingTransactions sorter, final Account sender, final long... nonces) { for (final long nonce : nonces) { - sorter.addLocalTransaction(createTransaction(nonce), Optional.of(sender)); + sorter.addTransaction( + createLocalPendingTransaction(createTransaction(nonce)), Optional.of(sender)); } } - protected long getAddedCount(final String source, final String layer) { - return metricsSystem.getCounterValue(TransactionPoolMetrics.ADDED_COUNTER_NAME, source, layer); + protected long getAddedCount(final String source, final String priority, final String layer) { + return metricsSystem.getCounterValue( + TransactionPoolMetrics.ADDED_COUNTER_NAME, source, priority, layer); } - protected long getRemovedCount(final String source, final String operation, final String layer) { + protected long getRemovedCount( + final String source, final String priority, final String operation, final String layer) { return metricsSystem.getCounterValue( - TransactionPoolMetrics.REMOVED_COUNTER_NAME, source, operation, layer); + TransactionPoolMetrics.REMOVED_COUNTER_NAME, source, priority, operation, layer); } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java index 7d044de859a..c878184370a 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayeredPendingTransactionsTest.java @@ -71,6 +71,7 @@ public class LayeredPendingTransactionsTest extends BaseTransactionPoolTest { protected static final int LIMITED_TRANSACTIONS_BY_SENDER = 4; protected static final String REMOTE = "remote"; protected static final String LOCAL = "local"; + protected static final String NO_PRIORITY = "no"; protected final PendingTransactionAddedListener listener = mock(PendingTransactionAddedListener.class); protected final PendingTransactionDroppedListener droppedListener = @@ -148,13 +149,16 @@ public void setup() { @Test public void returnExclusivelyLocalTransactionsWhenAppropriate() { final Transaction localTransaction0 = createTransaction(0, KEYS2); - pendingTransactions.addLocalTransaction(localTransaction0, Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(localTransaction0), Optional.empty()); assertThat(pendingTransactions.size()).isEqualTo(1); - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); assertThat(pendingTransactions.size()).isEqualTo(2); - pendingTransactions.addRemoteTransaction(transaction1, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty()); assertThat(pendingTransactions.size()).isEqualTo(3); final List localTransactions = pendingTransactions.getLocalTransactions(); @@ -163,15 +167,19 @@ public void returnExclusivelyLocalTransactionsWhenAppropriate() { @Test public void addRemoteTransactions() { - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); assertThat(pendingTransactions.size()).isEqualTo(1); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())).isEqualTo(1); + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())) + .isEqualTo(1); - pendingTransactions.addRemoteTransaction(transaction1, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty()); assertThat(pendingTransactions.size()).isEqualTo(2); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())).isEqualTo(2); + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())) + .isEqualTo(2); } @Test @@ -181,7 +189,8 @@ public void getNotPresentTransaction() { @Test public void getTransactionByHash() { - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); assertTransactionPending(pendingTransactions, transaction0); } @@ -200,7 +209,7 @@ public void evictTransactionsWhenSizeLimitExceeded() { Wei.of((i + 1) * 100L), (int) poolConf.getPendingTransactionsLayerMaxCapacityBytes() + 1, SIGNATURE_ALGORITHM.get().generateKeyPair()); - pendingTransactions.addRemoteTransaction(tx, Optional.of(sender)); + pendingTransactions.addTransaction(createRemotePendingTransaction(tx), Optional.of(sender)); firstTxs.add(tx); assertTransactionPending(pendingTransactions, tx); } @@ -215,11 +224,13 @@ public void evictTransactionsWhenSizeLimitExceeded() { SIGNATURE_ALGORITHM.get().generateKeyPair()); final Account lastSender = mock(Account.class); when(lastSender.getNonce()).thenReturn(0L); - pendingTransactions.addRemoteTransaction(lastBigTx, Optional.of(lastSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(lastBigTx), Optional.of(lastSender)); assertTransactionPending(pendingTransactions, lastBigTx); assertTransactionNotPending(pendingTransactions, firstTxs.get(0)); - assertThat(getRemovedCount(REMOTE, DROPPED.label(), layers.evictedCollector.name())) + assertThat( + getRemovedCount(REMOTE, NO_PRIORITY, DROPPED.label(), layers.evictedCollector.name())) .isEqualTo(1); assertThat(layers.evictedCollector.getEvictedTransactions()) .map(PendingTransaction::getTransaction) @@ -231,10 +242,14 @@ public void evictTransactionsWhenSizeLimitExceeded() { public void addTransactionForMultipleSenders() { final var transactionSenderA = createTransaction(0, KEYS1); final var transactionSenderB = createTransaction(0, KEYS2); - assertThat(pendingTransactions.addRemoteTransaction(transactionSenderA, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(transactionSenderA), Optional.empty())) .isEqualTo(ADDED); assertTransactionPending(pendingTransactions, transactionSenderA); - assertThat(pendingTransactions.addRemoteTransaction(transactionSenderB, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(transactionSenderB), Optional.empty())) .isEqualTo(ADDED); assertTransactionPending(pendingTransactions, transactionSenderB); } @@ -243,7 +258,9 @@ public void addTransactionForMultipleSenders() { public void dropIfTransactionTooFarInFutureForTheSender() { final var futureTransaction = createTransaction(poolConf.getTxPoolMaxFutureTransactionByAccount() + 1); - assertThat(pendingTransactions.addRemoteTransaction(futureTransaction, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(futureTransaction), Optional.empty())) .isEqualTo(NONCE_TOO_FAR_IN_FUTURE_FOR_SENDER); assertTransactionNotPending(pendingTransactions, futureTransaction); } @@ -254,7 +271,9 @@ public void dropAlreadyConfirmedTransaction() { when(sender.getNonce()).thenReturn(5L); final Transaction oldTransaction = createTransaction(2); - assertThat(pendingTransactions.addRemoteTransaction(oldTransaction, Optional.of(sender))) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(oldTransaction), Optional.of(sender))) .isEqualTo(ALREADY_KNOWN); assertThat(pendingTransactions.size()).isEqualTo(0); assertTransactionNotPending(pendingTransactions, oldTransaction); @@ -264,7 +283,8 @@ public void dropAlreadyConfirmedTransaction() { public void notifyListenerWhenRemoteTransactionAdded() { pendingTransactions.subscribePendingTransactions(listener); - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); verify(listener).onTransactionAdded(transaction0); } @@ -273,7 +293,8 @@ public void notifyListenerWhenRemoteTransactionAdded() { public void notifyListenerWhenLocalTransactionAdded() { pendingTransactions.subscribePendingTransactions(listener); - pendingTransactions.addLocalTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction0), Optional.empty()); verify(listener).onTransactionAdded(transaction0); } @@ -282,13 +303,15 @@ public void notifyListenerWhenLocalTransactionAdded() { public void notNotifyListenerAfterUnsubscribe() { final long id = pendingTransactions.subscribePendingTransactions(listener); - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); verify(listener).onTransactionAdded(transaction0); pendingTransactions.unsubscribePendingTransactions(id); - pendingTransactions.addRemoteTransaction(transaction1, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty()); verifyNoMoreInteractions(listener); } @@ -297,13 +320,15 @@ public void notNotifyListenerAfterUnsubscribe() { @MethodSource public void selectTransactionsUntilSelectorRequestsNoMore( final TransactionSelectionResult selectionResult) { - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); - pendingTransactions.addRemoteTransaction(transaction1, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty()); final List parsedTransactions = new ArrayList<>(); pendingTransactions.selectTransactions( - pendingTX -> { - parsedTransactions.add(pendingTX.getTransaction()); + pendingTx -> { + parsedTransactions.add(pendingTx.getTransaction()); return selectionResult; }); @@ -317,8 +342,10 @@ static Stream selectTransactionsUntilSelectorRequest @Test public void selectTransactionsUntilPendingIsEmpty() { - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); - pendingTransactions.addRemoteTransaction(transaction1, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty()); final List parsedTransactions = new ArrayList<>(); pendingTransactions.selectTransactions( @@ -337,8 +364,10 @@ public void notSelectReplacedTransaction() { final Transaction transaction1 = createTransaction(0, KEYS1); final Transaction transaction1b = createTransactionReplacement(transaction1, KEYS1); - pendingTransactions.addRemoteTransaction(transaction1, Optional.empty()); - pendingTransactions.addRemoteTransaction(transaction1b, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1b), Optional.empty()); final List parsedTransactions = new ArrayList<>(); pendingTransactions.selectTransactions( @@ -357,9 +386,12 @@ public void selectTransactionsFromSameSenderInNonceOrder() { final Transaction transaction2 = createTransaction(2, KEYS1); // add out of order - pendingTransactions.addLocalTransaction(transaction2, Optional.empty()); - pendingTransactions.addLocalTransaction(transaction1, Optional.empty()); - pendingTransactions.addLocalTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction2), Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction1), Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction0), Optional.empty()); final List iterationOrder = new ArrayList<>(3); pendingTransactions.selectTransactions( @@ -380,10 +412,14 @@ public void ignoreSenderTransactionsAfterASkippedOne( final Transaction transaction2a = createTransaction(2, DEFAULT_BASE_FEE.add(Wei.of(20)), KEYS1); final Transaction transaction0b = createTransaction(0, DEFAULT_BASE_FEE.add(Wei.of(10)), KEYS2); - pendingTransactions.addLocalTransaction(transaction0a, Optional.empty()); - pendingTransactions.addLocalTransaction(transaction1a, Optional.empty()); - pendingTransactions.addLocalTransaction(transaction2a, Optional.empty()); - pendingTransactions.addLocalTransaction(transaction0b, Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction0a), Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction1a), Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction2a), Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction0b), Optional.empty()); final List iterationOrder = new ArrayList<>(3); pendingTransactions.selectTransactions( @@ -415,8 +451,10 @@ public void notForceNonceOrderWhenSendersDiffer() { final Transaction transactionSender1 = createTransaction(0, Wei.of(100), KEYS1); final Transaction transactionSender2 = createTransaction(1, Wei.of(200), KEYS2); - pendingTransactions.addLocalTransaction(transactionSender1, Optional.empty()); - pendingTransactions.addLocalTransaction(transactionSender2, Optional.of(sender2)); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transactionSender1), Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transactionSender2), Optional.of(sender2)); final List iterationOrder = new ArrayList<>(2); pendingTransactions.selectTransactions( @@ -430,38 +468,39 @@ public void notForceNonceOrderWhenSendersDiffer() { @Test public void invalidTransactionIsDeletedFromPendingTransactions() { - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); - pendingTransactions.addRemoteTransaction(transaction1, Optional.empty()); + final var pendingTx0 = createRemotePendingTransaction(transaction0); + final var pendingTx1 = createRemotePendingTransaction(transaction1); + pendingTransactions.addTransaction(pendingTx0, Optional.empty()); + pendingTransactions.addTransaction(pendingTx1, Optional.empty()); - final List parsedTransactions = new ArrayList<>(1); + final List parsedTransactions = new ArrayList<>(1); pendingTransactions.selectTransactions( pendingTx -> { - parsedTransactions.add(pendingTx.getTransaction()); + parsedTransactions.add(pendingTx); return TransactionSelectionResult.invalid(UPFRONT_COST_EXCEEDS_BALANCE.name()); }); // only the first is processed since not being selected will automatically skip the processing // all the other txs from the same sender - assertThat(parsedTransactions).containsExactly(transaction0); - assertThat(pendingTransactions.getPendingTransactions()) - .map(PendingTransaction::getTransaction) - .containsExactly(transaction1); + assertThat(parsedTransactions).containsExactly(pendingTx0); + assertThat(pendingTransactions.getPendingTransactions()).containsExactly(pendingTx1); } @Test public void temporarilyInvalidTransactionIsKeptInPendingTransactions() { - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); + final var pendingTx0 = createRemotePendingTransaction(transaction0); + pendingTransactions.addTransaction(pendingTx0, Optional.empty()); - final List parsedTransactions = new ArrayList<>(1); + final List parsedTransactions = new ArrayList<>(1); pendingTransactions.selectTransactions( pendingTx -> { - parsedTransactions.add(pendingTx.getTransaction()); + parsedTransactions.add(pendingTx); return TransactionSelectionResult.invalidTransient( GAS_PRICE_BELOW_CURRENT_BASE_FEE.name()); }); - assertThat(parsedTransactions).containsExactly(transaction0); + assertThat(parsedTransactions).containsExactly(pendingTx0); assertThat(pendingTransactions.getPendingTransactions()) .map(PendingTransaction::getTransaction) .containsExactly(transaction0); @@ -477,13 +516,17 @@ public void replaceTransactionWithSameSenderAndNonce() { final Transaction transaction1 = createTransaction(0, Wei.of(200), KEYS1); final Transaction transaction1b = createTransactionReplacement(transaction1, KEYS1); final Transaction transaction2 = createTransaction(1, Wei.of(100), KEYS1); - assertThat(pendingTransactions.addRemoteTransaction(transaction1, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty())) .isEqualTo(ADDED); - assertThat(pendingTransactions.addRemoteTransaction(transaction2, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction2), Optional.empty())) .isEqualTo(ADDED); assertThat( pendingTransactions - .addRemoteTransaction(transaction1b, Optional.empty()) + .addTransaction(createRemotePendingTransaction(transaction1b), Optional.empty()) .isReplacement()) .isTrue(); @@ -491,8 +534,11 @@ public void replaceTransactionWithSameSenderAndNonce() { assertTransactionPending(pendingTransactions, transaction1b); assertTransactionPending(pendingTransactions, transaction2); assertThat(pendingTransactions.size()).isEqualTo(2); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())).isEqualTo(3); - assertThat(getRemovedCount(REMOTE, REPLACED.label(), layers.prioritizedTransactions.name())) + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())) + .isEqualTo(3); + assertThat( + getRemovedCount( + REMOTE, NO_PRIORITY, REPLACED.label(), layers.prioritizedTransactions.name())) .isEqualTo(1); } @@ -503,15 +549,20 @@ public void replaceTransactionWithSameSenderAndNonce_multipleReplacements() { Transaction duplicateTx = createTransaction(0, DEFAULT_BASE_FEE.add(Wei.of(50)), KEYS1); for (int i = 0; i < replacedTxCount; i++) { replacedTransactions.add(duplicateTx); - pendingTransactions.addRemoteTransaction(duplicateTx, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(duplicateTx), Optional.empty()); duplicateTx = createTransactionReplacement(duplicateTx, KEYS1); } final Transaction independentTx = createTransaction(1, DEFAULT_BASE_FEE.add(Wei.ONE), KEYS1); - assertThat(pendingTransactions.addRemoteTransaction(independentTx, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(independentTx), Optional.empty())) .isEqualTo(ADDED); assertThat( - pendingTransactions.addRemoteTransaction(duplicateTx, Optional.empty()).isReplacement()) + pendingTransactions + .addTransaction(createRemotePendingTransaction(duplicateTx), Optional.empty()) + .isReplacement()) .isTrue(); // All txs except the last duplicate should be removed @@ -521,9 +572,11 @@ public void replaceTransactionWithSameSenderAndNonce_multipleReplacements() { assertTransactionPending(pendingTransactions, independentTx); assertThat(pendingTransactions.size()).isEqualTo(2); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())) + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())) .isEqualTo(replacedTxCount + 2); - assertThat(getRemovedCount(REMOTE, REPLACED.label(), layers.prioritizedTransactions.name())) + assertThat( + getRemovedCount( + REMOTE, NO_PRIORITY, REPLACED.label(), layers.prioritizedTransactions.name())) .isEqualTo(replacedTxCount); } @@ -537,19 +590,25 @@ public void replaceTransactionWithSameSenderAndNonce_multipleReplacements() { for (int i = 0; i < replacedTxCount; i++) { replacedTransactions.add(replacingTx); if (i % 2 == 0) { - pendingTransactions.addRemoteTransaction(replacingTx, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(replacingTx), Optional.empty()); remoteDuplicateCount++; } else { - pendingTransactions.addLocalTransaction(replacingTx, Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(replacingTx), Optional.empty()); } replacingTx = createTransactionReplacement(replacingTx, KEYS1); } final Transaction independentTx = createTransaction(1); assertThat( - pendingTransactions.addLocalTransaction(replacingTx, Optional.empty()).isReplacement()) + pendingTransactions + .addTransaction(createLocalPendingTransaction(replacingTx), Optional.empty()) + .isReplacement()) .isTrue(); - assertThat(pendingTransactions.addRemoteTransaction(independentTx, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(independentTx), Optional.empty())) .isEqualTo(ADDED); // All txs except the last duplicate should be removed @@ -561,13 +620,17 @@ public void replaceTransactionWithSameSenderAndNonce_multipleReplacements() { final int localDuplicateCount = replacedTxCount - remoteDuplicateCount; assertThat(pendingTransactions.size()).isEqualTo(2); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())) + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())) .isEqualTo(remoteDuplicateCount + 1); - assertThat(getAddedCount(LOCAL, layers.prioritizedTransactions.name())) + assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name())) .isEqualTo(localDuplicateCount + 1); - assertThat(getRemovedCount(REMOTE, REPLACED.label(), layers.prioritizedTransactions.name())) + assertThat( + getRemovedCount( + REMOTE, NO_PRIORITY, REPLACED.label(), layers.prioritizedTransactions.name())) .isEqualTo(remoteDuplicateCount); - assertThat(getRemovedCount(LOCAL, REPLACED.label(), layers.prioritizedTransactions.name())) + assertThat( + getRemovedCount( + LOCAL, NO_PRIORITY, REPLACED.label(), layers.prioritizedTransactions.name())) .isEqualTo(localDuplicateCount); } @@ -575,11 +638,15 @@ public void replaceTransactionWithSameSenderAndNonce_multipleReplacements() { public void notReplaceTransactionWithSameSenderAndNonceWhenGasPriceIsLower() { final Transaction transaction1 = createTransaction(0, Wei.of(2)); final Transaction transaction1b = createTransaction(0, Wei.ONE); - assertThat(pendingTransactions.addRemoteTransaction(transaction1, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty())) .isEqualTo(ADDED); pendingTransactions.subscribePendingTransactions(listener); - assertThat(pendingTransactions.addRemoteTransaction(transaction1b, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction1b), Optional.empty())) .isEqualTo(REJECTED_UNDERPRICED_REPLACEMENT); assertTransactionNotPending(pendingTransactions, transaction1b); @@ -595,13 +662,16 @@ public void trackNextNonceForEachSender() { when(firstSender.getNonce()).thenReturn(0L); when(firstSender.getAddress()).thenReturn(SENDER1); assertNoNextNonceForSender(pendingTransactions, SENDER1); - pendingTransactions.addRemoteTransaction(createTransaction(0, KEYS1), Optional.of(firstSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(0, KEYS1)), Optional.of(firstSender)); assertNextNonceForSender(pendingTransactions, SENDER1, 1); - pendingTransactions.addRemoteTransaction(createTransaction(1, KEYS1), Optional.of(firstSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(1, KEYS1)), Optional.of(firstSender)); assertNextNonceForSender(pendingTransactions, SENDER1, 2); - pendingTransactions.addRemoteTransaction(createTransaction(2, KEYS1), Optional.of(firstSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(2, KEYS1)), Optional.of(firstSender)); assertNextNonceForSender(pendingTransactions, SENDER1, 3); // second sender not in orders: 3->0->2->1 @@ -609,21 +679,21 @@ public void trackNextNonceForEachSender() { when(secondSender.getNonce()).thenReturn(0L); when(secondSender.getAddress()).thenReturn(SENDER2); assertNoNextNonceForSender(pendingTransactions, SENDER2); - pendingTransactions.addRemoteTransaction( - createTransaction(3, KEYS2), Optional.of(secondSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(3, KEYS2)), Optional.of(secondSender)); assertNoNextNonceForSender(pendingTransactions, SENDER2); - pendingTransactions.addRemoteTransaction( - createTransaction(0, KEYS2), Optional.of(secondSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(0, KEYS2)), Optional.of(secondSender)); assertNextNonceForSender(pendingTransactions, SENDER2, 1); - pendingTransactions.addRemoteTransaction( - createTransaction(2, KEYS2), Optional.of(secondSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(2, KEYS2)), Optional.of(secondSender)); assertNextNonceForSender(pendingTransactions, SENDER2, 1); // tx 1 will fill the nonce gap and all txs will be ready - pendingTransactions.addRemoteTransaction( - createTransaction(1, KEYS2), Optional.of(secondSender)); + pendingTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(1, KEYS2)), Optional.of(secondSender)); assertNextNonceForSender(pendingTransactions, SENDER2, 4); } @@ -701,38 +771,49 @@ public void correctNonceIsReturnedWithRepeatedTransactions() { @Test public void shouldNotIncrementAddedCounterWhenRemoteTransactionAlreadyPresent() { - pendingTransactions.addLocalTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction0), Optional.empty()); assertThat(pendingTransactions.size()).isEqualTo(1); - assertThat(getAddedCount(LOCAL, layers.prioritizedTransactions.name())).isEqualTo(1); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())).isZero(); + assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name())) + .isEqualTo(1); + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero(); - assertThat(pendingTransactions.addRemoteTransaction(transaction0, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty())) .isEqualTo(ALREADY_KNOWN); assertThat(pendingTransactions.size()).isEqualTo(1); - assertThat(getAddedCount(LOCAL, layers.prioritizedTransactions.name())).isEqualTo(1); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())).isZero(); + assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name())) + .isEqualTo(1); + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero(); } @Test public void shouldNotIncrementAddedCounterWhenLocalTransactionAlreadyPresent() { - pendingTransactions.addRemoteTransaction(transaction0, Optional.empty()); + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction0), Optional.empty()); assertThat(pendingTransactions.size()).isEqualTo(1); - assertThat(getAddedCount(LOCAL, layers.prioritizedTransactions.name())).isZero(); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())).isEqualTo(1); + assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero(); + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())) + .isEqualTo(1); - assertThat(pendingTransactions.addLocalTransaction(transaction0, Optional.empty())) + assertThat( + pendingTransactions.addTransaction( + createLocalPendingTransaction(transaction0), Optional.empty())) .isEqualTo(ALREADY_KNOWN); assertThat(pendingTransactions.size()).isEqualTo(1); - assertThat(getAddedCount(LOCAL, layers.prioritizedTransactions.name())).isZero(); - assertThat(getAddedCount(REMOTE, layers.prioritizedTransactions.name())).isEqualTo(1); + assertThat(getAddedCount(LOCAL, NO_PRIORITY, layers.prioritizedTransactions.name())).isZero(); + assertThat(getAddedCount(REMOTE, NO_PRIORITY, layers.prioritizedTransactions.name())) + .isEqualTo(1); } @Test public void doNothingIfTransactionAlreadyPending() { final var addedTxs = populateCache(1, 0); assertThat( - pendingTransactions.addRemoteTransaction( - addedTxs[0].transaction, Optional.of(addedTxs[0].account))) + pendingTransactions.addTransaction( + createRemotePendingTransaction(addedTxs[0].transaction), + Optional.of(addedTxs[0].account))) .isEqualTo(ALREADY_KNOWN); assertTransactionPending(pendingTransactions, addedTxs[0].transaction); } @@ -766,7 +847,9 @@ private TransactionAndAccount[] populateCache( final var transaction = createTransaction(nonce, keys); final Account sender = mock(Account.class); when(sender.getNonce()).thenReturn(startingNonce); - final var res = pendingTransactions.addRemoteTransaction(transaction, Optional.of(sender)); + final var res = + pendingTransactions.addTransaction( + createRemotePendingTransaction(transaction), Optional.of(sender)); assertTransactionPending(pendingTransactions, transaction); assertThat(res).isEqualTo(ADDED); addedTransactions.add(new TransactionAndAccount(transaction, sender)); diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayersTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayersTest.java index 0137fa17c44..666a5d66005 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayersTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/LayersTest.java @@ -19,6 +19,8 @@ import static org.hyperledger.besu.ethereum.eth.transactions.layered.LayersTest.Sender.S2; import static org.hyperledger.besu.ethereum.eth.transactions.layered.LayersTest.Sender.S3; import static org.hyperledger.besu.ethereum.eth.transactions.layered.LayersTest.Sender.S4; +import static org.hyperledger.besu.ethereum.eth.transactions.layered.LayersTest.Sender.SP1; +import static org.hyperledger.besu.ethereum.eth.transactions.layered.LayersTest.Sender.SP2; import static org.hyperledger.besu.ethereum.eth.transactions.layered.TransactionsLayer.RemovalReason.INVALIDATED; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -146,6 +148,12 @@ void asyncWorldStateUpdates(final Scenario scenario) { assertScenario(scenario); } + @ParameterizedTest + @MethodSource("providerPrioritySenders") + void prioritySenders(final Scenario scenario) { + assertScenario(scenario); + } + private void assertScenario(final Scenario scenario) { scenario.execute( pendingTransactions, @@ -1023,6 +1031,141 @@ static Stream providerAsyncWorldStateUpdates() { .expectedSparseForSender(S1, 8, 9, 7))); } + static Stream providerPrioritySenders() { + return Stream.of( + Arguments.of( + new Scenario("priority first same fee") + .addForSenders(S1, 0, SP1, 0) + .expectedPrioritizedForSenders(SP1, 0, S1, 0)), + Arguments.of( + new Scenario("priority first lower fee") + .addForSenders(S2, 0, SP1, 0) + .expectedPrioritizedForSenders(SP1, 0, S2, 0)), + Arguments.of( + new Scenario("priority first higher fee") + .addForSenders(S1, 0, SP2, 0) + .expectedPrioritizedForSenders(SP2, 0, S1, 0)), + Arguments.of( + new Scenario("same priority order by fee") + .addForSenders(SP1, 0, SP2, 0) + .expectedPrioritizedForSenders(SP2, 0, SP1, 0)), + Arguments.of( + new Scenario("same priority order by fee") + .addForSenders(SP2, 0, SP1, 0) + .expectedPrioritizedForSenders(SP2, 0, SP1, 0)), + Arguments.of( + new Scenario("priority first overflow to ready") + .addForSender(S2, 0, 1, 2) + .expectedPrioritizedForSender(S2, 0, 1, 2) + .addForSender(SP1, 0) + .expectedPrioritizedForSenders(SP1, 0, S2, 0, S2, 1) + .expectedReadyForSender(S2, 2)), + Arguments.of( + new Scenario("priority first overflow to ready 2") + .addForSender(S2, 0, 1, 2) + .expectedPrioritizedForSender(S2, 0, 1, 2) + .addForSender(SP1, 0, 1, 2) + .expectedPrioritizedForSender(SP1, 0, 1, 2) + .expectedReadyForSender(S2, 0, 1, 2)), + Arguments.of( + new Scenario("multiple priority senders first overflow to ready") + .addForSender(S2, 0, 1, 2) + .expectedPrioritizedForSender(S2, 0, 1, 2) + .addForSenders(SP2, 0, SP1, 0) + .expectedPrioritizedForSenders(SP2, 0, SP1, 0, S2, 0) + .expectedReadyForSender(S2, 1, 2)), + Arguments.of( + new Scenario("priority with initial gap") + .addForSender(S2, 0) + .expectedPrioritizedForSender(S2, 0) + .addForSender(SP1, 1) // initial gap + .expectedPrioritizedForSender(S2, 0) + .expectedSparseForSender(SP1, 1) + .addForSender(SP1, 0) // fill gap + .expectedPrioritizedForSenders(SP1, 0, SP1, 1, S2, 0) + .expectedSparseForSenders()), + Arguments.of( + new Scenario("priority with initial gap overflow to ready") + .addForSender(S2, 0, 1) + .expectedPrioritizedForSender(S2, 0, 1) + .addForSender(SP1, 1) // initial gap + .expectedSparseForSender(SP1, 1) + .addForSender(SP1, 0) // fill gap + .expectedPrioritizedForSenders(SP1, 0, SP1, 1, S2, 0) + .expectedReadyForSender(S2, 1) + .expectedSparseForSenders()), + Arguments.of( + new Scenario("priority with initial gap overflow to ready when prioritized is full") + .addForSender(S2, 0, 1, 2) + .expectedPrioritizedForSender(S2, 0, 1, 2) + .addForSender(SP1, 1) // initial gap + .expectedSparseForSender(SP1, 1) + .addForSender(SP1, 0) // fill gap, but there is not enough space to promote + .expectedPrioritizedForSenders(SP1, 0, S2, 0, S2, 1) + .expectedReadyForSender(S2, 2) + .expectedSparseForSender(SP1, 1) + .confirmedForSenders( + SP1, 0) // asap there is new space the priority tx is promoted first + .expectedPrioritizedForSenders(SP1, 1, S2, 0, S2, 1) + .expectedReadyForSender(S2, 2) + .expectedSparseForSenders()), + Arguments.of( + new Scenario("overflow to ready promote priority first") + .addForSender(SP2, 0, 1, 2) + .expectedPrioritizedForSender(SP2, 0, 1, 2) + .addForSender(S2, 0) + .expectedReadyForSender(S2, 0) + .addForSender(SP1, 0) + .expectedReadyForSenders(SP1, 0, S2, 0) + .confirmedForSenders( + SP2, 0) // asap there is new space the priority tx is promoted first + .expectedPrioritizedForSenders(SP2, 1, SP2, 2, SP1, 0) + .expectedReadyForSender(S2, 0)), + Arguments.of( + new Scenario("priority first overflow to sparse") + .addForSender(SP2, 0, 1, 2) + .addForSender(S3, 0) + .expectedPrioritizedForSender(SP2, 0, 1, 2) + .expectedReadyForSender(S3, 0) + .addForSender(SP1, 0, 1, 2) + .expectedPrioritizedForSender(SP2, 0, 1, 2) + .expectedReadyForSender(SP1, 0, 1, 2) + .expectedSparseForSender(S3, 0)), + Arguments.of( + new Scenario("priority first overflow to sparse 2") + .addForSender(S2, 0, 1, 2) + .addForSender(S3, 0, 1, 2) + .expectedPrioritizedForSender(S3, 0, 1, 2) + .expectedReadyForSender(S2, 0, 1, 2) + .addForSender(SP1, 0) + .expectedPrioritizedForSenders(SP1, 0, S3, 0, S3, 1) + .expectedReadyForSenders(S3, 2, S2, 0, S2, 1) + .expectedSparseForSender(S2, 2)), + Arguments.of( + new Scenario("overflow to sparse promote priority first") + .addForSender(SP2, 0, 1, 2, 3, 4, 5) + .expectedPrioritizedForSender(SP2, 0, 1, 2) + .expectedReadyForSender(SP2, 3, 4, 5) + .addForSender(S3, 0) + .expectedSparseForSender(S3, 0) + .addForSender(SP1, 0) + .expectedSparseForSenders(S3, 0, SP1, 0) + .confirmedForSenders(SP2, 0) + .expectedPrioritizedForSender(SP2, 1, 2, 3) + .expectedReadyForSenders(SP2, 4, SP2, 5, SP1, 0) + .expectedSparseForSender(S3, 0)), + Arguments.of( + new Scenario("discard priority as last") + .addForSender(SP2, 0, 1, 2, 3, 4, 5) + .expectedPrioritizedForSender(SP2, 0, 1, 2) + .expectedReadyForSender(SP2, 3, 4, 5) + .addForSender(S3, 0) + .expectedSparseForSender(S3, 0) + .addForSender(SP1, 0, 1, 2) + .expectedSparseForSender(SP1, 0, 1, 2) + .expectedDroppedForSender(S3, 0))); + } + private static BlockHeader mockBlockHeader() { final BlockHeader blockHeader = mock(BlockHeader.class); when(blockHeader.getBaseFee()).thenReturn(Optional.of(Wei.ONE)); @@ -1145,7 +1288,7 @@ private PendingTransaction get(final Sender sender, final long nonce) { private PendingTransaction createEIP1559PendingTransactions( final Sender sender, final long nonce) { return createRemotePendingTransaction( - createEIP1559Transaction(nonce, sender.key, sender.gasFeeMultiplier)); + createEIP1559Transaction(nonce, sender.key, sender.gasFeeMultiplier), sender.hasPriority); } public Scenario expectedPrioritizedForSender(final Sender sender, final long... nonce) { @@ -1338,19 +1481,24 @@ public String toString() { } enum Sender { - S1(1), - S2(2), - S3(3), - S4(4); + S1(false, 1), + S2(false, 2), + S3(false, 3), + S4(false, 4), + SP1(true, 1), + SP2(true, 2); final KeyPair key; final Address address; final int gasFeeMultiplier; - Sender(final int gasFeeMultiplier) { + final boolean hasPriority; + + Sender(final boolean hasPriority, final int gasFeeMultiplier) { this.key = SIGNATURE_ALGORITHM.get().generateKeyPair(); this.address = Util.publicKeyToAddress(key.getPublicKey()); this.gasFeeMultiplier = gasFeeMultiplier; + this.hasPriority = hasPriority; } } } diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReplayTest.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReplayTest.java index 864efcd7f26..589818398b5 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReplayTest.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/layered/ReplayTest.java @@ -45,10 +45,12 @@ import java.math.BigInteger; import java.nio.charset.StandardCharsets; import java.time.Instant; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.function.BiFunction; import java.util.zip.GZIPInputStream; import com.google.common.base.Splitter; @@ -61,9 +63,6 @@ public class ReplayTest { private static final Logger LOG = LoggerFactory.getLogger(ReplayTest.class); - private final TransactionPoolConfiguration poolConfig = - ImmutableTransactionPoolConfiguration.builder().build(); - private final StubMetricsSystem metricsSystem = new StubMetricsSystem(); private final TransactionPoolMetrics txPoolMetrics = new TransactionPoolMetrics(metricsSystem); @@ -113,6 +112,11 @@ public void replay() throws IOException { currBlockHeader = mockBlockHeader(br.readLine()); final BaseFeeMarket baseFeeMarket = FeeMarket.london(0L); + final TransactionPoolConfiguration poolConfig = + ImmutableTransactionPoolConfiguration.builder() + .prioritySenders(readPrioritySenders(br.readLine())) + .build(); + final AbstractPrioritizedTransactions prioritizedTransactions = createLayers(poolConfig, txPoolMetrics, baseFeeMarket); final LayeredPendingTransactions pendingTransactions = @@ -155,6 +159,10 @@ public void replay() throws IOException { } } + private List
readPrioritySenders(final String line) { + return Arrays.stream(line.split(",")).map(Address::fromHexString).toList(); + } + private BlockHeader mockBlockHeader(final String line) { final List commaSplit = Splitter.on(',').splitToList(line); final long number = Long.parseLong(commaSplit.get(0)); @@ -176,20 +184,20 @@ private BaseFeePrioritizedTransactions createLayers( final TransactionPoolMetrics txPoolMetrics, final BaseFeeMarket baseFeeMarket) { final EvictCollectorLayer evictCollector = new EvictCollectorLayer(txPoolMetrics); + final BiFunction txReplacementTester = + (tx1, tx2) -> transactionReplacementTester(poolConfig, tx1, tx2); final SparseTransactions sparseTransactions = - new SparseTransactions( - poolConfig, evictCollector, txPoolMetrics, this::transactionReplacementTester); + new SparseTransactions(poolConfig, evictCollector, txPoolMetrics, txReplacementTester); final ReadyTransactions readyTransactions = - new ReadyTransactions( - poolConfig, sparseTransactions, txPoolMetrics, this::transactionReplacementTester); + new ReadyTransactions(poolConfig, sparseTransactions, txPoolMetrics, txReplacementTester); return new BaseFeePrioritizedTransactions( poolConfig, () -> currBlockHeader, readyTransactions, txPoolMetrics, - this::transactionReplacementTester, + txReplacementTester, baseFeeMarket); } @@ -263,7 +271,10 @@ private void processTransaction( tx.getNonce(), prioritizedTransactions.logSender(senderToLog)); } - assertThat(pendingTransactions.addRemoteTransaction(tx, Optional.of(mockAccount))) + assertThat( + pendingTransactions.addTransaction( + PendingTransaction.newPendingTransaction(tx, false, false), + Optional.of(mockAccount))) .isNotEqualTo(TransactionAddedResult.INTERNAL_ERROR); if (tx.getSender().equals(senderToLog)) { LOG.warn("After {}", prioritizedTransactions.logSender(senderToLog)); @@ -283,11 +294,6 @@ private void processInvalid( } } - private boolean transactionReplacementTester( - final PendingTransaction pt1, final PendingTransaction pt2) { - return transactionReplacementTester(poolConfig, pt1, pt2); - } - private boolean transactionReplacementTester( final TransactionPoolConfiguration poolConfig, final PendingTransaction pt1, diff --git a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsTestBase.java b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsTestBase.java index f1d9ce1016d..4511dbd585e 100644 --- a/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsTestBase.java +++ b/ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/sorter/AbstractPendingTransactionsTestBase.java @@ -36,6 +36,7 @@ import org.hyperledger.besu.ethereum.core.TransactionTestFixture; import org.hyperledger.besu.ethereum.core.Util; import org.hyperledger.besu.ethereum.eth.transactions.ImmutableTransactionPoolConfiguration; +import org.hyperledger.besu.ethereum.eth.transactions.PendingTransaction; import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactionAddedListener; import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactionDroppedListener; import org.hyperledger.besu.ethereum.eth.transactions.PendingTransactions; @@ -109,13 +110,13 @@ abstract AbstractPendingTransactionsSorter getPendingTransactions( @Test public void shouldReturnExclusivelyLocalTransactionsWhenAppropriate() { final Transaction localTransaction0 = createTransaction(0); - transactions.addLocalTransaction(localTransaction0, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(localTransaction0), Optional.empty()); assertThat(transactions.size()).isEqualTo(1); - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); assertThat(transactions.size()).isEqualTo(2); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); assertThat(transactions.size()).isEqualTo(3); final List localTransactions = transactions.getLocalTransactions(); @@ -124,11 +125,11 @@ public void shouldReturnExclusivelyLocalTransactionsWhenAppropriate() { @Test public void shouldAddATransaction() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); assertThat(transactions.size()).isEqualTo(1); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, REMOTE)).isEqualTo(1); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); assertThat(transactions.size()).isEqualTo(2); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, REMOTE)).isEqualTo(2); } @@ -140,7 +141,7 @@ public void shouldReturnEmptyOptionalWhenNoTransactionWithGivenHashExists() { @Test public void shouldGetTransactionByHash() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); assertTransactionPending(transaction1); } @@ -150,13 +151,15 @@ public void shouldDropOldestTransactionWhenLimitExceeded() { transactionWithNonceSenderAndGasPrice(0, SIGNATURE_ALGORITHM.get().generateKeyPair(), 10L); final Account oldestSender = mock(Account.class); when(oldestSender.getNonce()).thenReturn(0L); - senderLimitedTransactions.addRemoteTransaction(oldestTransaction, Optional.of(oldestSender)); + senderLimitedTransactions.addTransaction( + createRemotePendingTransaction(oldestTransaction), Optional.of(oldestSender)); for (int i = 1; i < MAX_TRANSACTIONS; i++) { final Account sender = mock(Account.class); when(sender.getNonce()).thenReturn((long) i); - senderLimitedTransactions.addRemoteTransaction( - transactionWithNonceSenderAndGasPrice( - i, SIGNATURE_ALGORITHM.get().generateKeyPair(), 10L), + senderLimitedTransactions.addTransaction( + createRemotePendingTransaction( + transactionWithNonceSenderAndGasPrice( + i, SIGNATURE_ALGORITHM.get().generateKeyPair(), 10L)), Optional.of(sender)); } assertThat(senderLimitedTransactions.size()).isEqualTo(MAX_TRANSACTIONS); @@ -164,8 +167,9 @@ public void shouldDropOldestTransactionWhenLimitExceeded() { final Account lastSender = mock(Account.class); when(lastSender.getNonce()).thenReturn(6L); - senderLimitedTransactions.addRemoteTransaction( - createTransaction(MAX_TRANSACTIONS + 1), Optional.of(lastSender)); + senderLimitedTransactions.addTransaction( + createRemotePendingTransaction(createTransaction(MAX_TRANSACTIONS + 1)), + Optional.of(lastSender)); assertThat(senderLimitedTransactions.size()).isEqualTo(MAX_TRANSACTIONS); assertTransactionNotPending(oldestTransaction); assertThat(metricsSystem.getCounterValue(REMOVED_COUNTER, REMOTE, DROPPED)).isEqualTo(1); @@ -176,7 +180,8 @@ public void shouldDropTransactionWithATooFarNonce() { Transaction furthestFutureTransaction = null; for (int i = 0; i < MAX_TRANSACTIONS; i++) { furthestFutureTransaction = transactionWithNonceSenderAndGasPrice(i, KEYS1, 10L); - senderLimitedTransactions.addRemoteTransaction(furthestFutureTransaction, Optional.empty()); + senderLimitedTransactions.addTransaction( + createRemotePendingTransaction(furthestFutureTransaction), Optional.empty()); } assertThat(senderLimitedTransactions.size()) .isEqualTo(senderLimitedConfig.getTxPoolMaxFutureTransactionByAccount()); @@ -187,26 +192,32 @@ public void shouldDropTransactionWithATooFarNonce() { @Test public void shouldHandleMaximumTransactionLimitCorrectlyWhenSameTransactionAddedMultipleTimes() { - transactions.addRemoteTransaction(createTransaction(0), Optional.empty()); - transactions.addRemoteTransaction(createTransaction(0), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(createTransaction(0)), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(createTransaction(0)), Optional.empty()); for (int i = 1; i < MAX_TRANSACTIONS; i++) { - transactions.addRemoteTransaction(createTransaction(i), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(createTransaction(i)), Optional.empty()); } assertThat(transactions.size()).isEqualTo(MAX_TRANSACTIONS); - transactions.addRemoteTransaction(createTransaction(MAX_TRANSACTIONS + 1), Optional.empty()); - transactions.addRemoteTransaction(createTransaction(MAX_TRANSACTIONS + 2), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(createTransaction(MAX_TRANSACTIONS + 1)), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(createTransaction(MAX_TRANSACTIONS + 2)), Optional.empty()); assertThat(transactions.size()).isEqualTo(MAX_TRANSACTIONS); } @Test public void shouldPrioritizeLocalTransaction() { final Transaction localTransaction = createTransaction(0); - transactions.addLocalTransaction(localTransaction, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(localTransaction), Optional.empty()); for (int i = 1; i <= MAX_TRANSACTIONS; i++) { - transactions.addRemoteTransaction(createTransaction(i), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(createTransaction(i)), Optional.empty()); } assertThat(transactions.size()).isEqualTo(MAX_TRANSACTIONS); assertTransactionPending(localTransaction); @@ -218,7 +229,8 @@ public void shouldStartDroppingLocalTransactionsWhenPoolIsFullOfLocalTransaction for (int i = 0; i <= MAX_TRANSACTIONS; i++) { lastLocalTransactionForSender = createTransaction(i); - transactions.addLocalTransaction(lastLocalTransactionForSender, Optional.empty()); + transactions.addTransaction( + createLocalPendingTransaction(lastLocalTransactionForSender), Optional.empty()); } assertThat(transactions.size()).isEqualTo(MAX_TRANSACTIONS); assertTransactionNotPending(lastLocalTransactionForSender); @@ -228,7 +240,7 @@ public void shouldStartDroppingLocalTransactionsWhenPoolIsFullOfLocalTransaction public void shouldNotifyListenerWhenRemoteTransactionAdded() { transactions.subscribePendingTransactions(listener); - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); verify(listener).onTransactionAdded(transaction1); } @@ -237,13 +249,13 @@ public void shouldNotifyListenerWhenRemoteTransactionAdded() { public void shouldNotNotifyListenerAfterUnsubscribe() { final long id = transactions.subscribePendingTransactions(listener); - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); verify(listener).onTransactionAdded(transaction1); transactions.unsubscribePendingTransactions(id); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); verifyNoMoreInteractions(listener); } @@ -252,14 +264,14 @@ public void shouldNotNotifyListenerAfterUnsubscribe() { public void shouldNotifyListenerWhenLocalTransactionAdded() { transactions.subscribePendingTransactions(listener); - transactions.addLocalTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction1), Optional.empty()); verify(listener).onTransactionAdded(transaction1); } @Test public void shouldNotifyDroppedListenerWhenRemoteTransactionDropped() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); transactions.subscribeDroppedTransactions(droppedListener); @@ -270,8 +282,8 @@ public void shouldNotifyDroppedListenerWhenRemoteTransactionDropped() { @Test public void shouldNotNotifyDroppedListenerAfterUnsubscribe() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); final long id = transactions.subscribeDroppedTransactions(droppedListener); @@ -288,7 +300,7 @@ public void shouldNotNotifyDroppedListenerAfterUnsubscribe() { @Test public void shouldNotifyDroppedListenerWhenLocalTransactionDropped() { - transactions.addLocalTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction1), Optional.empty()); transactions.subscribeDroppedTransactions(droppedListener); @@ -299,7 +311,7 @@ public void shouldNotifyDroppedListenerWhenLocalTransactionDropped() { @Test public void shouldNotNotifyDroppedListenerWhenTransactionAddedToBlock() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); transactions.subscribeDroppedTransactions(droppedListener); @@ -310,8 +322,8 @@ public void shouldNotNotifyDroppedListenerWhenTransactionAddedToBlock() { @Test public void selectTransactionsUntilSelectorRequestsNoMore() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); final List parsedTransactions = Lists.newArrayList(); transactions.selectTransactions( @@ -326,8 +338,8 @@ public void selectTransactionsUntilSelectorRequestsNoMore() { @Test public void selectTransactionsUntilPendingIsEmpty() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); final List parsedTransactions = Lists.newArrayList(); transactions.selectTransactions( @@ -346,8 +358,8 @@ public void shouldNotSelectReplacedTransaction() { final Transaction transaction1 = transactionWithNonceSenderAndGasPrice(1, KEYS1, 1); final Transaction transaction2 = transactionWithNonceSenderAndGasPrice(1, KEYS1, 2); - transactions.addRemoteTransaction(transaction1, Optional.empty()); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); final List parsedTransactions = Lists.newArrayList(); transactions.selectTransactions( @@ -361,8 +373,8 @@ public void shouldNotSelectReplacedTransaction() { @Test public void invalidTransactionIsDeletedFromPendingTransactions() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); final List parsedTransactions = Lists.newArrayList(); transactions.selectTransactions( @@ -387,7 +399,7 @@ public void shouldReturnEmptyOptionalAsMaximumNonceWhenNoTransactionsPresent() { @Test public void shouldReturnEmptyOptionalAsMaximumNonceWhenLastTransactionForSenderRemoved() { final Transaction transaction = transactionWithNonceAndSender(1, KEYS1); - transactions.addRemoteTransaction(transaction, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction), Optional.empty()); transactions.removeTransaction(transaction); assertThat(transactions.getNextNonceForSender(SENDER1)).isEmpty(); } @@ -397,9 +409,18 @@ public void shouldReplaceTransactionWithSameSenderAndNonce() { final Transaction transaction1 = transactionWithNonceSenderAndGasPrice(1, KEYS1, 1); final Transaction transaction1b = transactionWithNonceSenderAndGasPrice(1, KEYS1, 2); final Transaction transaction2 = transactionWithNonceSenderAndGasPrice(2, KEYS1, 1); - assertThat(transactions.addRemoteTransaction(transaction1, Optional.empty())).isEqualTo(ADDED); - assertThat(transactions.addRemoteTransaction(transaction2, Optional.empty())).isEqualTo(ADDED); - assertThat(transactions.addRemoteTransaction(transaction1b, Optional.empty())).isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty())) + .isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction2), Optional.empty())) + .isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction1b), Optional.empty())) + .isEqualTo(ADDED); assertTransactionNotPending(transaction1); assertTransactionPending(transaction1b); @@ -416,12 +437,17 @@ public void shouldReplaceTransactionWithSameSenderAndNonce_multipleReplacements( for (int i = 0; i < replacedTxCount; i++) { final Transaction duplicateTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, i + 1); replacedTransactions.add(duplicateTx); - transactions.addRemoteTransaction(duplicateTx, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(duplicateTx), Optional.empty()); } final Transaction finalReplacingTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, 100); final Transaction independentTx = transactionWithNonceSenderAndGasPrice(2, KEYS1, 1); - assertThat(transactions.addRemoteTransaction(independentTx, Optional.empty())).isEqualTo(ADDED); - assertThat(transactions.addRemoteTransaction(finalReplacingTx, Optional.empty())) + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(independentTx), Optional.empty())) + .isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(finalReplacingTx), Optional.empty())) .isEqualTo(ADDED); // All tx's except the last duplicate should be removed @@ -447,17 +473,22 @@ public void shouldReplaceTransactionWithSameSenderAndNonce_multipleReplacements( transactionWithNonceSenderAndGasPrice(1, KEYS1, (i * 110 / 100) + 1); replacedTransactions.add(duplicateTx); if (i % 2 == 0) { - transactions.addRemoteTransaction(duplicateTx, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(duplicateTx), Optional.empty()); remoteDuplicateCount++; } else { - transactions.addLocalTransaction(duplicateTx, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(duplicateTx), Optional.empty()); } } final Transaction finalReplacingTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, 100); final Transaction independentTx = transactionWithNonceSenderAndGasPrice(2, KEYS1, 1); - assertThat(transactions.addLocalTransaction(finalReplacingTx, Optional.empty())) + assertThat( + transactions.addTransaction( + createLocalPendingTransaction(finalReplacingTx), Optional.empty())) + .isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(independentTx), Optional.empty())) .isEqualTo(ADDED); - assertThat(transactions.addRemoteTransaction(independentTx, Optional.empty())).isEqualTo(ADDED); // All tx's except the last duplicate should be removed replacedTransactions.forEach(this::assertTransactionNotPending); @@ -481,8 +512,14 @@ public void shouldReplaceTransactionWithSameSenderAndNonce_multipleReplacements( public void shouldReplaceOnlyTransactionFromSenderWhenItHasTheSameNonce() { final Transaction transaction1 = transactionWithNonceSenderAndGasPrice(1, KEYS1, 1); final Transaction transaction1b = transactionWithNonceSenderAndGasPrice(1, KEYS1, 2); - assertThat(transactions.addRemoteTransaction(transaction1, Optional.empty())).isEqualTo(ADDED); - assertThat(transactions.addRemoteTransaction(transaction1b, Optional.empty())).isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty())) + .isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction1b), Optional.empty())) + .isEqualTo(ADDED); assertTransactionNotPending(transaction1); assertTransactionPending(transaction1b); @@ -495,10 +532,15 @@ public void shouldReplaceOnlyTransactionFromSenderWhenItHasTheSameNonce() { public void shouldNotReplaceTransactionWithSameSenderAndNonceWhenGasPriceIsLower() { final Transaction transaction1 = transactionWithNonceSenderAndGasPrice(1, KEYS1, 2); final Transaction transaction1b = transactionWithNonceSenderAndGasPrice(1, KEYS1, 1); - assertThat(transactions.addRemoteTransaction(transaction1, Optional.empty())).isEqualTo(ADDED); + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty())) + .isEqualTo(ADDED); transactions.subscribePendingTransactions(listener); - assertThat(transactions.addRemoteTransaction(transaction1b, Optional.empty())) + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction1b), Optional.empty())) .isEqualTo(REJECTED_UNDERPRICED_REPLACEMENT); assertTransactionNotPending(transaction1b); @@ -509,16 +551,20 @@ public void shouldNotReplaceTransactionWithSameSenderAndNonceWhenGasPriceIsLower @Test public void shouldTrackMaximumNonceForEachSender() { - transactions.addRemoteTransaction(transactionWithNonceAndSender(0, KEYS1), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(transactionWithNonceAndSender(0, KEYS1)), Optional.empty()); assertMaximumNonceForSender(SENDER1, 1); - transactions.addRemoteTransaction(transactionWithNonceAndSender(1, KEYS1), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(transactionWithNonceAndSender(1, KEYS1)), Optional.empty()); assertMaximumNonceForSender(SENDER1, 2); - transactions.addRemoteTransaction(transactionWithNonceAndSender(2, KEYS1), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(transactionWithNonceAndSender(2, KEYS1)), Optional.empty()); assertMaximumNonceForSender(SENDER1, 3); - transactions.addRemoteTransaction(transactionWithNonceAndSender(4, KEYS2), Optional.empty()); + transactions.addTransaction( + createRemotePendingTransaction(transactionWithNonceAndSender(4, KEYS2)), Optional.empty()); assertMaximumNonceForSender(SENDER2, 5); assertMaximumNonceForSender(SENDER1, 3); } @@ -529,9 +575,9 @@ public void shouldIterateTransactionsFromSameSenderInNonceOrder() { final Transaction transaction2 = transactionWithNonceAndSender(1, KEYS1); final Transaction transaction3 = transactionWithNonceAndSender(2, KEYS1); - transactions.addLocalTransaction(transaction1, Optional.empty()); - transactions.addLocalTransaction(transaction2, Optional.empty()); - transactions.addLocalTransaction(transaction3, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction2), Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction3), Optional.empty()); final List iterationOrder = new ArrayList<>(); transactions.selectTransactions( @@ -548,8 +594,8 @@ public void shouldNotForceNonceOrderWhenSendersDiffer() { final Transaction transaction1 = transactionWithNonceAndSender(0, KEYS1); final Transaction transaction2 = transactionWithNonceAndSender(1, KEYS2); - transactions.addLocalTransaction(transaction1, Optional.empty()); - transactions.addLocalTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction2), Optional.empty()); final List iterationOrder = new ArrayList<>(); transactions.selectTransactions( @@ -568,10 +614,10 @@ public void shouldNotIncreasePriorityOfTransactionsBecauseOfNonceOrder() { final Transaction transaction3 = transactionWithNonceAndSender(2, KEYS1); final Transaction transaction4 = transactionWithNonceAndSender(4, KEYS2); - transactions.addLocalTransaction(transaction1, Optional.empty()); - transactions.addLocalTransaction(transaction4, Optional.empty()); - transactions.addLocalTransaction(transaction2, Optional.empty()); - transactions.addLocalTransaction(transaction3, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction1), Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction4), Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction2), Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction3), Optional.empty()); final List iterationOrder = new ArrayList<>(); transactions.selectTransactions( @@ -616,6 +662,19 @@ protected Transaction createTransaction(final long transactionNumber) { .createTransaction(KEYS1); } + private PendingTransaction createRemotePendingTransaction( + final Transaction transaction, final long addedAt) { + return PendingTransaction.newPendingTransaction(transaction, false, false, addedAt); + } + + private PendingTransaction createRemotePendingTransaction(final Transaction transaction) { + return PendingTransaction.newPendingTransaction(transaction, false, false); + } + + private PendingTransaction createLocalPendingTransaction(final Transaction transaction) { + return PendingTransaction.newPendingTransaction(transaction, true, true); + } + @Test public void shouldEvictMultipleOldTransactions() { final int maxTransactionRetentionHours = 1; @@ -628,9 +687,9 @@ public void shouldEvictMultipleOldTransactions() { .build(), Optional.of(clock)); - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); assertThat(transactions.size()).isEqualTo(1); - transactions.addRemoteTransaction(transaction2, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction2), Optional.empty()); assertThat(transactions.size()).isEqualTo(2); clock.step(2L, ChronoUnit.HOURS); @@ -649,7 +708,8 @@ public void shouldEvictSingleOldTransaction() { .txPoolLimitByAccountPercentage(Fraction.fromFloat(1.0f)) .build(), Optional.of(clock)); - evictSingleTransactions.addRemoteTransaction(transaction1, Optional.empty()); + evictSingleTransactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty()); assertThat(evictSingleTransactions.size()).isEqualTo(1); clock.step(2L, ChronoUnit.HOURS); evictSingleTransactions.evictOldTransactions(); @@ -668,10 +728,12 @@ public void shouldEvictExclusivelyOldTransactions() { .build(), Optional.of(clock)); - twoHourEvictionTransactionPool.addRemoteTransaction(transaction1, Optional.empty()); + twoHourEvictionTransactionPool.addTransaction( + createRemotePendingTransaction(transaction1, clock.millis()), Optional.empty()); assertThat(twoHourEvictionTransactionPool.size()).isEqualTo(1); clock.step(3L, ChronoUnit.HOURS); - twoHourEvictionTransactionPool.addRemoteTransaction(transaction2, Optional.empty()); + twoHourEvictionTransactionPool.addTransaction( + createRemotePendingTransaction(transaction2, clock.millis()), Optional.empty()); assertThat(twoHourEvictionTransactionPool.size()).isEqualTo(2); twoHourEvictionTransactionPool.evictOldTransactions(); assertThat(twoHourEvictionTransactionPool.size()).isEqualTo(1); @@ -680,12 +742,14 @@ public void shouldEvictExclusivelyOldTransactions() { @Test public void shouldNotIncrementAddedCounterWhenRemoteTransactionAlreadyPresent() { - transactions.addLocalTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createLocalPendingTransaction(transaction1), Optional.empty()); assertThat(transactions.size()).isEqualTo(1); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, LOCAL)).isEqualTo(1); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, REMOTE)).isEqualTo(0); - assertThat(transactions.addRemoteTransaction(transaction1, Optional.empty())) + assertThat( + transactions.addTransaction( + createRemotePendingTransaction(transaction1), Optional.empty())) .isEqualTo(ALREADY_KNOWN); assertThat(transactions.size()).isEqualTo(1); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, LOCAL)).isEqualTo(1); @@ -694,12 +758,14 @@ public void shouldNotIncrementAddedCounterWhenRemoteTransactionAlreadyPresent() @Test public void shouldNotIncrementAddedCounterWhenLocalTransactionAlreadyPresent() { - transactions.addRemoteTransaction(transaction1, Optional.empty()); + transactions.addTransaction(createRemotePendingTransaction(transaction1), Optional.empty()); assertThat(transactions.size()).isEqualTo(1); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, LOCAL)).isEqualTo(0); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, REMOTE)).isEqualTo(1); - assertThat(transactions.addLocalTransaction(transaction1, Optional.empty())) + assertThat( + transactions.addTransaction( + createLocalPendingTransaction(transaction1), Optional.empty())) .isEqualTo(ALREADY_KNOWN); assertThat(transactions.size()).isEqualTo(1); assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, LOCAL)).isEqualTo(0); @@ -774,7 +840,8 @@ protected void addLocalTransactions(final PendingTransactions sorter, final long for (final long nonce : nonces) { final Account sender = mock(Account.class); when(sender.getNonce()).thenReturn(1L); - sorter.addLocalTransaction(createTransaction(nonce), Optional.of(sender)); + sorter.addTransaction( + createLocalPendingTransaction(createTransaction(nonce)), Optional.of(sender)); } } @@ -798,7 +865,8 @@ public void shouldPrioritizeGasPriceThenTimeAddedToPool() { final Transaction lowPriceTx = transactionWithNonceSenderAndGasPrice( 0, SIGNATURE_ALGORITHM.get().generateKeyPair(), 10); - transactions.addRemoteTransaction(lowPriceTx, Optional.of(randomSender)); + transactions.addTransaction( + createRemotePendingTransaction(lowPriceTx), Optional.of(randomSender)); return lowPriceTx; }) .collect(Collectors.toUnmodifiableList()); @@ -807,7 +875,8 @@ public void shouldPrioritizeGasPriceThenTimeAddedToPool() { final Account highPriceSender = mock(Account.class); final Transaction highGasPriceTransaction = transactionWithNonceSenderAndGasPrice(0, KEYS1, 100); - transactions.addRemoteTransaction(highGasPriceTransaction, Optional.of(highPriceSender)); + transactions.addTransaction( + createRemotePendingTransaction(highGasPriceTransaction), Optional.of(highPriceSender)); assertThat(transactions.size()).isEqualTo(MAX_TRANSACTIONS); assertTransactionPending(highGasPriceTransaction); From 2267bc7f142d4fc89b4dfb32df72005e49fcb6c9 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 17 Oct 2023 15:16:55 +0200 Subject: [PATCH 08/12] Fix javadoc Signed-off-by: Fabio Di Fabio --- .../common/bft/blockcreation/BftBlockCreatorFactory.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java index 528f4e72062..fab04b1a770 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreatorFactory.java @@ -51,8 +51,9 @@ public class BftBlockCreatorFactory { /** The Forks schedule. */ protected final ForksSchedule forksSchedule; - + /** The Mining parameters */ protected final MiningParameters miningParameters; + private final TransactionPool transactionPool; /** The Protocol context. */ protected final ProtocolContext protocolContext; From c6b56d241d195eb20cd81beb10307756eb42d8e3 Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 17 Oct 2023 15:21:39 +0200 Subject: [PATCH 09/12] Remove code not belonging to this PR Signed-off-by: Fabio Di Fabio --- .../besu/plugin/data/TransactionSelectionResult.java | 6 ------ 1 file changed, 6 deletions(-) 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 dee35f82b7b..ffae842ca72 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 @@ -28,7 +28,6 @@ private enum Status { SELECTED, BLOCK_FULL(true, false), BLOCK_OCCUPANCY_ABOVE_THRESHOLD(true, false), - BLOCK_SELECTION_TIMEOUT(true, false), INVALID_TRANSIENT(false, false), INVALID(false, true); @@ -57,11 +56,6 @@ public String toString() { /** The transaction has not been selected since the block is full. */ public static final TransactionSelectionResult BLOCK_FULL = new TransactionSelectionResult(Status.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); - ; - /** * The transaction has not been selected since too large and the occupancy of the block is enough * to stop the selection. From 477acf93deeb5c3ab950fa1e27f37357645c2f8d Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 17 Oct 2023 16:04:55 +0200 Subject: [PATCH 10/12] coinbase is an updatable parameter Signed-off-by: Fabio Di Fabio --- .../BesuNodeConfigurationBuilder.java | 6 ++++- .../node/configuration/BesuNodeFactory.java | 2 +- .../bft/BftMiningAcceptanceTest.java | 2 +- .../cli/options/stable/MiningOptions.java | 10 +++---- .../subcommands/blocks/BlocksSubCommand.java | 2 +- .../controller/IbftBesuControllerBuilder.java | 1 + .../hyperledger/besu/cli/BesuCommandTest.java | 11 ++++++-- .../cli/options/stable/MiningOptionsTest.java | 2 +- .../blockcreation/CliqueBlockCreator.java | 4 --- .../blockcreation/CliqueMinerExecutor.java | 4 +-- .../blockcreation/CliqueBlockCreatorTest.java | 5 +--- .../CliqueMinerExecutorTest.java | 2 +- .../bft/blockcreation/BftBlockCreator.java | 1 - .../ibft/support/TestContextBuilder.java | 2 +- .../blockcreation/BftBlockCreatorTest.java | 2 +- .../blockcreation/MergeBlockCreator.java | 5 +--- .../merge/blockcreation/MergeCoordinator.java | 4 ++- .../blockcreation/MergeCoordinatorTest.java | 9 +++---- .../merge/blockcreation/MergeReorgTest.java | 5 +++- .../qbft/support/TestContextBuilder.java | 2 +- .../blockcreation/AbstractBlockCreator.java | 5 +--- .../blockcreation/PoWBlockCreator.java | 5 +--- .../blockcreation/PoWMinerExecutor.java | 9 +++---- .../AbstractBlockCreatorTest.java | 5 +--- .../blockcreation/PoWBlockCreatorTest.java | 22 +++++----------- .../besu/ethereum/core/MiningParameters.java | 19 ++++++++++++-- .../bonsai/AbstractIsolationTests.java | 5 +--- .../ethereum/retesteth/RetestethContext.java | 26 ++++++++++--------- .../retesteth/methods/TestMineBlocks.java | 15 ++--------- 29 files changed, 90 insertions(+), 102 deletions(-) diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java index 287f4fe6cfe..6e925f6a2c7 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java @@ -30,6 +30,7 @@ import org.hyperledger.besu.ethereum.api.tls.FileBasedPasswordProvider; import org.hyperledger.besu.ethereum.core.AddressHelpers; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.UpdatableInitValues; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.p2p.config.NetworkingConfiguration; @@ -56,7 +57,10 @@ public class BesuNodeConfigurationBuilder { private String name; private Optional dataPath = Optional.empty(); private MiningParameters miningParameters = - ImmutableMiningParameters.builder().coinbase(AddressHelpers.ofValue(1)).build(); + ImmutableMiningParameters.builder() + .updatableInitValues( + UpdatableInitValues.builder().coinbase(AddressHelpers.ofValue(1)).build()) + .build(); private JsonRpcConfiguration jsonRpcConfiguration = JsonRpcConfiguration.createDefault(); private JsonRpcConfiguration engineRpcConfiguration = JsonRpcConfiguration.createEngineDefault(); diff --git a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java index fc61b998a50..7960fd46437 100644 --- a/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java +++ b/acceptance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeFactory.java @@ -310,8 +310,8 @@ public BesuNode createNodeWithMultiTenantedPrivacy( UpdatableInitValues.builder() .isMiningEnabled(true) .minTransactionGasPrice(Wei.ZERO) + .coinbase(AddressHelpers.ofValue(1)) .build()) - .coinbase(AddressHelpers.ofValue(1)) .build(); return create( diff --git a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bft/BftMiningAcceptanceTest.java b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bft/BftMiningAcceptanceTest.java index b4732cc7113..8093661ebeb 100644 --- a/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bft/BftMiningAcceptanceTest.java +++ b/acceptance-tests/tests/src/test/java/org/hyperledger/besu/tests/acceptance/bft/BftMiningAcceptanceTest.java @@ -66,8 +66,8 @@ public void shouldMineOnSingleNodeWithFreeGas_Berlin() throws Exception { UpdatableInitValues.builder() .isMiningEnabled(true) .minTransactionGasPrice(Wei.ZERO) + .coinbase(AddressHelpers.ofValue(1)) .build()) - .coinbase(AddressHelpers.ofValue(1)) .build(); minerNode.setMiningParameters(zeroGasMiningParams); diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MiningOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MiningOptions.java index b1b87321af5..29733ed0b89 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MiningOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MiningOptions.java @@ -30,6 +30,7 @@ import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.UpdatableInitValues; import org.hyperledger.besu.ethereum.core.MiningParameters; import java.util.List; @@ -262,7 +263,7 @@ static MiningOptions fromConfig(final MiningParameters miningParameters) { @Override public MiningParameters toDomainObject() { final var updatableInitValuesBuilder = - ImmutableMiningParameters.UpdatableInitValues.builder() + UpdatableInitValues.builder() .isMiningEnabled(isMiningEnabled) .extraData(extraData) .minTransactionGasPrice(minTransactionGasPrice) @@ -271,6 +272,9 @@ public MiningParameters toDomainObject() { if (targetGasLimit != null) { updatableInitValuesBuilder.targetGasLimit(targetGasLimit); } + if (coinbase != null) { + updatableInitValuesBuilder.coinbase(coinbase); + } final var miningParametersBuilder = ImmutableMiningParameters.builder() @@ -290,10 +294,6 @@ public MiningParameters toDomainObject() { unstableOptions.posBlockCreationRepetitionMinDuration) .build()); - if (coinbase != null) { - miningParametersBuilder.coinbase(coinbase); - } - return miningParametersBuilder.build(); } diff --git a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/blocks/BlocksSubCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/blocks/BlocksSubCommand.java index c8579c2263f..3242ce9799a 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/subcommands/blocks/BlocksSubCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/subcommands/blocks/BlocksSubCommand.java @@ -275,8 +275,8 @@ private MiningParameters getMiningParameters() { .nonceGenerator(new IncrementingNonceGenerator(0)) .extraData(extraData) .minTransactionGasPrice(minTransactionGasPrice) + .coinbase(coinbase) .build()) - .coinbase(coinbase) .build(); } diff --git a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java index 40765769e6e..1f9fd70f745 100644 --- a/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java +++ b/besu/src/main/java/org/hyperledger/besu/controller/IbftBesuControllerBuilder.java @@ -145,6 +145,7 @@ protected MiningCoordinator createMiningCoordinator( final Address localAddress = Util.publicKeyToAddress(nodeKey.getPublicKey()); final BftProtocolSchedule bftProtocolSchedule = (BftProtocolSchedule) protocolSchedule; + miningParameters.setCoinbase(localAddress); final BftBlockCreatorFactory blockCreatorFactory = new BftBlockCreatorFactory<>( transactionPool, diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java index 013da67e8c2..8f2124ddfa9 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java @@ -71,6 +71,7 @@ import org.hyperledger.besu.ethereum.api.jsonrpc.websocket.WebSocketConfiguration; import org.hyperledger.besu.ethereum.api.tls.TlsConfiguration; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.UpdatableInitValues; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.PrivacyParameters; import org.hyperledger.besu.ethereum.eth.sync.SyncMode; @@ -902,7 +903,10 @@ public void envVariableOverridesValueFromConfigFile() { verify(mockControllerBuilder) .miningParameters( ImmutableMiningParameters.builder() - .coinbase(Address.fromHexString(expectedCoinbase)) + .updatableInitValues( + UpdatableInitValues.builder() + .coinbase(Address.fromHexString(expectedCoinbase)) + .build()) .build()); } @@ -916,7 +920,10 @@ public void cliOptionOverridesEnvVariableAndConfig() { verify(mockControllerBuilder) .miningParameters( ImmutableMiningParameters.builder() - .coinbase(Address.fromHexString(expectedCoinbase)) + .updatableInitValues( + UpdatableInitValues.builder() + .coinbase(Address.fromHexString(expectedCoinbase)) + .build()) .build()); } diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/MiningOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/stable/MiningOptionsTest.java index 5fe050067ad..95a83e7ce87 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/MiningOptionsTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/options/stable/MiningOptionsTest.java @@ -322,8 +322,8 @@ protected MiningParameters createCustomizedDomainObject() { .isMiningEnabled(true) .extraData(Bytes.fromHexString("0xabc321")) .minBlockOccupancyRatio(0.5) + .coinbase(Address.ZERO) .build()) - .coinbase(Address.ZERO) .isStratumMiningEnabled(true) .unstable(Unstable.builder().posBlockCreationMaxTime(1000).build()) .build(); diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java index 1f4206b7ec8..d991fb9db4c 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueBlockCreator.java @@ -23,7 +23,6 @@ import org.hyperledger.besu.consensus.common.EpochManager; import org.hyperledger.besu.consensus.common.validator.ValidatorVote; import org.hyperledger.besu.cryptoservices.NodeKey; -import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Hash; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.blockcreation.AbstractBlockCreator; @@ -49,7 +48,6 @@ public class CliqueBlockCreator extends AbstractBlockCreator { * Instantiates a new Clique block creator. * * @param miningParameters the mining parameters - * @param coinbase the coinbase * @param extraDataCalculator the extra data calculator * @param transactionPool the pending transactions * @param protocolContext the protocol context @@ -60,7 +58,6 @@ public class CliqueBlockCreator extends AbstractBlockCreator { */ public CliqueBlockCreator( final MiningParameters miningParameters, - final Address coinbase, final ExtraDataCalculator extraDataCalculator, final TransactionPool transactionPool, final ProtocolContext protocolContext, @@ -70,7 +67,6 @@ public CliqueBlockCreator( final EpochManager epochManager) { super( miningParameters, - coinbase, __ -> Util.publicKeyToAddress(nodeKey.getPublicKey()), extraDataCalculator, transactionPool, diff --git a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java index ac06e776e44..81b754b267c 100644 --- a/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java +++ b/consensus/clique/src/main/java/org/hyperledger/besu/consensus/clique/blockcreation/CliqueMinerExecutor.java @@ -71,6 +71,7 @@ public CliqueMinerExecutor( this.nodeKey = nodeKey; this.localAddress = Util.publicKeyToAddress(nodeKey.getPublicKey()); this.epochManager = epochManager; + miningParams.setCoinbase(localAddress); } @Override @@ -82,7 +83,6 @@ public CliqueBlockMiner createMiner( (header) -> new CliqueBlockCreator( miningParameters, - localAddress, // TOOD(tmm): This can be removed (used for voting not coinbase). this::calculateExtraData, transactionPool, protocolContext, @@ -103,7 +103,7 @@ public CliqueBlockMiner createMiner( @Override public Optional
getCoinbase() { - return Optional.of(localAddress); + return miningParameters.getCoinbase(); } /** 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 787eff3bda6..208d5ac64b5 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 @@ -141,7 +141,6 @@ public void proposerAddressCanBeExtractFromAConstructedBlock() { final CliqueBlockCreator blockCreator = new CliqueBlockCreator( miningParameters, - coinbase, parent -> extraData, createTransactionPool(), protocolContext, @@ -170,7 +169,6 @@ public void insertsValidVoteIntoConstructedBlock() { final CliqueBlockCreator blockCreator = new CliqueBlockCreator( miningParameters, - coinbase, parent -> extraData, createTransactionPool(), protocolContext, @@ -204,7 +202,6 @@ public void insertsNoVoteWhenAtEpoch() { final CliqueBlockCreator blockCreator = new CliqueBlockCreator( miningParameters, - coinbase, parent -> extraData, createTransactionPool(), protocolContext, @@ -252,8 +249,8 @@ private static MiningParameters createMiningParameters( .extraData(extraData) .targetGasLimit(10_000_000L) .minTransactionGasPrice(Wei.ZERO) + .coinbase(coinbase) .build()) - .coinbase(coinbase) .build(); return miningParameters; } 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 6bd197cd908..a538a491b67 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 @@ -240,8 +240,8 @@ private static MiningParameters createMiningParameters(final Bytes vanityData) { UpdatableInitValues.builder() .extraData(vanityData) .minTransactionGasPrice(Wei.ZERO) + .coinbase(AddressHelpers.ofValue(1)) .build()) - .coinbase(AddressHelpers.ofValue(1)) .build(); } } diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java index 692d64e7988..47bf915a142 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java @@ -63,7 +63,6 @@ public BftBlockCreator( final BftExtraDataCodec bftExtraDataCodec) { super( miningParameters, - localAddress, miningBeneficiaryCalculator(localAddress, forksSchedule), extraDataCalculator, transactionPool, 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 88eba5b8930..41fbbbe1940 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 @@ -312,8 +312,8 @@ private static ControllerAndState createControllerAndFinalState( .isMiningEnabled(true) .minTransactionGasPrice(Wei.ZERO) .extraData(Bytes.wrap("Ibft Int tests".getBytes(UTF_8))) + .coinbase(AddressHelpers.ofValue(1)) .build()) - .coinbase(AddressHelpers.ofValue(1)) .build(); final StubGenesisConfigOptions genesisConfigOptions = new StubGenesisConfigOptions(); diff --git a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java index 14c1eddc70f..afd967a2caf 100644 --- a/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java +++ b/consensus/ibft/src/test/java/org/hyperledger/besu/consensus/ibft/blockcreation/BftBlockCreatorTest.java @@ -165,8 +165,8 @@ public BlockHeaderValidator.Builder createBlockHeaderRuleset( 0, initialValidatorList))) .minTransactionGasPrice(Wei.ZERO) + .coinbase(AddressHelpers.ofValue(1)) .build()) - .coinbase(AddressHelpers.ofValue(1)) .build(); final BftBlockCreator blockCreator = diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeBlockCreator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeBlockCreator.java index 294e0943598..b81a941816a 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeBlockCreator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeBlockCreator.java @@ -44,7 +44,6 @@ class MergeBlockCreator extends AbstractBlockCreator { * @param transactionPool the pending transactions * @param protocolContext the protocol context * @param protocolSchedule the protocol schedule - * @param miningBeneficiary the mining beneficiary * @param parentHeader the parent header */ public MergeBlockCreator( @@ -53,13 +52,11 @@ public MergeBlockCreator( final TransactionPool transactionPool, final ProtocolContext protocolContext, final ProtocolSchedule protocolSchedule, - final Address miningBeneficiary, final BlockHeader parentHeader, final Optional
depositContractAddress) { super( miningParameters, - miningBeneficiary, - __ -> miningBeneficiary, + __ -> miningParameters.getCoinbase().orElseThrow(), extraDataCalculator, transactionPool, protocolContext, diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index 308fd9b4aba..b83e607ef4b 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -118,6 +118,9 @@ public MergeCoordinator( this.mergeContext = protocolContext.getConsensusContext(MergeContext.class); this.backwardSyncContext = backwardSyncContext; + if (miningParams.getCoinbase().isEmpty()) { + miningParams.setCoinbase(Address.ZERO); + } if (miningParams.getTargetGasLimit().isEmpty()) { miningParams.setTargetGasLimit(30000000L); } @@ -133,7 +136,6 @@ public MergeCoordinator( transactionPool, protocolContext, protocolSchedule, - address.or(miningParameters::getCoinbase).orElse(Address.ZERO), parentHeader, depositContractAddress); diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java index 28fe40d5d70..3eeb87539bd 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinatorTest.java @@ -57,6 +57,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.Unstable; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.UpdatableInitValues; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.TransactionTestFixture; @@ -134,9 +135,9 @@ public class MergeCoordinatorTest implements MergeGenesisConfigHelper { MiningParameters miningParameters = ImmutableMiningParameters.builder() - .coinbase(coinbase) + .updatableInitValues(UpdatableInitValues.builder().coinbase(coinbase).build()) .unstable( - ImmutableMiningParameters.Unstable.builder() + Unstable.builder() .posBlockCreationRepetitionMinDuration(REPETITION_MIN_DURATION) .build()) .build(); @@ -285,7 +286,6 @@ public void exceptionDuringBuildingBlockShouldNotBeInvalid() transactionPool, protocolContext, protocolSchedule, - address.or(miningParameters::getCoinbase).orElse(Address.ZERO), parentHeader, Optional.empty())); @@ -551,8 +551,7 @@ public void shouldStopRetryBlockCreationIfTimeExpired() throws InterruptedExcept miningParameters = ImmutableMiningParameters.builder() .from(miningParameters) - .unstable( - ImmutableMiningParameters.Unstable.builder().posBlockCreationMaxTime(100).build()) + .unstable(Unstable.builder().posBlockCreationMaxTime(100).build()) .build(); doAnswer( invocation -> { diff --git a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeReorgTest.java b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeReorgTest.java index 4c9712c9b47..75de7e190d3 100644 --- a/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeReorgTest.java +++ b/consensus/merge/src/test/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeReorgTest.java @@ -34,6 +34,7 @@ import org.hyperledger.besu.ethereum.core.BlockHeaderTestFixture; import org.hyperledger.besu.ethereum.core.Difficulty; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.UpdatableInitValues; import org.hyperledger.besu.ethereum.eth.sync.backwardsync.BackwardSyncContext; import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool; import org.hyperledger.besu.ethereum.mainnet.BlockHeaderValidator; @@ -93,7 +94,9 @@ public void setUp() { mockProtocolSchedule, CompletableFuture::runAsync, mockTransactionPool, - ImmutableMiningParameters.builder().coinbase(coinbase).build(), + ImmutableMiningParameters.builder() + .updatableInitValues(UpdatableInitValues.builder().coinbase(coinbase).build()) + .build(), mock(BackwardSyncContext.class), Optional.empty()); mergeContext.setIsPostMerge(genesisState.getBlock().getHeader().getDifficulty()); diff --git a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java index afbc2742637..c1a05a9709e 100644 --- a/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java +++ b/consensus/qbft/src/integration-test/java/org/hyperledger/besu/consensus/qbft/support/TestContextBuilder.java @@ -372,8 +372,8 @@ private static ControllerAndState createControllerAndFinalState( .isMiningEnabled(true) .minTransactionGasPrice(Wei.ZERO) .extraData(Bytes.wrap("Qbft Int tests".getBytes(UTF_8))) + .coinbase(AddressHelpers.ofValue(1)) .build()) - .coinbase(AddressHelpers.ofValue(1)) .build(); final StubGenesisConfigOptions genesisConfigOptions = new StubGenesisConfigOptions(); diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java index 060771b7ca6..a8bb17e93cd 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreator.java @@ -79,7 +79,6 @@ public interface ExtraDataCalculator { private static final Logger LOG = LoggerFactory.getLogger(AbstractBlockCreator.class); - protected final Address coinbase; private final MiningBeneficiaryCalculator miningBeneficiaryCalculator; private final ExtraDataCalculator extraDataCalculator; private final TransactionPool transactionPool; @@ -94,7 +93,6 @@ public interface ExtraDataCalculator { protected AbstractBlockCreator( final MiningParameters miningParameters, - final Address coinbase, final MiningBeneficiaryCalculator miningBeneficiaryCalculator, final ExtraDataCalculator extraDataCalculator, final TransactionPool transactionPool, @@ -103,7 +101,6 @@ protected AbstractBlockCreator( final BlockHeader parentHeader, final Optional
depositContractAddress) { this.miningParameters = miningParameters; - this.coinbase = coinbase; this.miningBeneficiaryCalculator = miningBeneficiaryCalculator; this.extraDataCalculator = extraDataCalculator; this.transactionPool = transactionPool; @@ -411,7 +408,7 @@ private ProcessableBlockHeader createPendingBlockHeader( final Bytes32 parentBeaconBlockRoot = maybeParentBeaconBlockRoot.orElse(null); return BlockHeaderBuilder.create() .parentHash(parentHeader.getHash()) - .coinbase(coinbase) + .coinbase(miningParameters.getCoinbase().orElseThrow()) .difficulty(Difficulty.of(difficulty)) .number(newBlockNumber) .gasLimit(gasLimit) diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreator.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreator.java index 032ad0b8fed..66c57379ccb 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreator.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreator.java @@ -14,7 +14,6 @@ */ package org.hyperledger.besu.ethereum.blockcreation; -import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderBuilder; @@ -40,7 +39,6 @@ public class PoWBlockCreator extends AbstractBlockCreator { public PoWBlockCreator( final MiningParameters miningParameters, - final Address coinbase, final ExtraDataCalculator extraDataCalculator, final TransactionPool transactionPool, final ProtocolContext protocolContext, @@ -49,8 +47,7 @@ public PoWBlockCreator( final BlockHeader parentHeader) { super( miningParameters, - coinbase, - __ -> coinbase, + __ -> miningParameters.getCoinbase().orElseThrow(), extraDataCalculator, transactionPool, protocolContext, diff --git a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutor.java b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutor.java index f10fe3d7771..cd44318e960 100644 --- a/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutor.java +++ b/ethereum/blockcreation/src/main/java/org/hyperledger/besu/ethereum/blockcreation/PoWMinerExecutor.java @@ -32,7 +32,6 @@ public class PoWMinerExecutor extends AbstractMinerExecutor { - protected volatile Optional
coinbase; protected boolean stratumMiningEnabled; protected final EpochCalculator epochCalculator; @@ -44,7 +43,6 @@ public PoWMinerExecutor( final AbstractBlockScheduler blockScheduler, final EpochCalculator epochCalculator) { super(protocolContext, protocolSchedule, transactionPool, miningParams, blockScheduler); - this.coinbase = miningParams.getCoinbase(); if (miningParams.getNonceGenerator().isEmpty()) { miningParams.setNonceGenerator(new RandomNonceGenerator()); } @@ -56,7 +54,7 @@ public Optional startAsyncMining( final Subscribers observers, final Subscribers ethHashObservers, final BlockHeader parentHeader) { - if (coinbase.isEmpty()) { + if (miningParameters.getCoinbase().isEmpty()) { throw new CoinbaseNotSetException("Unable to start mining without a coinbase."); } return super.startAsyncMining(observers, ethHashObservers, parentHeader); @@ -82,7 +80,6 @@ public PoWBlockMiner createMiner( (header) -> new PoWBlockCreator( miningParameters, - coinbase.orElse(Address.ZERO), parent -> miningParameters.getExtraData(), transactionPool, protocolContext, @@ -98,7 +95,7 @@ public void setCoinbase(final Address coinbase) { if (coinbase == null) { throw new IllegalArgumentException("Coinbase cannot be unset."); } else { - this.coinbase = Optional.of(Address.wrap(coinbase.copy())); + miningParameters.setCoinbase(Address.wrap(coinbase.copy())); } } @@ -108,7 +105,7 @@ void setStratumMiningEnabled(final boolean stratumMiningEnabled) { @Override public Optional
getCoinbase() { - return coinbase; + return miningParameters.getCoinbase(); } public EpochCalculator getEpochCalculator() { diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java index 8a2e91f664f..3ed629cb935 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/AbstractBlockCreatorTest.java @@ -392,13 +392,12 @@ private AbstractBlockCreator createBlockCreator( .extraData(Bytes.fromHexString("deadbeef")) .minTransactionGasPrice(Wei.ONE) .minBlockOccupancyRatio(0d) + .coinbase(Address.ZERO) .build()) - .coinbase(Address.ZERO) .build(); return new TestBlockCreator( miningParameters, - Address.ZERO, __ -> Address.ZERO, __ -> Bytes.fromHexString("deadbeef"), transactionPool, @@ -412,7 +411,6 @@ static class TestBlockCreator extends AbstractBlockCreator { protected TestBlockCreator( final MiningParameters miningParameters, - final Address coinbase, final MiningBeneficiaryCalculator miningBeneficiaryCalculator, final ExtraDataCalculator extraDataCalculator, final TransactionPool transactionPool, @@ -422,7 +420,6 @@ protected TestBlockCreator( final Optional
depositContractAddress) { super( miningParameters, - coinbase, miningBeneficiaryCalculator, extraDataCalculator, transactionPool, diff --git a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java index 2d428dcc054..cddb0073f8d 100644 --- a/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java +++ b/ethereum/blockcreation/src/test/java/org/hyperledger/besu/ethereum/blockcreation/PoWBlockCreatorTest.java @@ -94,8 +94,7 @@ void createMainnetBlock1() throws IOException { .createProtocolSchedule()) .build(); - final MiningParameters miningParameters = - createMiningParameters(Lists.newArrayList(BLOCK_1_NONCE)); + final MiningParameters miningParameters = createMiningParameters(); final PoWSolver solver = new PoWSolver( @@ -110,7 +109,6 @@ void createMainnetBlock1() throws IOException { final PoWBlockCreator blockCreator = new PoWBlockCreator( miningParameters, - BLOCK_1_COINBASE, parent -> BLOCK_1_EXTRA_DATA, transactionPool, executionContextTestFixture.getProtocolContext(), @@ -149,8 +147,7 @@ void createMainnetBlock1_fixedDifficulty1() { .createProtocolSchedule()) .build(); - final MiningParameters miningParameters = - createMiningParameters(Lists.newArrayList(BLOCK_1_NONCE)); + final MiningParameters miningParameters = createMiningParameters(); final PoWSolver solver = new PoWSolver( @@ -165,7 +162,6 @@ void createMainnetBlock1_fixedDifficulty1() { final PoWBlockCreator blockCreator = new PoWBlockCreator( miningParameters, - BLOCK_1_COINBASE, parent -> BLOCK_1_EXTRA_DATA, transactionPool, executionContextTestFixture.getProtocolContext(), @@ -195,8 +191,7 @@ void rewardBeneficiary_zeroReward_skipZeroRewardsFalse() { final ExecutionContextTestFixture executionContextTestFixture = ExecutionContextTestFixture.builder().protocolSchedule(protocolSchedule).build(); - final MiningParameters miningParameters = - createMiningParameters(Lists.newArrayList(BLOCK_1_NONCE)); + final MiningParameters miningParameters = createMiningParameters(); final PoWSolver solver = new PoWSolver( @@ -211,7 +206,6 @@ void rewardBeneficiary_zeroReward_skipZeroRewardsFalse() { final PoWBlockCreator blockCreator = new PoWBlockCreator( miningParameters, - BLOCK_1_COINBASE, parent -> BLOCK_1_EXTRA_DATA, transactionPool, executionContextTestFixture.getProtocolContext(), @@ -263,8 +257,7 @@ void rewardBeneficiary_zeroReward_skipZeroRewardsTrue() { final ExecutionContextTestFixture executionContextTestFixture = ExecutionContextTestFixture.builder().protocolSchedule(protocolSchedule).build(); - final MiningParameters miningParameters = - createMiningParameters(Lists.newArrayList(BLOCK_1_NONCE)); + final MiningParameters miningParameters = createMiningParameters(); final PoWSolver solver = new PoWSolver( @@ -279,7 +272,6 @@ void rewardBeneficiary_zeroReward_skipZeroRewardsTrue() { final PoWBlockCreator blockCreator = new PoWBlockCreator( miningParameters, - BLOCK_1_COINBASE, parent -> BLOCK_1_EXTRA_DATA, transactionPool, executionContextTestFixture.getProtocolContext(), @@ -344,15 +336,15 @@ private TransactionPool createTransactionPool( return transactionPool; } - private MiningParameters createMiningParameters(final Iterable nonceList) { + private MiningParameters createMiningParameters() { return ImmutableMiningParameters.builder() .updatableInitValues( ImmutableMiningParameters.UpdatableInitValues.builder() - .nonceGenerator(nonceList) + .nonceGenerator(Lists.newArrayList(BLOCK_1_NONCE)) .extraData(BLOCK_1_EXTRA_DATA) .minTransactionGasPrice(Wei.ONE) + .coinbase(BLOCK_1_COINBASE) .build()) - .coinbase(BLOCK_1_COINBASE) .build(); } } diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/MiningParameters.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/MiningParameters.java index 298dc87d573..ea14d863839 100644 --- a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/MiningParameters.java +++ b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/core/MiningParameters.java @@ -40,8 +40,6 @@ public static final MiningParameters newDefault() { return ImmutableMiningParameters.builder().build(); } - public abstract Optional
getCoinbase(); - public boolean isMiningEnabled() { return getUpdatableRuntimeValues().miningEnabled; } @@ -69,6 +67,15 @@ public MiningParameters setMinTransactionGasPrice(final Wei minTransactionGasPri return this; } + public Optional
getCoinbase() { + return getUpdatableRuntimeValues().coinbase; + } + + public MiningParameters setCoinbase(final Address coinbase) { + getUpdatableRuntimeValues().coinbase = Optional.of(coinbase); + return this; + } + public OptionalLong getTargetGasLimit() { return getUpdatableRuntimeValues().targetGasLimit; } @@ -154,6 +161,8 @@ default double getMinBlockOccupancyRatio() { return DEFAULT_MIN_BLOCK_OCCUPANCY_RATIO; } + Optional
getCoinbase(); + OptionalLong getTargetGasLimit(); Optional> nonceGenerator(); @@ -164,6 +173,7 @@ static class UpdatableRuntimeValues { private volatile Bytes extraData; private volatile Wei minTransactionGasPrice; private volatile double minBlockOccupancyRatio; + private volatile Optional
coinbase; private volatile OptionalLong targetGasLimit; private volatile Optional> nonceGenerator; @@ -172,6 +182,7 @@ private UpdatableRuntimeValues(final UpdatableInitValues initValues) { extraData = initValues.getExtraData(); minTransactionGasPrice = initValues.getMinTransactionGasPrice(); minBlockOccupancyRatio = initValues.getMinBlockOccupancyRatio(); + coinbase = initValues.getCoinbase(); targetGasLimit = initValues.getTargetGasLimit(); nonceGenerator = initValues.nonceGenerator(); } @@ -185,6 +196,7 @@ public boolean equals(final Object o) { && Double.compare(minBlockOccupancyRatio, that.minBlockOccupancyRatio) == 0 && Objects.equals(extraData, that.extraData) && Objects.equals(minTransactionGasPrice, that.minTransactionGasPrice) + && Objects.equals(coinbase, that.coinbase) && Objects.equals(targetGasLimit, that.targetGasLimit) && Objects.equals(nonceGenerator, that.nonceGenerator); } @@ -196,6 +208,7 @@ public int hashCode() { extraData, minTransactionGasPrice, minBlockOccupancyRatio, + coinbase, targetGasLimit, nonceGenerator); } @@ -211,6 +224,8 @@ public String toString() { + minTransactionGasPrice + ", minBlockOccupancyRatio=" + minBlockOccupancyRatio + + ", coinbase=" + + coinbase + ", targetGasLimit=" + targetGasLimit + ", nonceGenerator=" diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java index a51846b528e..0d30843cb38 100644 --- a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java +++ b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/bonsai/AbstractIsolationTests.java @@ -207,7 +207,6 @@ public int getDatabaseVersion() { static class TestBlockCreator extends AbstractBlockCreator { private TestBlockCreator( final MiningParameters miningParameters, - final Address coinbase, final MiningBeneficiaryCalculator miningBeneficiaryCalculator, final ExtraDataCalculator extraDataCalculator, final TransactionPool transactionPool, @@ -216,7 +215,6 @@ private TestBlockCreator( final BlockHeader parentHeader) { super( miningParameters, - coinbase, miningBeneficiaryCalculator, extraDataCalculator, transactionPool, @@ -240,13 +238,12 @@ static TestBlockCreator forHeader( .targetGasLimit(30_000_000L) .minTransactionGasPrice(Wei.ONE) .minBlockOccupancyRatio(0d) + .coinbase(Address.ZERO) .build()) - .coinbase(Address.ZERO) .build(); return new TestBlockCreator( miningParameters, - Address.ZERO, __ -> Address.ZERO, __ -> Bytes.fromHexString("deadbeef"), transactionPool, diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java index 706120b1bfc..2327bfaae07 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/RetestethContext.java @@ -32,6 +32,8 @@ import org.hyperledger.besu.ethereum.core.BlockHeader; import org.hyperledger.besu.ethereum.core.BlockHeaderFunctions; import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.Unstable; +import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters.UpdatableInitValues; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.core.MutableWorldState; import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration; @@ -99,7 +101,7 @@ public class RetestethContext { private HeaderValidationMode headerValidationMode; private BlockReplay blockReplay; private RetestethClock retestethClock; - + private MiningParameters miningParameters; private TransactionPool transactionPool; private EthScheduler ethScheduler; private PoWSolver poWSolver; @@ -180,13 +182,17 @@ private boolean buildContext( ? HeaderValidationMode.LIGHT : HeaderValidationMode.FULL; - final MiningParameters miningParameters = + miningParameters = ImmutableMiningParameters.builder() - .unstable( - ImmutableMiningParameters.Unstable.builder() - .powJobTimeToLive(1000) - .maxOmmerDepth(8) + .updatableInitValues( + UpdatableInitValues.builder() + .coinbase(coinbase) + .extraData(extraData) + .targetGasLimit(blockchain.getChainHeadHeader().getGasLimit()) + .minBlockOccupancyRatio(0.0) + .minTransactionGasPrice(Wei.ZERO) .build()) + .unstable(Unstable.builder().powJobTimeToLive(1000).maxOmmerDepth(8).build()) .build(); miningParameters.setMinTransactionGasPrice(Wei.ZERO); poWSolver = @@ -311,12 +317,8 @@ public TransactionPool getTransactionPool() { return transactionPool; } - public Address getCoinbase() { - return coinbase; - } - - public Bytes getExtraData() { - return extraData; + public MiningParameters getMiningParameters() { + return miningParameters; } public MutableBlockchain getBlockchain() { diff --git a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/methods/TestMineBlocks.java b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/methods/TestMineBlocks.java index 4b6b868383d..6c3fddae1c7 100644 --- a/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/methods/TestMineBlocks.java +++ b/ethereum/retesteth/src/main/java/org/hyperledger/besu/ethereum/retesteth/methods/TestMineBlocks.java @@ -14,7 +14,6 @@ */ package org.hyperledger.besu.ethereum.retesteth.methods; -import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.ProtocolContext; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.JsonRpcRequestContext; import org.hyperledger.besu.ethereum.api.jsonrpc.internal.methods.JsonRpcMethod; @@ -24,7 +23,6 @@ import org.hyperledger.besu.ethereum.chain.MutableBlockchain; import org.hyperledger.besu.ethereum.core.Block; import org.hyperledger.besu.ethereum.core.BlockImporter; -import org.hyperledger.besu.ethereum.core.ImmutableMiningParameters; import org.hyperledger.besu.ethereum.core.MiningParameters; import org.hyperledger.besu.ethereum.mainnet.BlockImportResult; import org.hyperledger.besu.ethereum.mainnet.HeaderValidationMode; @@ -62,20 +60,11 @@ private boolean mineNewBlock() { final ProtocolContext protocolContext = context.getProtocolContext(); final MutableBlockchain blockchain = context.getBlockchain(); final HeaderValidationMode headerValidationMode = context.getHeaderValidationMode(); - final MiningParameters miningParameters = - ImmutableMiningParameters.builder() - .updatableInitValues( - ImmutableMiningParameters.UpdatableInitValues.builder() - .targetGasLimit(blockchain.getChainHeadHeader().getGasLimit()) - .minBlockOccupancyRatio(0.0) - .minTransactionGasPrice(Wei.ZERO) - .build()) - .build(); + final MiningParameters miningParameters = context.getMiningParameters(); final PoWBlockCreator blockCreator = new PoWBlockCreator( miningParameters, - context.getCoinbase(), - header -> context.getExtraData(), + header -> miningParameters.getExtraData(), context.getTransactionPool(), protocolContext, protocolSchedule, From 4626230467a960ed2f4a6b8a7085d4775c19674b Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 17 Oct 2023 17:48:09 +0200 Subject: [PATCH 11/12] Move MiningOptions to upper package Signed-off-by: Fabio Di Fabio --- .../org/hyperledger/besu/cli/BesuCommand.java | 2 +- .../options/{stable => }/MiningOptions.java | 3 +-- .../besu/cli/CommandTestAbstract.java | 2 +- .../{stable => }/MiningOptionsTest.java | 3 +-- .../merge/blockcreation/MergeCoordinator.java | 20 ++++++++++--------- plugin-api/build.gradle | 2 +- 6 files changed, 16 insertions(+), 16 deletions(-) rename besu/src/main/java/org/hyperledger/besu/cli/options/{stable => }/MiningOptions.java (99%) rename besu/src/test/java/org/hyperledger/besu/cli/options/{stable => }/MiningOptionsTest.java (99%) diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java index 916face134e..2e63b236342 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java @@ -53,10 +53,10 @@ import org.hyperledger.besu.cli.custom.RpcAuthFileValidator; import org.hyperledger.besu.cli.error.BesuExecutionExceptionHandler; import org.hyperledger.besu.cli.error.BesuParameterExceptionHandler; +import org.hyperledger.besu.cli.options.MiningOptions; import org.hyperledger.besu.cli.options.stable.DataStorageOptions; import org.hyperledger.besu.cli.options.stable.EthstatsOptions; import org.hyperledger.besu.cli.options.stable.LoggingLevelOption; -import org.hyperledger.besu.cli.options.stable.MiningOptions; import org.hyperledger.besu.cli.options.stable.NodePrivateKeyFileOption; import org.hyperledger.besu.cli.options.stable.P2PTLSConfigOptions; import org.hyperledger.besu.cli.options.stable.TransactionPoolOptions; diff --git a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MiningOptions.java b/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java similarity index 99% rename from besu/src/main/java/org/hyperledger/besu/cli/options/stable/MiningOptions.java rename to besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java index 29733ed0b89..aff3212d0b1 100644 --- a/besu/src/main/java/org/hyperledger/besu/cli/options/stable/MiningOptions.java +++ b/besu/src/main/java/org/hyperledger/besu/cli/options/MiningOptions.java @@ -12,7 +12,7 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.cli.options.stable; +package org.hyperledger.besu.cli.options; import static java.util.Arrays.asList; import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_MAX_OMMERS_DEPTH; @@ -25,7 +25,6 @@ import static org.hyperledger.besu.ethereum.core.MiningParameters.UpdatableInitValues.DEFAULT_MIN_BLOCK_OCCUPANCY_RATIO; import static org.hyperledger.besu.ethereum.core.MiningParameters.UpdatableInitValues.DEFAULT_MIN_TRANSACTION_GAS_PRICE; -import org.hyperledger.besu.cli.options.CLIOptions; import org.hyperledger.besu.cli.util.CommandLineUtils; import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; diff --git a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java index 84a45de2514..bccf28c4522 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/CommandTestAbstract.java @@ -31,8 +31,8 @@ import org.hyperledger.besu.chainimport.JsonBlockImporter; import org.hyperledger.besu.chainimport.RlpBlockImporter; import org.hyperledger.besu.cli.config.EthNetworkConfig; +import org.hyperledger.besu.cli.options.MiningOptions; import org.hyperledger.besu.cli.options.stable.EthstatsOptions; -import org.hyperledger.besu.cli.options.stable.MiningOptions; import org.hyperledger.besu.cli.options.unstable.EthProtocolOptions; import org.hyperledger.besu.cli.options.unstable.MetricsCLIOptions; import org.hyperledger.besu.cli.options.unstable.NetworkingOptions; diff --git a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/MiningOptionsTest.java b/besu/src/test/java/org/hyperledger/besu/cli/options/MiningOptionsTest.java similarity index 99% rename from besu/src/test/java/org/hyperledger/besu/cli/options/stable/MiningOptionsTest.java rename to besu/src/test/java/org/hyperledger/besu/cli/options/MiningOptionsTest.java index 95a83e7ce87..5509df0fee5 100644 --- a/besu/src/test/java/org/hyperledger/besu/cli/options/stable/MiningOptionsTest.java +++ b/besu/src/test/java/org/hyperledger/besu/cli/options/MiningOptionsTest.java @@ -12,14 +12,13 @@ * * SPDX-License-Identifier: Apache-2.0 */ -package org.hyperledger.besu.cli.options.stable; +package org.hyperledger.besu.cli.options; import static org.assertj.core.api.Assertions.assertThat; import static org.hyperledger.besu.ethereum.core.MiningParameters.Unstable.DEFAULT_POS_BLOCK_CREATION_MAX_TIME; import static org.mockito.Mockito.atMost; import static org.mockito.Mockito.verify; -import org.hyperledger.besu.cli.options.AbstractCLIOptionsTest; import org.hyperledger.besu.datatypes.Address; import org.hyperledger.besu.datatypes.Wei; import org.hyperledger.besu.ethereum.GasLimitCalculator; diff --git a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java index b83e607ef4b..60a88649a04 100644 --- a/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java +++ b/consensus/merge/src/main/java/org/hyperledger/besu/consensus/merge/blockcreation/MergeCoordinator.java @@ -129,15 +129,17 @@ public MergeCoordinator( this.miningParameters = miningParams; this.mergeBlockCreatorFactory = - (parentHeader, address) -> - new MergeBlockCreator( - miningParameters, - parent -> miningParameters.getExtraData(), - transactionPool, - protocolContext, - protocolSchedule, - parentHeader, - depositContractAddress); + (parentHeader, address) -> { + address.ifPresent(miningParams::setCoinbase); + return new MergeBlockCreator( + miningParameters, + parent -> miningParameters.getExtraData(), + transactionPool, + protocolContext, + protocolSchedule, + parentHeader, + depositContractAddress); + }; this.backwardSyncContext.subscribeBadChainListener(this); } diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index 66877c43c9d..f79852f2011 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 = 'aRwRZCqqkThbFyseJ9VRqe9rCoLsnsaL5vgfSOHylpY=' + knownHash = 'j6NRklFHlG35Pq/t6t/oJBrT8DbYOyruGq3cJNh4ENw=' } check.dependsOn('checkAPIChanges') From 25b152264bb71bf5b6f6ed3a19187e53d8634afa Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Tue, 17 Oct 2023 18:48:20 +0200 Subject: [PATCH 12/12] Fix coinbase for *bft Signed-off-by: Fabio Di Fabio --- .../consensus/common/bft/blockcreation/BftBlockCreator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java index 47bf915a142..4c5e773966f 100644 --- a/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java +++ b/consensus/common/src/main/java/org/hyperledger/besu/consensus/common/bft/blockcreation/BftBlockCreator.java @@ -62,7 +62,7 @@ public BftBlockCreator( final BlockHeader parentHeader, final BftExtraDataCodec bftExtraDataCodec) { super( - miningParameters, + miningParameters.setCoinbase(localAddress), miningBeneficiaryCalculator(localAddress, forksSchedule), extraDataCalculator, transactionPool,