hyperledger · shemnon · Feb 22, 2020 · Feb 22, 2020
diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/EthProtocol.java
@@ -64,6 +64,7 @@ public int messageSpace(final int protocolVersion) {
       case EthVersion.V62:
         return 8;
       case EthVersion.V63:
+      case EthVersion.V64:
         return 17;
       default:
         return 0;
@@ -76,6 +77,7 @@ public boolean isValidMessageCode(final int protocolVersion, final int code) {
       case EthVersion.V62:
         return eth62Messages.contains(code);
       case EthVersion.V63:
+      case EthVersion.V64:
         return eth63Messages.contains(code);
       default:
         return false;

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/EthProtocolManager.java
@@ -23,6 +23,7 @@
 import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.eth.EthProtocol;
 import org.hyperledger.besu.ethereum.eth.EthProtocolConfiguration;
+import org.hyperledger.besu.ethereum.eth.manager.ForkIdManager.ForkId;
 import org.hyperledger.besu.ethereum.eth.messages.EthPV62;
 import org.hyperledger.besu.ethereum.eth.messages.StatusMessage;
 import org.hyperledger.besu.ethereum.eth.peervalidation.PeerValidator;
@@ -41,21 +42,21 @@
 
 import java.math.BigInteger;
 import java.time.Clock;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
 public class EthProtocolManager implements ProtocolManager, MinedBlockObserver {
   private static final Logger LOG = LogManager.getLogger();
   private static final List<Capability> FAST_SYNC_CAPS =
-      Collections.singletonList(EthProtocol.ETH63);
+      List.of(EthProtocol.ETH63, EthProtocol.ETH64);
   private static final List<Capability> FULL_SYNC_CAPS =
-      Arrays.asList(EthProtocol.ETH62, EthProtocol.ETH63);
+      List.of(EthProtocol.ETH62, EthProtocol.ETH63, EthProtocol.ETH64);
 
   private final EthScheduler scheduler;
   private final CountDownLatch shutdown;
@@ -93,7 +94,7 @@ public EthProtocolManager(
     this.shutdown = new CountDownLatch(1);
     genesisHash = blockchain.getBlockHashByNumber(0L).get();
 
-    this.forkIdManager = ForkIdManager.buildCollection(genesisHash);
+    this.forkIdManager = forkIdManager;
 
     ethPeers = new EthPeers(getSupportedProtocol(), clock, metricsSystem);
     ethMessages = new EthMessages();
@@ -110,30 +111,7 @@ public EthProtocolManager(
     new EthServer(blockchain, worldStateArchive, ethMessages, ethereumWireProtocolConfiguration);
   }
 
-  public EthProtocolManager(
-      final Blockchain blockchain,
-      final WorldStateArchive worldStateArchive,
-      final BigInteger networkId,
-      final List<PeerValidator> peerValidators,
-      final boolean fastSyncEnabled,
-      final int syncWorkers,
-      final int txWorkers,
-      final int computationWorkers,
-      final Clock clock,
-      final MetricsSystem metricsSystem) {
-    this(
-        blockchain,
-        worldStateArchive,
-        networkId,
-        peerValidators,
-        fastSyncEnabled,
-        new EthScheduler(syncWorkers, txWorkers, computationWorkers, metricsSystem),
-        EthProtocolConfiguration.defaultConfig(),
-        clock,
-        metricsSystem,
-        ForkIdManager.buildCollection(blockchain.getBlockHashByNumber(0L).get()));
-  }
-
+  @VisibleForTesting
   public EthProtocolManager(
       final Blockchain blockchain,
       final WorldStateArchive worldStateArchive,
@@ -156,7 +134,7 @@ public EthProtocolManager(
         ethereumWireProtocolConfiguration,
         clock,
         metricsSystem,
-        ForkIdManager.buildCollection(blockchain.getBlockHashByNumber(0L).get()));
+        new ForkIdManager(blockchain, Collections.emptyList()));
   }
 
   public EthProtocolManager(
@@ -182,8 +160,7 @@ public EthProtocolManager(
         ethereumWireProtocolConfiguration,
         clock,
         metricsSystem,
-        ForkIdManager.buildCollection(
-            blockchain.getBlockHashByNumber(0L).get(), forks, blockchain));
+        new ForkIdManager(blockchain, forks));
   }
 
   public EthContext ethContext() {
@@ -274,6 +251,7 @@ public void handleNewConnection(final PeerConnection connection) {
     }
 
     final Capability cap = connection.capability(getSupportedProtocol());
+    final ForkId latestForkId = cap.getVersion() >= 64 ? forkIdManager.getLatestForkId() : null;
     // TODO: look to consolidate code below if possible
     // making status non-final and implementing it above would be one way.
     final StatusMessage status =
@@ -283,7 +261,7 @@ public void handleNewConnection(final PeerConnection connection) {
             blockchain.getChainHead().getTotalDifficulty(),
             blockchain.getChainHeadHash(),
             genesisHash,
-            forkIdManager.getLatestForkId());
+            latestForkId);
     try {
       LOG.debug("Sending status message to {}.", peer);
       peer.send(status);

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/manager/ForkIdManager.java
@@ -14,8 +14,6 @@
  */
 package org.hyperledger.besu.ethereum.eth.manager;
 
-import static java.util.Collections.emptyList;
-
 import org.hyperledger.besu.ethereum.chain.Blockchain;
 import org.hyperledger.besu.ethereum.core.Hash;
 import org.hyperledger.besu.ethereum.rlp.BytesValueRLPOutput;
@@ -27,52 +25,28 @@
 import java.util.stream.Collectors;
 import java.util.zip.CRC32;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.bytes.Bytes32;
 
 public class ForkIdManager {
 
+  private final Blockchain blockchain;
   private final Hash genesisHash;
-  private final Long currentHead;
-  private Long forkNext;
-  private final Long highestKnownFork = 0L;
-  private final List<ForkId> forkAndHashList;
-
-  public ForkIdManager(final Hash genesisHash, final List<Long> forks, final Long currentHead) {
-    this.genesisHash = genesisHash;
-    this.currentHead = currentHead;
-    this.forkAndHashList =
-        createForkIds(
-            // If there are two forks at the same block height, we only want to add it once to the
-            // crc checksum
-            forks.stream().distinct().collect(Collectors.toUnmodifiableList()));
-  };
-
-  static ForkIdManager buildCollection(
-      final Hash genesisHash, final List<Long> forks, final Blockchain blockchain) {
-    return new ForkIdManager(genesisHash, forks, blockchain.getChainHeadBlockNumber());
-  };
-
-  @VisibleForTesting
-  public static ForkIdManager buildCollection(final Hash genesisHash, final List<Long> forks) {
-    return new ForkIdManager(genesisHash, forks, Long.MAX_VALUE);
-  };
-
-  static ForkIdManager buildCollection(final Hash genesisHash) {
-    return new ForkIdManager(genesisHash, emptyList(), Long.MAX_VALUE);
+  private final List<Long> forks;
+  private long forkNext;
+  private final long highestKnownFork;
+  private List<ForkId> forkAndHashList;
+
+  public ForkIdManager(final Blockchain blockchain, final List<Long> forks) {
+    this.blockchain = blockchain;
+    this.genesisHash = blockchain.getGenesisBlock().getHash();
+    // de-dupe and sanitize forks
+    this.forks =
+        forks.stream().filter(fork -> fork > 0).distinct().collect(Collectors.toUnmodifiableList());
+    highestKnownFork = forks.size() > 0 ? forks.get(forks.size() - 1) : 0L;
+    createForkIds();
   };
 
-  // Non-generated entry (for tests)
-  public static ForkId createIdEntry(final String hash, final long next) {
-    return new ForkId(hash, next);
-  }
-
-  // Non-generated entry (for tests)
-  public static ForkId createIdEntry(final String hash, final String next) {
-    return new ForkId(hash, next);
-  }
-
   public List<ForkId> getForkAndHashList() {
     return this.forkAndHashList;
   }
@@ -98,11 +72,11 @@ public static ForkId readFrom(final RLPInput in) {
    * @param forkId to be validated.
    * @return boolean (peer valid (true) or invalid (false))
    */
-  public boolean peerCheck(final ForkId forkId) {
+  boolean peerCheck(final ForkId forkId) {
     if (forkId == null) {
       return true; // Another method must be used to validate (i.e. genesis hash)
     }
-    // Run the fork checksum validation ruleset:
+    // Run the fork checksum validation rule set:
     //   1. If local and remote FORK_CSUM matches, connect.
     //        The two nodes are in the same fork state currently. They might know
     //        of differing future forks, but that's not relevant until the fork
@@ -119,7 +93,7 @@ public boolean peerCheck(final ForkId forkId) {
     //        information.
     //   4. Reject in all other cases.
     if (isHashKnown(forkId.getHash())) {
-      if (currentHead < forkNext) {
+      if (blockchain.getChainHeadBlockNumber() < forkNext) {
         return true;
       } else {
         if (isForkKnown(forkId.getNext())) {
@@ -153,7 +127,7 @@ private boolean isForkKnown(final Long nextFork) {
   }
 
   private boolean isRemoteAwareOfPresent(final Bytes forkHash, final Long nextFork) {
-    for (ForkId j : forkAndHashList) {
+    for (final ForkId j : forkAndHashList) {
       if (forkHash.equals(j.getHash())) {
         if (nextFork.equals(j.getNext())) {
           return true;
@@ -167,9 +141,9 @@ private boolean isRemoteAwareOfPresent(final Bytes forkHash, final Long nextFork
     return false;
   }
 
-  private List<ForkId> createForkIds(final List<Long> forks) {
+  private void createForkIds() {
     final CRC32 crc = new CRC32();
-    crc.update(this.genesisHash.toArray());
+    crc.update(genesisHash.toArray());
     final List<Bytes> forkHashes = new ArrayList<>(List.of(getCurrentCrcHash(crc)));
     for (final Long fork : forks) {
       updateCrc(crc, fork);
@@ -181,18 +155,14 @@ private List<ForkId> createForkIds(final List<Long> forks) {
       forkIds.add(new ForkId(forkHashes.get(i), forks.get(i)));
     }
     if (!forks.isEmpty()) {
-      this.forkNext = forkIds.get(forkIds.size() - 1).getNext();
-      forkIds.add(
-          new ForkId(
-              forkHashes.get(forkHashes.size() - 1),
-              currentHead > forkNext ? 0 : forkNext // Use 0 if there are no known next forks
-              ));
+      forkNext = forkIds.get(forkIds.size() - 1).getNext();
+      forkIds.add(new ForkId(forkHashes.get(forkHashes.size() - 1), 0));
     }
-    return forkIds;
+    this.forkAndHashList = forkIds;
   }
 
   private void updateCrc(final CRC32 crc, final Long block) {
-    byte[] byteRepresentationFork = longToBigEndian(block);
+    final byte[] byteRepresentationFork = longToBigEndian(block);
     crc.update(byteRepresentationFork, 0, byteRepresentationFork.length);
   }
 
@@ -205,35 +175,14 @@ public static class ForkId {
     final Bytes next;
     Bytes forkIdRLP;
 
-    ForkId(final Bytes hash, final Bytes next) {
+    private ForkId(final Bytes hash, final Bytes next) {
       this.hash = hash;
       this.next = next;
       createForkIdRLP();
     }
 
-    ForkId(final String hash, final String next) {
-      this.hash = padToEightBytes(Bytes.fromHexString((hash.length() % 2 == 0 ? "" : "0") + hash));
-      if (next.equals("") || next.equals("0x")) {
-        this.next = Bytes.EMPTY;
-      } else if (next.startsWith("0x")) {
-        long asLong = Long.parseLong(next.replaceFirst("0x", ""), 16);
-        this.next = Bytes.wrap(longToBigEndian(asLong)).trimLeadingZeros();
-      } else {
-        this.next = Bytes.wrap(longToBigEndian(Long.parseLong(next)));
-      }
-      createForkIdRLP();
-    }
-
-    ForkId(final String hash, final long next) {
-      this.hash = Bytes.fromHexString(hash);
-      this.next = Bytes.wrap(longToBigEndian(next));
-      createForkIdRLP();
-    }
-
-    ForkId(final Bytes hash, final long next) {
-      this.hash = hash;
-      this.next = Bytes.wrap(longToBigEndian(next));
-      createForkIdRLP();
+    public ForkId(final Bytes hash, final long next) {
+      this(hash, Bytes.wrap(longToBigEndian(next)).trimLeadingZeros());
     }
 
     public long getNext() {
@@ -245,7 +194,7 @@ public Bytes getHash() {
     }
 
     void createForkIdRLP() {
-      BytesValueRLPOutput out = new BytesValueRLPOutput();
+      final BytesValueRLPOutput out = new BytesValueRLPOutput();
       writeTo(out);
       forkIdRLP = out.encoded();
     }
@@ -260,26 +209,17 @@ public void writeTo(final RLPOutput out) {
     public static ForkId readFrom(final RLPInput in) {
       in.enterList();
       final Bytes hash = in.readBytes();
-      final long next = in.readLong();
+      final long next = in.readLongScalar();
       in.leaveList();
       return new ForkId(hash, next);
     }
 
     public List<ForkId> asList() {
-      ArrayList<ForkId> forRLP = new ArrayList<>();
+      final ArrayList<ForkId> forRLP = new ArrayList<>();
       forRLP.add(this);
       return forRLP;
     }
 
-    private static Bytes padToEightBytes(final Bytes hash) {
-      if (hash.size() < 4) {
-        Bytes padded = Bytes.concatenate(hash, Bytes.fromHexString("0x00"));
-        return padToEightBytes(padded);
-      } else {
-        return hash;
-      }
-    }
-
     @Override
     public String toString() {
       return "ForkId(hash=" + this.hash + ", next=" + next.toLong() + ")";
@@ -288,8 +228,8 @@ public String toString() {
     @Override
     public boolean equals(final Object obj) {
       if (obj instanceof ForkId) {
-        ForkId other = (ForkId) obj;
-        long thisNext = next.toLong();
+        final ForkId other = (ForkId) obj;
+        final long thisNext = next.toLong();
         return other.getHash().equals(this.hash) && thisNext == other.getNext();
       }
       return false;
@@ -304,7 +244,7 @@ public int hashCode() {
   // next two methods adopted from:
   // https://github.com/bcgit/bc-java/blob/master/core/src/main/java/org/bouncycastle/util/Pack.java
   private static byte[] longToBigEndian(final long n) {
-    byte[] bs = new byte[8];
+    final byte[] bs = new byte[8];
     intToBigEndian((int) (n >>> 32), bs, 0);
     intToBigEndian((int) (n & 0xffffffffL), bs, 4);
     return bs;

diff --git a/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessage.java b/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/messages/StatusMessage.java
@@ -182,15 +182,16 @@ public static EthStatus readFrom(final RLPInput in) {
       final Difficulty totalDifficulty = Difficulty.of(in.readUInt256Scalar());
       final Hash bestHash = Hash.wrap(in.readBytes32());
       final Hash genesisHash = Hash.wrap(in.readBytes32());
+      final ForkIdManager.ForkId forkId;
       if (in.nextIsList()) {
-        final ForkIdManager.ForkId forkId = ForkIdManager.ForkId.readFrom(in);
-        in.leaveList();
-        return new EthStatus(
-            protocolVersion, networkId, totalDifficulty, bestHash, genesisHash, forkId);
+        forkId = ForkIdManager.ForkId.readFrom(in);
+      } else {
+        forkId = null;
       }
       in.leaveList();
 
-      return new EthStatus(protocolVersion, networkId, totalDifficulty, bestHash, genesisHash);
+      return new EthStatus(
+          protocolVersion, networkId, totalDifficulty, bestHash, genesisHash, forkId);
     }
   }
 }
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
@@ -1094,7 +1094,7 @@ public void transactionMessagesGoToTheCorrectExecutor() {
             EthProtocolConfiguration.defaultConfig(),
             TestClock.fixed(),
             metricsSystem,
-            ForkIdManager.buildCollection(blockchain.getBlockHashByNumber(0L).get()))) {
+            new ForkIdManager(blockchain, Collections.emptyList()))) {
 
       // Create a transaction pool.  This has a side effect of registering a listener for the
       // transactions message.