Fix NPE for legacy ForkId with no forks (hyperledger#7349)

Fix NPE for legacy ForkId with no forks Signed-off-by: stefan.pingel@consensys.net <stefan.pingel@consensys.net>
fab-10 · Jul 22, 2024 · bcbfec0 · bcbfec0
1 parent 0269d83
commit bcbfec0
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 17 deletions.
diff --git a/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java b/ethereum/core/src/main/java/org/hyperledger/besu/ethereum/forkid/ForkIdManager.java
@@ -79,8 +79,7 @@ public ForkIdManager(
             .sorted()
             .collect(Collectors.toUnmodifiableList());
     final List<Long> allForkNumbers =
-        Stream.concat(blockNumberForks.stream(), timestampForks.stream())
-            .collect(Collectors.toList());
+        Stream.concat(blockNumberForks.stream(), timestampForks.stream()).toList();
     this.forkNext = createForkIds();
     this.allForkIds =
         Stream.concat(blockNumbersForkIds.stream(), timestampsForkIds.stream())
@@ -93,7 +92,7 @@ public ForkIdManager(
   public ForkId getForkIdForChainHead() {
     if (legacyEth64) {
       return blockNumbersForkIds.isEmpty()
-          ? null
+          ? new ForkId(genesisHashCrc, 0)
           : blockNumbersForkIds.get(blockNumbersForkIds.size() - 1);
     }
     final BlockHeader header = chainHeadSupplier.get();

diff --git a/...e/src/test/java/org/hyperledger/besu/ethereum/forkid/ForkIdBackwardCompatibilityTest.java b/...e/src/test/java/org/hyperledger/besu/ethereum/forkid/ForkIdBackwardCompatibilityTest.java
@@ -18,6 +18,8 @@
 import static org.hyperledger.besu.ethereum.forkid.ForkIdTestUtil.GenesisHash;
 import static org.hyperledger.besu.ethereum.forkid.ForkIdTestUtil.mockBlockchain;
 
+import org.hyperledger.besu.ethereum.chain.Blockchain;
+
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
@@ -77,6 +79,13 @@ public static Stream<Arguments> data() {
             8L,
             Arrays.asList(0L, 0L, 4L, 5L, 6L),
             true,
+            null),
+        Arguments.of(
+            "no forks and legacyEth64=true",
+            GenesisHash.PRIVATE,
+            8L,
+            Collections.emptyList(),
+            true,
             null));
   }
 
@@ -90,13 +99,11 @@ public void assertBackwardCompatibilityWorks(
       final boolean legacyEth64,
       final ForkId wantForkId) {
     LOG.info("Running test case {}", name);
+    final Blockchain blockchain = mockBlockchain(genesisHash, head, 0);
     final ForkIdManager forkIdManager =
-        new ForkIdManager(
-            mockBlockchain(genesisHash, head, 0), forks, Collections.emptyList(), legacyEth64);
+        new ForkIdManager(blockchain, forks, Collections.emptyList(), legacyEth64);
     final ForkId legacyForkId =
-        legacyEth64
-            ? new LegacyForkIdManager(mockBlockchain(genesisHash, head, 0), forks).getLatestForkId()
-            : null;
+        legacyEth64 ? new LegacyForkIdManager(blockchain, forks).getLatestForkId() : null;
     assertThat(forkIdManager.getForkIdForChainHead())
         .isEqualTo(legacyEth64 ? legacyForkId : wantForkId);
   }

diff --git a/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/forkid/LegacyForkIdManager.java b/ethereum/core/src/test/java/org/hyperledger/besu/ethereum/forkid/LegacyForkIdManager.java
@@ -20,7 +20,6 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.stream.Collectors;
 import java.util.zip.CRC32;
 
 import org.apache.tuweni.bytes.Bytes;
@@ -30,12 +29,13 @@ public class LegacyForkIdManager {
   private final Hash genesisHash;
   private final List<Long> forks;
   private List<ForkId> forkAndHashList;
+  private CRC32 crc;
+  private Bytes genesisHashCrc;
 
   public LegacyForkIdManager(final Blockchain blockchain, final List<Long> forks) {
     this.genesisHash = blockchain.getGenesisBlock().getHash();
     // de-dupe and sanitize forks
-    this.forks =
-        forks.stream().filter(fork -> fork > 0).distinct().collect(Collectors.toUnmodifiableList());
+    this.forks = forks.stream().filter(fork -> fork > 0).distinct().toList();
     createForkIds();
   }
 
@@ -44,10 +44,10 @@ public List<ForkId> getForkAndHashList() {
   }
 
   public ForkId getLatestForkId() {
-    if (forkAndHashList.size() > 0) {
-      return forkAndHashList.get(forkAndHashList.size() - 1);
+    if (!forkAndHashList.isEmpty()) {
+      return forkAndHashList.getLast();
     }
-    return null;
+    return new ForkId(genesisHashCrc, 0);
   }
 
   public static ForkId readFrom(final RLPInput in) {
@@ -59,8 +59,9 @@ public static ForkId readFrom(final RLPInput in) {
   }
 
   private void createForkIds() {
-    final CRC32 crc = new CRC32();
+    crc = new CRC32();
     crc.update(genesisHash.toArray());
+    genesisHashCrc = getCurrentCrcHash(crc);
     final List<Bytes> forkHashes = new ArrayList<>(List.of(getCurrentCrcHash(crc)));
     for (final Long fork : forks) {
       updateCrc(crc, fork);

diff --git a/...m/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java b/...m/eth/src/test/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManagerTest.java
@@ -64,6 +64,7 @@
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPool;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolConfiguration;
 import org.hyperledger.besu.ethereum.eth.transactions.TransactionPoolFactory;
+import org.hyperledger.besu.ethereum.forkid.ForkId;
 import org.hyperledger.besu.ethereum.forkid.ForkIdManager;
 import org.hyperledger.besu.ethereum.mainnet.ProtocolSchedule;
 import org.hyperledger.besu.ethereum.p2p.rlpx.connections.PeerConnection;
@@ -93,6 +94,7 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
+import java.util.zip.CRC32;
 
 import com.google.common.collect.Lists;
 import org.apache.tuweni.bytes.Bytes;
@@ -1136,7 +1138,7 @@ public void transactionMessagesGoToTheCorrectExecutor() {
   }
 
   @Test
-  public void forkIdForChainHeadMayBeNull() {
+  public void forkIdForChainHeadLegacyNoForksNotEmpty() {
     final EthScheduler ethScheduler = mock(EthScheduler.class);
     try (final EthProtocolManager ethManager =
         EthProtocolManagerTestUtil.create(
@@ -1149,7 +1151,13 @@ public void forkIdForChainHeadMayBeNull() {
             new ForkIdManager(
                 blockchain, Collections.emptyList(), Collections.emptyList(), true))) {
 
-      assertThat(ethManager.getForkIdAsBytesList()).isEmpty();
+      assertThat(ethManager.getForkIdAsBytesList()).isNotEmpty();
+      final CRC32 genesisHashCRC = new CRC32();
+      genesisHashCRC.update(blockchain.getGenesisBlock().getHash().toArray());
+      assertThat(ethManager.getForkIdAsBytesList())
+          .isEqualTo(
+              new ForkId(Bytes.ofUnsignedInt(genesisHashCRC.getValue()), 0L)
+                  .getForkIdAsBytesList());
     }
   }