Change test to tease NPEs during DefaultBlockchain.<init>() caused by…

… initilization order (#7607) * Change test to tease NPEs during DefaultBlockchain.<init>() caused by initilization order * fixup! Change test to tease NPEs during DefaultBlockchain.<init>() caused by initilization order * move counters into their own method Signed-off-by: Luis Pinto <[email protected]>
hyperledger · Sep 12, 2024 · e0cd508 · e0cd508
1 parent 6ed1db3
commit e0cd508
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 27 deletions.
diff --git 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
@@ -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<Cache<Hash, List<TransactionReceipt>>> transactionReceiptsCache;
   private final Optional<Cache<Hash, Difficulty>> 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<Block> 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
@@ -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<Block> blocks = gen.blockSequence(10);
+    final ArrayDeque<Block> blocks = new ArrayDeque<>(gen.blockSequence(10));
     final List<List<TransactionReceipt>> 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<TransactionReceipt> 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<TransactionReceipt> receipts = gen.receipts(block);
+              blockReceipts.add(receipts);
+              mutableBlockchain.appendBlock(block, receipts);
+            });
+
+    // To make sure there are no surprising NPEs during DefaultBlockchain.<init>
+    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<List<TransactionReceipt>> it = blockReceipts.iterator();
+    blocks.forEach(block -> assertBlockDataIsStored(blockchain, block, it.next()));
+    assertBlockIsHead(blockchain, blocks.getLast());
+    assertTotalDifficultiesAreConsistent(blockchain, blocks.getLast());
   }
 
   @Test