Refactoring, discovery updated to 24.9.1

Author: zilm13

data/beaconrestapi/src/main/java/tech/pegasys/teku/beaconrestapi/handlers/v1/node/GetPeerById.java

@@ -64,7 +64,7 @@ public GetPeerById(final DataProvider provider) {
   @Override
   public void handleRequest(final RestApiRequest request) throws JsonProcessingException {
     request.header(Header.CACHE_CONTROL, CACHE_NONE);
-    Optional<Eth2PeerWithEnr> peer =
+    final Optional<Eth2PeerWithEnr> peer =
         network.getEth2PeerById(request.getPathParameter(PEER_ID_PARAMETER));
     if (peer.isEmpty()) {
       request.respondError(SC_NOT_FOUND, "Peer not found");

data/provider/src/main/java/tech/pegasys/teku/api/NetworkDataProvider.java

@@ -32,18 +32,21 @@
 import tech.pegasys.teku.spec.datastructures.networking.libp2p.rpc.metadata.MetadataMessage;
 
 public class NetworkDataProvider {
-  final Function<UInt256, Optional<String>> enrLookupFunction;
+  final Function<Optional<UInt256>, Optional<String>> enrLookupFunction;
 
   private final Eth2P2PNetwork network;
 
   public NetworkDataProvider(final Eth2P2PNetwork network) {
     this.network = network;
     this.enrLookupFunction =
-        nodeId -> {
+        maybeNodeId -> {
+          if (maybeNodeId.isEmpty()) {
+            return Optional.empty();
+          }
           final Optional<DiscoveryService> maybeDiscoveryService =
               network.getDiscoveryNetwork().map(DiscoveryNetwork::getDiscoveryService);
           return maybeDiscoveryService.flatMap(
-              discoveryService -> discoveryService.lookupEnr(nodeId));
+              discoveryService -> discoveryService.lookupEnr(maybeNodeId.get()));
         };
   }
 

gradle/versions.gradle

@@ -159,7 +159,7 @@ dependencyManagement {
 
     // discovery includes tuweni libraries under a different name so version resolution doesn't work
     // exclude them here and leave them to be included on the classpath by the version we use
-    dependency('tech.pegasys.discovery:discovery:24.9.0') {
+    dependency('tech.pegasys.discovery:discovery:24.9.1') {
       exclude 'org.apache.tuweni:bytes'
       exclude 'org.apache.tuweni:crypto'
       exclude 'org.apache.tuweni:units'

networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/Eth2P2PNetworkBuilder.java

@@ -17,15 +17,13 @@
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
 
-import io.libp2p.core.crypto.PubKey;
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import org.apache.tuweni.bytes.Bytes;
-import org.apache.tuweni.units.bigints.UInt256;
 import org.hyperledger.besu.plugin.services.MetricsSystem;
 import tech.pegasys.teku.infrastructure.async.AsyncRunner;
 import tech.pegasys.teku.infrastructure.events.EventChannels;
@@ -51,17 +49,16 @@
 import tech.pegasys.teku.networking.eth2.peers.DiscoveryNodeIdExtractor;
 import tech.pegasys.teku.networking.eth2.peers.Eth2PeerManager;
 import tech.pegasys.teku.networking.eth2.peers.Eth2PeerSelectionStrategy;
+import tech.pegasys.teku.networking.eth2.peers.LibP2PDiscoveryNodeIdExtractor;
 import tech.pegasys.teku.networking.eth2.rpc.beaconchain.methods.StatusMessageFactory;
 import tech.pegasys.teku.networking.eth2.rpc.core.encodings.RpcEncoding;
 import tech.pegasys.teku.networking.p2p.connection.PeerPools;
 import tech.pegasys.teku.networking.p2p.connection.TargetPeerRange;
 import tech.pegasys.teku.networking.p2p.discovery.DiscoveryConfig;
 import tech.pegasys.teku.networking.p2p.discovery.DiscoveryNetwork;
 import tech.pegasys.teku.networking.p2p.discovery.DiscoveryNetworkBuilder;
-import tech.pegasys.teku.networking.p2p.discovery.discv5.DiscV5Service;
 import tech.pegasys.teku.networking.p2p.gossip.PreparedGossipMessageFactory;
 import tech.pegasys.teku.networking.p2p.libp2p.LibP2PNetworkBuilder;
-import tech.pegasys.teku.networking.p2p.libp2p.LibP2PPeer;
 import tech.pegasys.teku.networking.p2p.libp2p.LibP2PPrivateKeyLoader;
 import tech.pegasys.teku.networking.p2p.libp2p.gossip.GossipTopicFilter;
 import tech.pegasys.teku.networking.p2p.network.P2PNetwork;
@@ -145,17 +142,7 @@ public Eth2P2PNetwork build() {
     final SubnetSubscriptionService attestationSubnetService = new SubnetSubscriptionService();
     final SubnetSubscriptionService syncCommitteeSubnetService = new SubnetSubscriptionService();
 
-    // TODO a bit hacky solution, subject to be refactored
-    DiscoveryNodeIdExtractor discoveryNodeIdExtractor =
-        peer -> {
-          LibP2PPeer libP2PPeer = (LibP2PPeer) peer;
-          PubKey libP2PPubKey = libP2PPeer.getPubKey();
-          Bytes discoveryNodeIdBytes =
-              DiscV5Service.DEFAULT_NODE_RECORD_CONVERTER.convertPublicKeyToNodeId(
-                  Bytes.wrap(libP2PPubKey.raw()));
-          return UInt256.fromBytes(discoveryNodeIdBytes);
-        };
-
+    final DiscoveryNodeIdExtractor discoveryNodeIdExtractor = new LibP2PDiscoveryNodeIdExtractor();
     final RpcEncoding rpcEncoding =
         RpcEncoding.createSszSnappyEncoding(spec.getNetworkingConfig().getMaxChunkSize());
     if (statusMessageFactory == null) {

networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/peers/DefaultEth2Peer.java

@@ -73,7 +73,7 @@ class DefaultEth2Peer extends DelegatingPeer implements Eth2Peer {
   private static final Logger LOG = LogManager.getLogger();
 
   private final Spec spec;
-  private final UInt256 discoveryNodeId;
+  private final Optional<UInt256> discoveryNodeId;
   private final BeaconChainMethods rpcMethods;
   private final StatusMessageFactory statusMessageFactory;
   private final MetadataMessagesFactory metadataMessagesFactory;
@@ -97,7 +97,7 @@ class DefaultEth2Peer extends DelegatingPeer implements Eth2Peer {
   DefaultEth2Peer(
       final Spec spec,
       final Peer peer,
-      final UInt256 discoveryNodeId,
+      final Optional<UInt256> discoveryNodeId,
       final BeaconChainMethods rpcMethods,
       final StatusMessageFactory statusMessageFactory,
       final MetadataMessagesFactory metadataMessagesFactory,
@@ -133,7 +133,7 @@ class DefaultEth2Peer extends DelegatingPeer implements Eth2Peer {
   }
 
   @Override
-  public UInt256 getDiscoveryNodeId() {
+  public Optional<UInt256> getDiscoveryNodeId() {
     return discoveryNodeId;
   }
 

networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/peers/DiscoveryNodeIdExtractor.java

@@ -13,9 +13,10 @@
 
 package tech.pegasys.teku.networking.eth2.peers;
 
+import java.util.Optional;
 import org.apache.tuweni.units.bigints.UInt256;
 import tech.pegasys.teku.networking.p2p.peer.Peer;
 
 public interface DiscoveryNodeIdExtractor {
-  UInt256 calculateDiscoveryNodeId(Peer peer);
+  Optional<UInt256> calculateDiscoveryNodeId(Peer peer);
 }

networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/peers/Eth2Peer.java

@@ -42,7 +42,7 @@ public interface Eth2Peer extends Peer, SyncSource {
   static Eth2Peer create(
       final Spec spec,
       final Peer peer,
-      final UInt256 discoveryNodeId,
+      final Optional<UInt256> discoveryNodeId,
       final BeaconChainMethods rpcMethods,
       final StatusMessageFactory statusMessageFactory,
       final MetadataMessagesFactory metadataMessagesFactory,
@@ -124,7 +124,7 @@ void adjustBlobSidecarsRequest(
 
   int getUnansweredPingCount();
 
-  UInt256 getDiscoveryNodeId();
+  Optional<UInt256> getDiscoveryNodeId();
 
   interface PeerStatusSubscriber {
     void onPeerStatus(final PeerStatus initialStatus);

networking/eth2/src/main/java/tech/pegasys/teku/networking/eth2/peers/LibP2PDiscoveryNodeIdExtractor.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright Consensys Software Inc., 2024
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+
+package tech.pegasys.teku.networking.eth2.peers;
+
+import java.util.Optional;
+import org.apache.tuweni.bytes.Bytes;
+import org.apache.tuweni.units.bigints.UInt256;
+import tech.pegasys.teku.networking.p2p.discovery.discv5.DiscV5Service;
+import tech.pegasys.teku.networking.p2p.libp2p.LibP2PPeer;
+import tech.pegasys.teku.networking.p2p.peer.Peer;
+
+public class LibP2PDiscoveryNodeIdExtractor implements DiscoveryNodeIdExtractor {
+  @Override
+  public Optional<UInt256> calculateDiscoveryNodeId(final Peer peer) {
+    if (peer instanceof LibP2PPeer libP2PPeer) {
+      try {
+        final Bytes libP2PPeerPublicKey = Bytes.wrap(libP2PPeer.getPubKey().raw());
+        final Bytes discoveryNodeIdBytes =
+            DiscV5Service.DEFAULT_NODE_RECORD_CONVERTER.convertPublicKeyToNodeId(
+                libP2PPeerPublicKey);
+        return Optional.of(UInt256.fromBytes(discoveryNodeIdBytes));
+      } catch (final Exception ignored) {
+        return Optional.empty();
+      }
+    }
+
+    return Optional.empty();
+  }
+}

networking/eth2/src/test/java/tech/pegasys/teku/networking/eth2/peers/Eth2PeerTest.java

@@ -25,7 +25,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.stream.IntStream;
-import org.apache.tuweni.units.bigints.UInt256;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
 import tech.pegasys.teku.infrastructure.async.SafeFuture;
@@ -72,7 +71,7 @@ class Eth2PeerTest {
       Eth2Peer.create(
           spec,
           delegate,
-          UInt256.ZERO,
+          Optional.empty(),
           rpcMethods,
           statusMessageFactory,
           metadataMessagesFactory,

networking/eth2/src/testFixtures/java/tech/pegasys/teku/networking/eth2/Eth2P2PNetworkFactory.java

@@ -33,7 +33,6 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
-import org.apache.tuweni.units.bigints.UInt256;
 import org.hyperledger.besu.metrics.noop.NoOpMetricsSystem;
 import tech.pegasys.teku.infrastructure.async.AsyncRunner;
 import tech.pegasys.teku.infrastructure.async.DelayedExecutorAsyncRunner;
@@ -230,7 +229,7 @@ protected Eth2P2PNetwork buildNetwork(final P2PConfig config) {
                 50,
                 spec,
                 KZG.NOOP,
-                (pk) -> UInt256.ZERO);
+                (__) -> Optional.empty());
 
         List<RpcMethod<?, ?, ?>> rpcMethods =
             eth2PeerManager.getBeaconChainMethods().all().stream()

networking/eth2/src/testFixtures/java/tech/pegasys/teku/networking/eth2/peers/RespondingEth2Peer.java

@@ -357,8 +357,8 @@ public int getUnansweredPingCount() {
   }
 
   @Override
-  public UInt256 getDiscoveryNodeId() {
-    return UInt256.ZERO;
+  public Optional<UInt256> getDiscoveryNodeId() {
+    return Optional.empty();
   }
 
   @Override

networking/p2p/src/main/java/tech/pegasys/teku/networking/p2p/discovery/discv5/NodeRecordConverter.java

@@ -23,11 +23,9 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.tuweni.bytes.Bytes;
-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 tech.pegasys.teku.infrastructure.ssz.collections.SszBitvector;
 import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszBitvectorSchema;
 import tech.pegasys.teku.networking.p2p.discovery.DiscoveryPeer;
@@ -38,13 +36,7 @@ public class NodeRecordConverter {
   private static final Logger LOG = LogManager.getLogger();
 
   public Bytes convertPublicKeyToNodeId(final Bytes publicKey) {
-    // TODO need to open an additional API in discovery instead of this hack
-    NodeRecord tempNodeRecord =
-        NodeRecordFactory.DEFAULT.createFromValues(
-            UInt64.ZERO,
-            new EnrField(EnrField.PKEY_SECP256K1, publicKey),
-            new EnrField(EnrField.ID, IdentitySchema.V4));
-    return tempNodeRecord.getNodeId();
+    return IdentitySchemaInterpreter.V4.calculateNodeId(publicKey);
   }
 
   public Optional<DiscoveryPeer> convertToDiscoveryPeer(