Merge branch 'main' of github.com:Consensys/maru into appDiscoveryTest

Author: pinges

config/src/main/kotlin/maru/config/Config.kt

@@ -52,8 +52,7 @@ data class P2PConfig(
   val gossiping: Gossiping = Gossiping(),
 ) {
   init {
-    // just a sanity check to ensure the IP address is valid
-    InetAddress.getByName(ipAddress)
+    validateIpAddress(ipAddress)
     require(reputation.smallChange > 0) {
       "smallChange must be a positive number"
     }
@@ -66,7 +65,27 @@ data class P2PConfig(
     val port: UInt = 9000u,
     val bootnodes: List<String> = emptyList(),
     val refreshInterval: Duration,
-  )
+    val advertisedIp: String? = null,
+  ) {
+    init {
+      advertisedIp?.let { validateIpAddress(it) }
+    }
+  }
+
+  companion object {
+    private fun validateIpAddress(ip: String) {
+      require(ip.isNotBlank()) {
+        "IP address must not be blank"
+      }
+      // InetAddress.getByName accepts both IP addresses and hostnames.
+      // We need to ensure it's actually an IP address by checking that
+      // the parsed address matches the input (no DNS resolution occurred)
+      val address = InetAddress.getByName(ip)
+      require(address.hostAddress == ip) {
+        "Invalid IP address format: $ip"
+      }
+    }
+  }
 
   data class StatusUpdate(
     val refreshInterval: Duration = 30.seconds,

config/src/test/kotlin/maru/config/HopliteFriendlinessTest.kt

@@ -48,6 +48,7 @@ class HopliteFriendlinessTest {
     port = 3324
     bootnodes = ["enr:-Iu4QHk0YN5IRRnufqsWkbO6Tn0iGTx4H_hnyiIEdXDuhIe0KKrxmaECisyvO40mEmmqKLhz_tdIhx2yFBK8XFKhvxABgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQOgBvD-dv0cX5szOeEsiAMtwxnP1q5CA5toYDrgUyOhV4N0Y3CCJBKDdWRwgiQT"]
     refresh-interval = "30 seconds"
+    advertised-ip = "13.12.11.10"
 
     [p2p.reputation]
     small-change = 1
@@ -116,6 +117,7 @@ class HopliteFriendlinessTest {
               "enr:-Iu4QHk0YN5IRRnufqsWkbO6Tn0iGTx4H_hnyiIEdXDuhIe0KKrxmaECisyvO40mEmmqKLhz_tdIhx2yFBK8XFKhvxABgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQOgBvD-dv0cX5szOeEsiAMtwxnP1q5CA5toYDrgUyOhV4N0Y3CCJBKDdWRwgiQT",
             ),
           refreshInterval = 30.seconds,
+          advertisedIp = "13.12.11.10",
         ),
       reputation =
         P2PConfig.Reputation(
@@ -652,4 +654,49 @@ class HopliteFriendlinessTest {
       ),
     )
   }
+
+  @Test
+  fun `p2p discovery with invalid advertisedIp format should fail`() {
+    val discoveryConfigToml =
+      """
+      port = 9000
+      refresh-interval = "30 seconds"
+      advertised-ip = "127.0.256.1"
+      """.trimIndent()
+
+    assertThatThrownBy {
+      parseConfig<P2PConfig.Discovery>(discoveryConfigToml)
+    }.isInstanceOf(ConfigException::class.java)
+      .hasMessageContaining("UnknownHostException")
+      .hasMessageContaining("127.0.256.1")
+  }
+
+  @Test
+  fun `p2p config with invalid ipAddress format should fail`() {
+    val p2pConfigToml =
+      """
+      ip-address = "127.O.0H.1"
+      """.trimIndent()
+
+    assertThatThrownBy {
+      parseConfig<P2PConfig>(p2pConfigToml)
+    }.isInstanceOf(ConfigException::class.java)
+      .hasMessageContaining("UnknownHostException")
+      .hasMessageContaining("127.O.0H.1")
+  }
+
+  @Test
+  fun `p2p config with valid dns instead of IP format should fail`() {
+    val p2pConfigToml =
+      """
+      ip-address = "test.com"
+      """.trimIndent()
+
+    assertThatThrownBy {
+      parseConfig<P2PConfig>(p2pConfigToml)
+    }.isInstanceOf(ConfigException::class.java)
+      .hasMessageContaining("IllegalArgumentException")
+      .hasMessageContaining("Invalid IP address format")
+      .hasMessageContaining("test.com")
+  }
 }

p2p/src/main/kotlin/maru/p2p/NetworkHelper.kt

@@ -36,8 +36,6 @@ object NetworkHelper {
       .filter { it is Inet4Address }
       .map { it.hostAddress }
 
-  fun hasInterfaceWithIpV4(ipV4: String): Boolean = listIpsV4(excludeLoopback = false).contains(ipV4)
-
   fun selectIpV4ForP2P(
     targetIpV4: String,
     excludeLoopback: Boolean = true,

p2p/src/main/kotlin/maru/p2p/discovery/MaruDiscoveryService.kt

@@ -223,9 +223,9 @@ class MaruDiscoveryService(
         .secretKey(privateKey)
         .seq(UInt64.valueOf(sequenceNumber.toBigInteger()))
         .address(
-          p2pConfig.ipAddress,
-          p2pConfig.discovery!!.port.toInt(),
-          p2pConfig.port.toInt(),
+          /* ipAddress = */ p2pConfig.discovery!!.advertisedIp ?: p2pConfig.ipAddress,
+          /* udpPort = */ p2pConfig.discovery!!.port.toInt(),
+          /* tcpPort = */ p2pConfig.port.toInt(),
         ).customField(FORK_ID_HASH_FIELD_NAME, Bytes.wrap(forkIdHashManager.currentForkHash()))
     // TODO: do we want more custom fields to identify version/topics/role/something else?
 

p2p/src/test/kotlin/maru/p2p/discovery/MaruDiscoveryServiceTest.kt

@@ -293,4 +293,50 @@ class MaruDiscoveryServiceTest {
 
     assertThat(result).isFalse()
   }
+
+  @Test
+  fun `local node record uses advertisedIp when set`() {
+    val ipAddress = "127.0.0.1"
+    val advertisedIp = "203.0.113.50"
+    val port = 9001u
+    val discoveryPort = 9000u
+
+    val p2pConfig =
+      P2PConfig(
+        ipAddress = ipAddress,
+        port = port,
+        discovery =
+          P2PConfig.Discovery(
+            port = discoveryPort,
+            bootnodes = listOf(),
+            refreshInterval = 10.seconds,
+            advertisedIp = advertisedIp,
+          ),
+      )
+
+    val discoveryService =
+      MaruDiscoveryService(
+        privateKeyBytes = keyPair.secretKey().bytesArray(),
+        p2pConfig = p2pConfig,
+        forkIdHashManager = forkIdHashProvider,
+        p2PState = InMemoryP2PState(),
+      )
+
+    val localNodeRecord = discoveryService.getLocalNodeRecord()
+
+    assertThat(localNodeRecord.udpAddress.isPresent).isTrue()
+    assertThat(
+      localNodeRecord.udpAddress
+        .get()
+        .address.hostAddress,
+    ).isEqualTo(advertisedIp)
+    assertThat(localNodeRecord.udpAddress.get().port).isEqualTo(discoveryPort.toInt())
+    assertThat(localNodeRecord.tcpAddress.isPresent).isTrue()
+    assertThat(
+      localNodeRecord.tcpAddress
+        .get()
+        .address.hostAddress,
+    ).isEqualTo(advertisedIp)
+    assertThat(localNodeRecord.tcpAddress.get().port).isEqualTo(port.toInt())
+  }
 }