From e0cd5084c7787a21c53e27f8591511887c84bf6b Mon Sep 17 00:00:00 2001 From: Luis Pinto Date: Thu, 12 Sep 2024 12:56:54 +0100 Subject: [PATCH] Change test to tease NPEs during DefaultBlockchain.() caused by initilization order (#7607) * Change test to tease NPEs during DefaultBlockchain.() caused by initilization order * fixup! Change test to tease NPEs during DefaultBlockchain.() caused by initilization order * move counters into their own method Signed-off-by: Luis Pinto --- CHANGELOG.md | 1 + .../ethereum/chain/DefaultBlockchain.java | 32 +++++++++------ .../ethereum/chain/DefaultBlockchainTest.java | 41 ++++++++++++------- 3 files changed, 47 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ea4ced222a..c38c30a4216 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ - Layered txpool: do not send notifications when moving tx between layers [#7539](https://github.com/hyperledger/besu/pull/7539) - Layered txpool: fix for unsent drop notifications on remove [#7538](https://github.com/hyperledger/besu/pull/7538) - Honor block number or tag parameter in eth_estimateGas and eth_createAccessList [#7502](https://github.com/hyperledger/besu/pull/7502) +- Fixed NPE during DefaultBlockchain object initialization [#7601](https://github.com/hyperledger/besu/pull/7601) ## 24.9.0 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 b2b4e6abfc8..bce94a19153 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.noop.NoOpMetricsSystem; import org.hyperledger.besu.metrics.prometheus.PrometheusMetricsSystem; import org.hyperledger.besu.plugin.services.MetricsSystem; import org.hyperledger.besu.plugin.services.metrics.Counter; @@ -84,8 +85,8 @@ public class DefaultBlockchain implements MutableBlockchain { private final Optional>> transactionReceiptsCache; private final Optional> totalDifficultyCache; - private final Counter gasUsedCounter; - private final Counter numberOfTransactionsCounter; + private Counter gasUsedCounter = NoOpMetricsSystem.NO_OP_COUNTER; + private Counter numberOfTransactionsCounter = NoOpMetricsSystem.NO_OP_COUNTER; private DefaultBlockchain( final Optional genesisBlock, @@ -147,6 +148,23 @@ private DefaultBlockchain( totalDifficultyCache = Optional.empty(); } + createCounters(metricsSystem); + createGauges(metricsSystem); + } + + private void createCounters(final MetricsSystem metricsSystem) { + gasUsedCounter = + metricsSystem.createCounter( + BesuMetricCategory.BLOCKCHAIN, "chain_head_gas_used_counter", "Counter for Gas used"); + + numberOfTransactionsCounter = + metricsSystem.createCounter( + BesuMetricCategory.BLOCKCHAIN, + "chain_head_transaction_count_counter", + "Counter for the number of transactions"); + } + + private void createGauges(final MetricsSystem metricsSystem) { metricsSystem.createLongGauge( BesuMetricCategory.ETHEREUM, "blockchain_height", @@ -183,10 +201,6 @@ private DefaultBlockchain( "Gas used by the current chain head block", () -> getChainHeadHeader().getGasUsed()); - gasUsedCounter = - metricsSystem.createCounter( - BesuMetricCategory.BLOCKCHAIN, "chain_head_gas_used_counter", "Counter for Gas used"); - metricsSystem.createLongGauge( BesuMetricCategory.BLOCKCHAIN, "chain_head_gas_limit", @@ -199,12 +213,6 @@ private DefaultBlockchain( "Number of transactions in the current chain head block", () -> chainHeadTransactionCount); - numberOfTransactionsCounter = - metricsSystem.createCounter( - BesuMetricCategory.BLOCKCHAIN, - "chain_head_transaction_count_counter", - "Counter for the number of transactions"); - metricsSystem.createIntegerGauge( BesuMetricCategory.BLOCKCHAIN, "chain_head_ommer_count", 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 69fbfcdbd65..7ad1d82bfd2 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 @@ -35,8 +35,10 @@ import org.hyperledger.besu.plugin.services.storage.KeyValueStorage; import org.hyperledger.besu.services.kvstore.InMemoryKeyValueStorage; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import java.util.Optional; import java.util.Set; @@ -161,33 +163,42 @@ public void initializeReadOnly_withGiantDifficultyAndLiveMetrics() { () -> BlockOptions.create().setDifficulty(Difficulty.of(Long.MAX_VALUE))); final KeyValueStorage kvStore = new InMemoryKeyValueStorage(); final KeyValueStorage kvStoreVariables = new InMemoryKeyValueStorage(); - final List blocks = gen.blockSequence(10); + final ArrayDeque blocks = new ArrayDeque<>(gen.blockSequence(10)); final List> blockReceipts = new ArrayList<>(blocks.size()); blockReceipts.add(Collections.emptyList()); // Write small chain to storage final MutableBlockchain mutableBlockchain = - createMutableBlockchain(kvStore, kvStoreVariables, blocks.get(0)); - for (int i = 1; i < blocks.size(); i++) { - final Block block = blocks.get(i); - final List receipts = gen.receipts(block); - blockReceipts.add(receipts); - mutableBlockchain.appendBlock(block, receipts); - } + createMutableBlockchain(kvStore, kvStoreVariables, blocks.getFirst()); + blocks.stream() + .filter(block -> !block.equals(blocks.getFirst())) + .forEach( + block -> { + final List receipts = gen.receipts(block); + blockReceipts.add(receipts); + mutableBlockchain.appendBlock(block, receipts); + }); + + // To make sure there are no surprising NPEs during DefaultBlockchain. + BlockchainStorage blockchainStorage = createStorage(kvStore, kvStoreVariables); + BlockchainStorage.Updater updater = blockchainStorage.updater(); + updater.setFinalized(blocks.getLast().getHash()); + updater.setSafeBlock(blocks.getLast().getHash()); + updater.setChainHead(blocks.getLast().getHash()); + updater.commit(); // Create read only chain final Blockchain blockchain = DefaultBlockchain.create( - createStorage(kvStore, kvStoreVariables), + blockchainStorage, MetricsSystemFactory.create(MetricsConfiguration.builder().enabled(true).build()), 0); - for (int i = 0; i < blocks.size(); i++) { - assertBlockDataIsStored(blockchain, blocks.get(i), blockReceipts.get(i)); - } - final Block lastBlock = blocks.get(blocks.size() - 1); - assertBlockIsHead(blockchain, lastBlock); - assertTotalDifficultiesAreConsistent(blockchain, lastBlock); + assertThat(blockReceipts.size()).isEqualTo(blocks.size()); + Iterator> it = blockReceipts.iterator(); + blocks.forEach(block -> assertBlockDataIsStored(blockchain, block, it.next())); + assertBlockIsHead(blockchain, blocks.getLast()); + assertTotalDifficultiesAreConsistent(blockchain, blocks.getLast()); } @Test