Fix for incorrect eip868 code (hyperledger#1858)

* Fix for incorrect eip868 code Signed-off-by: David Mechler <[email protected]> * PR updates Signed-off-by: David Mechler <[email protected]>
shemnon · Feb 1, 2021 · a052148 · a052148
1 parent a90160d
commit a052148
Show file tree

Hide file tree

Showing 10 changed files with 210 additions and 67 deletions.
diff --git a/...eum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java b/...eum/p2p/src/main/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgent.java
@@ -19,6 +19,7 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.tuweni.bytes.Bytes.wrapBuffer;
 
+import org.hyperledger.besu.crypto.Hash;
 import org.hyperledger.besu.crypto.NodeKey;
 import org.hyperledger.besu.ethereum.p2p.config.DiscoveryConfiguration;
 import org.hyperledger.besu.ethereum.p2p.discovery.internal.Packet;
@@ -57,9 +58,9 @@
 import org.apache.tuweni.units.bigints.UInt64;
 import org.ethereum.beacon.discovery.schema.EnrField;
 import org.ethereum.beacon.discovery.schema.IdentitySchema;
-import org.ethereum.beacon.discovery.schema.IdentitySchemaInterpreter;
 import org.ethereum.beacon.discovery.schema.NodeRecord;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
+import org.ethereum.beacon.discovery.util.Functions;
 
 /**
  * The peer discovery agent is the network component that sends and receives peer discovery messages
@@ -179,16 +180,16 @@ private Optional<NodeRecord> addLocalNodeRecord(
       final Integer udpPort) {
     final KeyValueStorage keyValueStorage =
         storageProvider.getStorageBySegmentIdentifier(KeyValueSegmentIdentifier.BLOCKCHAIN);
-    final NodeRecordFactory nodeRecordFactory = new NodeRecordFactory(IdentitySchemaInterpreter.V4);
+    final NodeRecordFactory nodeRecordFactory = NodeRecordFactory.DEFAULT;
     final Optional<NodeRecord> existingNodeRecord =
         keyValueStorage
             .get(Bytes.of(SEQ_NO_STORE_KEY.getBytes(UTF_8)).toArray())
             .map(Bytes::of)
             .flatMap(b -> Optional.of(nodeRecordFactory.fromBytes(b)));
 
-    final Bytes addressBytes = Bytes.of(advertisedAddress.getBytes(UTF_8));
+    final Bytes addressBytes = Bytes.of(InetAddresses.forString(advertisedAddress).getAddress());
     if (existingNodeRecord.isEmpty()
-        || !existingNodeRecord.get().getNodeId().equals(nodeId)
+        || !existingNodeRecord.get().get(EnrField.PKEY_SECP256K1).equals(nodeId)
         || !addressBytes.equals(existingNodeRecord.get().get(EnrField.IP_V4))
         || !tcpPort.equals(existingNodeRecord.get().get(EnrField.TCP))
         || !udpPort.equals(existingNodeRecord.get().get(EnrField.UDP))) {
@@ -198,10 +199,15 @@ private Optional<NodeRecord> addLocalNodeRecord(
           nodeRecordFactory.createFromValues(
               sequenceNumber,
               new EnrField(EnrField.ID, IdentitySchema.V4),
-              new EnrField(EnrField.PKEY_SECP256K1, nodeId),
+              new EnrField(EnrField.PKEY_SECP256K1, Functions.compressPublicKey(nodeId)),
               new EnrField(EnrField.IP_V4, addressBytes),
               new EnrField(EnrField.TCP, tcpPort),
               new EnrField(EnrField.UDP, udpPort));
+      nodeRecord.setSignature(
+          nodeKey
+              .sign(Hash.keccak256(nodeRecord.serializeNoSignature()))
+              .encodedBytes()
+              .slice(0, 64));
 
       final KeyValueStorageTransaction keyValueStorageTransaction =
           keyValueStorage.startTransaction();

diff --git a/.../main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/ENRRequestPacketData.java b/.../main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/ENRRequestPacketData.java
@@ -20,10 +20,6 @@
 import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 
 public class ENRRequestPacketData implements PacketData {
-
-  /* Fixed value that represents we're using v5 of the P2P discovery protocol. */
-  private static final int VERSION = 5;
-
   /* In seconds after epoch. */
   private final long expiration;
 
@@ -43,8 +39,6 @@ static ENRRequestPacketData create(final long expirationSec) {
 
   public static ENRRequestPacketData readFrom(final RLPInput in) {
     in.enterList();
-    // The first element signifies the "version", but this value is ignored as of EIP-8
-    in.readBigIntegerScalar();
     final long expiration = in.readLongScalar();
     in.leaveListLenient();
     return new ENRRequestPacketData(expiration);
@@ -53,7 +47,6 @@ public static ENRRequestPacketData readFrom(final RLPInput in) {
   @Override
   public void writeTo(final RLPOutput out) {
     out.startList();
-    out.writeIntScalar(VERSION);
     out.writeLongScalar(expiration);
     out.endList();
   }

diff --git a/...main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/ENRResponsePacketData.java b/...main/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/ENRResponsePacketData.java
@@ -20,7 +20,6 @@
 import org.hyperledger.besu.ethereum.rlp.RLPOutput;
 
 import org.apache.tuweni.bytes.Bytes;
-import org.ethereum.beacon.discovery.schema.IdentitySchemaInterpreter;
 import org.ethereum.beacon.discovery.schema.NodeRecord;
 import org.ethereum.beacon.discovery.schema.NodeRecordFactory;
 
@@ -39,24 +38,24 @@ private ENRResponsePacketData(final Bytes requestHash, final NodeRecord enr) {
     this.enr = enr;
   }
 
-  static ENRResponsePacketData create(final Bytes requestHash, final NodeRecord enr) {
+  public static ENRResponsePacketData create(final Bytes requestHash, final NodeRecord enr) {
     return new ENRResponsePacketData(requestHash, enr);
   }
 
   public static ENRResponsePacketData readFrom(final RLPInput in) {
     in.enterList();
     final Bytes requestHash = in.readBytes();
-    final NodeRecord enr =
-        new NodeRecordFactory(IdentitySchemaInterpreter.V4).fromBytes(in.readBytes());
     in.leaveListLenient();
+    final NodeRecord enr = NodeRecordFactory.DEFAULT.fromBytes(in.currentListAsBytes());
+
     return new ENRResponsePacketData(requestHash, enr);
   }
 
   @Override
   public void writeTo(final RLPOutput out) {
     out.startList();
     out.writeBytes(requestHash);
-    out.writeBytes(enr.serialize());
+    out.writeRLPBytes(enr.serialize());
     out.endList();
   }
 

diff --git a/...in/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java b/...in/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/PeerDiscoveryController.java
@@ -337,7 +337,7 @@ public void onMessage(final Packet packet, final DiscoveryPeer sender) {
         break;
       case ENR_REQUEST:
         if (PeerDiscoveryStatus.BONDED.equals(peer.getStatus())) {
-          LOG.trace("ENR_REQUEST recevied from bonded peer Id: " + peer.getId());
+          LOG.trace("ENR_REQUEST recevied from bonded peer Id: {}", peer.getId());
           packet
               .getPacketData(ENRRequestPacketData.class)
               .ifPresent(p -> respondToENRRequest(p, packet.getHash(), peer));

diff --git a/...p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java b/...p2p/src/test/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryAgentTest.java
@@ -24,6 +24,8 @@
 
 import org.hyperledger.besu.crypto.NodeKey;
 import org.hyperledger.besu.crypto.NodeKeyUtils;
+import org.hyperledger.besu.crypto.SECP256K1.KeyPair;
+import org.hyperledger.besu.crypto.SECP256K1.PrivateKey;
 import org.hyperledger.besu.ethereum.p2p.discovery.PeerDiscoveryTestHelper.AgentBuilder;
 import org.hyperledger.besu.ethereum.p2p.discovery.internal.FindNeighborsPacketData;
 import org.hyperledger.besu.ethereum.p2p.discovery.internal.MockPeerDiscoveryAgent;
@@ -43,6 +45,8 @@
 import java.util.Optional;
 import java.util.stream.Collectors;
 
+import org.apache.tuweni.bytes.Bytes32;
+import org.ethereum.beacon.discovery.schema.NodeRecord;
 import org.junit.Test;
 
 public class PeerDiscoveryAgentTest {
@@ -69,14 +73,30 @@ public void createAgentWithInvalidBootnodes() {
 
   @Test
   public void testNodeRecordCreated() {
-    final MockPeerDiscoveryAgent agent = helper.startDiscoveryAgent(Collections.emptyList());
+    KeyPair keyPair =
+        KeyPair.create(
+            PrivateKey.create(
+                Bytes32.fromHexString(
+                    "0xb71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")));
+    final MockPeerDiscoveryAgent agent =
+        helper.startDiscoveryAgent(
+            helper
+                .agentBuilder()
+                .nodeKey(NodeKeyUtils.createFrom(keyPair))
+                .advertisedHost("127.0.0.1")
+                .bindPort(30303));
     assertThat(agent.getAdvertisedPeer().isPresent()).isTrue();
     assertThat(agent.getAdvertisedPeer().get().getNodeRecord().isPresent()).isTrue();
-    assertThat(agent.getAdvertisedPeer().get().getNodeRecord().get().getNodeId()).isNotNull();
-    assertThat(agent.getAdvertisedPeer().get().getNodeRecord().get().getIdentityScheme())
-        .isNotNull();
-    assertThat(agent.getAdvertisedPeer().get().getNodeRecord().get().getSignature()).isNotNull();
-    assertThat(agent.getAdvertisedPeer().get().getNodeRecord().get().getSeq()).isNotNull();
+    final NodeRecord nodeRecord = agent.getAdvertisedPeer().get().getNodeRecord().get();
+    assertThat(nodeRecord.getNodeId()).isNotNull();
+    assertThat(nodeRecord.getIdentityScheme()).isNotNull();
+    assertThat(nodeRecord.getSignature()).isNotNull();
+    assertThat(nodeRecord.getSeq()).isNotNull();
+    assertThat(nodeRecord.asEnr())
+        .isEqualTo(
+            "enr:-Im4QIEcZbEzW8DSEX-0BPB36s1UwTT54D_I-mvrSHqsZpVzGg7wlXyHb6vRq3GTGNBNQyoUkKkJGryrTo"
+                + "DTersRuNYBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA"
+                + "8yWM0xOIN0Y3ACg3VkcIJ2Xw");
   }
 
   @Test

diff --git a/...st/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryPacketPcapSedesTest.java b/...st/java/org/hyperledger/besu/ethereum/p2p/discovery/PeerDiscoveryPacketPcapSedesTest.java
@@ -58,17 +58,14 @@ public class PeerDiscoveryPacketPcapSedesTest {
           + "de4223b36e213e03197fecc1250f34c52e1e1ec8cdff5b9cbe005f95567daa9fd96a64c0e3e3a8d55157bf9d87f1c4666cdae79b37bfa5"
           + "c1835353475845fda165c";
   private static final String enrResquestHexData =
-      "08218a2075159e8e48d5bc0561b3c9b107c4459454b82b2a0df818222178a7128e4"
-          + "3e951a7ee1669b5486458919bb6c5f9e910a400222eeabe12191d234b5ce94b5614cbf19c23ed4de001ffcdd009cbb38fa0ac475e272ae"
-          + "593ecc5e4cff2e90105c605845fda17cc";
+      "fed7cfd0a60b51d027d14d5bd1d4c5bc4ea289940c0d38f2ff8e72522e33e39be040ef1acbe25e2c40523821c0c536e17e0f7204a08260b842dc"
+          + "a830513a2f9e5169a3f711ecb5a512fdd56e5edfd7d8fdaa0e6982020dbd2f76949ef84d1a840005c58460146486";
   private static final String enrResponseHexData =
-      "f354dd86f7651dad9a4ab3845c985a0a603a049b8b9dd563d26ad4ecbd6d9d066c0"
-          + "f448523df89a396d2ee1bf3758d34d750ae4e46956824a11b9fcb39a757551f70be6f7a6692638c16cf903e241e6c777125ce377dd7bd0"
-          + "7139d4845a2d8f50106f8e7a008218a2075159e8e48d5bc0561b3c9b107c4459454b82b2a0df818222178a712b8c4f8c2b860000000000"
-          + "00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
-          + "000000000000000000000000000000000000000000000000000000000000000000000000003826964827634826970893132372e302e302"
-          + "e3189736563703235366b31b840fbe12329d5d99e3d46cba2d1f9d8d397a4f2955253396f6e0459f3f14bb29c0e4f37d8bac890ff9bfb4"
-          + "12879257ba2378a0b48bed6b81647c6972d323212d051";
+      "9c85a6d16e5222dc48df51654cc36fb372b5429646ca8fd85a7f79ea420dba326f2cc84456b6c7fa1e6dd4ed6e3e89934e6f4415d58b40899996"
+          + "ee461a8e147f62ff33177680cffe061d048183091b4254dd1edf05f7e92d1117b23035d94f7d0106f8a7a0fed7cfd0a60b51d027d14d5b"
+          + "d1d4c5bc4ea289940c0d38f2ff8e72522e33e39bf884b8407098ad865b00a582051940cb9cf36836572411a47278783077011599ed5cd1"
+          + "6b76f2635f4e234738f30813a89eb9137e3e3df5266e3a1f11df72ecf1145ccb9c01826964827634826970847f00000189736563703235"
+          + "366b31a103ca634cae0d49acb401d8a4c6b6fe8c55b70d115bf400769cc1400f3258cd31388375647082765f";
 
   @Test
   public void testUDPPingSerializeDeserialize() {

diff --git a/...t/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/ENRRequestPacketDataTest.java b/...t/java/org/hyperledger/besu/ethereum/p2p/discovery/internal/ENRRequestPacketDataTest.java
@@ -37,12 +37,10 @@ public void serializeDeserialize() {
 
   @Test
   public void readFrom() {
-    final int version = 4;
     final long time = System.currentTimeMillis();
 
     final BytesValueRLPOutput out = new BytesValueRLPOutput();
     out.startList();
-    out.writeIntScalar(version);
     out.writeLongScalar(time);
     out.endList();
 
@@ -54,12 +52,10 @@ public void readFrom() {
 
   @Test
   public void readFrom_withExtraFields() {
-    final int version = 4;
     final long time = System.currentTimeMillis();
 
     final BytesValueRLPOutput out = new BytesValueRLPOutput();
     out.startList();
-    out.writeIntScalar(version);
     out.writeLongScalar(time);
     // Add extra field
     out.writeLongScalar(11);
@@ -70,21 +66,4 @@ public void readFrom_withExtraFields() {
 
     assertThat(deserialized.getExpiration()).isEqualTo(time);
   }
-
-  @Test
-  public void readFrom_unknownVersion() {
-    final int version = 99;
-    final long time = System.currentTimeMillis();
-
-    final BytesValueRLPOutput out = new BytesValueRLPOutput();
-    out.startList();
-    out.writeIntScalar(version);
-    out.writeLongScalar(time);
-    out.endList();
-
-    final Bytes serialized = out.encoded();
-    final ENRRequestPacketData deserialized = ENRRequestPacketData.readFrom(RLP.input(serialized));
-
-    assertThat(deserialized.getExpiration()).isEqualTo(time);
-  }
 }