Add some networking functionality to Maru #89

pinges · 2025-04-30T08:41:19Z

This PR adds functionality based on libp2p using some internal classes from Teku.

static peers can be added
gossiping
sending of requests to individual peers and receiving of responses

Signed-off-by: [email protected] <[email protected]>

Copilot

Pull Request Overview

This PR adds peer-to-peer networking functionality to Maru using libp2p by implementing gossip, static peer management, and RPC communication.

Introduces several new classes for handling topics, building a P2P network, and processing RPC requests and responses.
Updates configuration and test files to support the new networking features.
Adds basic tests to verify static peer connection, disconnection, gossiping, and request/response behaviors.

Reviewed Changes

Copilot reviewed 15 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
p2p/src/main/kotlin/maru/p2p/TestTopicHandler.kt	Introduces a topic handler for gossip messages with basic validation.
p2p/src/main/kotlin/maru/p2p/P2PNetworkFactory.kt	Sets up the P2P network interface parsing (IPv4/IPv6) for network creation.
p2p/src/main/kotlin/maru/p2p/P2PNetworkBuilder.kt	Implements the construction of the full libp2p network including gossip and RPC configurations.
p2p/src/main/kotlin/maru/p2p/P2PManager.kt	Manages starting, stopping, and maintaining persistent static peer connections.
p2p/src/main/kotlin/maru/p2p/MaruRpcResponseHandler.kt	Handles RPC responses, completing a future on response.
p2p/src/main/kotlin/maru/p2p/MaruRpcMethod.kt	Configures creation of incoming/outgoing RPC handlers.
p2p/src/main/kotlin/maru/p2p/MaruPreparedGossipMessage.kt	Wraps original messages to be used in gossip transmission.
p2p/src/main/kotlin/maru/p2p/MaruPeerHandler.kt	Skeleton for handling peer connect/disconnect events.
p2p/src/main/kotlin/maru/p2p/MaruOutgoingRpcRequestHandler.kt	Processes outgoing RPC requests and responses.
p2p/src/main/kotlin/maru/p2p/MaruIncomingRpcRequestHandler.kt	Processes incoming RPC requests with simple echo logic.
config/src/main/kotlin/maru.config/Config.kt	Updates P2P configuration, including networks and static peers settings.
app/src/test/kotlin/maru/testutils/MaruFactory.kt	Adjusts test configurations to match the new networking port.
app/src/test/kotlin/maru/app/p2p/P2PTest.kt	Adds tests to validate static peer addition/removal, gossiping, and request handling.
app/src/main/kotlin/maru/app/MaruApp.kt	Integrates P2PManager into the application startup to enable networking.

Files not reviewed (6)

app/build.gradle: Language not supported
gradle/versions.gradle: Language not supported
key1: Language not supported
key2: Language not supported
p2p/build.gradle: Language not supported
settings.gradle: Language not supported

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

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

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

app/src/test/kotlin/maru/app/p2p/P2PTest.kt

build.gradle

app/build.gradle

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

app/src/test/kotlin/maru/app/p2p/P2PTest.kt

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

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

app/src/main/kotlin/maru/app/MaruApp.kt

app/src/test/kotlin/maru/app/p2p/P2PTest.kt

key1

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

app/src/main/kotlin/maru/app/MaruApp.kt

app/src/test/kotlin/maru/app/p2p/P2PTest.kt

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

jframe · 2025-05-01T05:46:59Z

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

+    p0: NodeId?,
+    p1: RpcStream?,
+  ) {
+    println("active $this")


can a logger be used for these print statements instead

Doesn't seem to be removed

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

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

Signed-off-by: [email protected] <[email protected]>

Filter94 · 2025-05-02T09:23:55Z

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

@@ -73,7 +96,7 @@ data class MaruConfig(
  val persistence: Persistence,
  val sotNode: ApiEndpointConfig,
  val qbftOptions: QbftOptions,
-  val p2pConfig: P2P?,
+  val p2pConfig: P2P,


I believe we agreed that it should be nullable after all. So we don't have to bother with P2P when we don't need it in some tests

Raising this again since this seems to be non-nullable still in the latest version of the code

Filter94 · 2025-05-02T09:25:21Z

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

@@ -66,7 +66,7 @@ class HopliteFriendlinessTest {
              endpoint = URI.create("http://localhost:8545").toURL(),
            ),
          qbftOptions = QbftOptions(100.milliseconds),
-          p2pConfig = P2P(port = 3322u),
+          p2pConfig = P2P(),


But the port should be 3322, not 9000. Something is wrong with this test. Also, we need to test that the rest of the P2P configs are configurable

Ok, I can actually see the test is failing because of that

p2p/build.gradle

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

Filter94 · 2025-05-02T09:37:56Z

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

+
+  private fun buildP2PNetwork(): P2PNetwork<Peer> {
+    val filePrivateKeySource = GeneratingFilePrivateKeySource(privateKeyFile.toString())
+    // TODO: reading/generating this key should be done early on, as it is needed for the validator as well


IMO generation should be done here, as a part of P2P network, but if it's set it can be read outside of this class

But moving the generation out of here also works

Filter94 · 2025-05-02T09:41:14Z

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

+    )
+  }
+
+  fun addStaticPeer(peerAddress: MultiaddrPeerAddress) {


Do we really need this method? I don't see how would it be useful

I guess we could expose an RPC method to add static peers, but we won't do it in the near future

That was what I was thinking, but we could just make it private for now?

The linter will probably complain about the dead code. Since it's already written, I guess we can leave it as is, but there's a change we'll never need it

Actually, I see it's used in the tests so it's fine to leave it

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

Filter94 · 2025-05-02T09:55:13Z

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

+      return SafeFuture.completedFuture(null)
+    }
+    return p2pNetwork
+      .connect(peerAddress)


Would it be better to keep the registry of open connections and check if any of them are broken instead of unconditionally connecting every time?

This is actually checked in the implementation in libp2p

If that's the case, I don't really see why we do this reconnect logic here

Can you please explain why we have this logic at all if the connection is maintained inside libp2p?

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

Filter94 · 2025-05-02T10:08:08Z

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

+    )
+  }
+
+  private fun getMessageFactory(


Can you please explain what this message factory is supposed to do?

To prepare messages for topics that you are not subscribed to.
When you are sending a message on a topic that you are subscribed to the prepareMessage() of the handler is called. If you are not subscribed a default class can be configured here.
Not sure if we need that ...

It could be useful for some sniffing, but I don't really see how it's useful on a regular Maru node. It's safer if we don't react on the topics we're not subscribed to in any way

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

Signed-off-by: [email protected] <[email protected]>

Filter94 · 2025-05-09T09:57:04Z

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

+
+class P2PManager(
+  private val privateKeyBytes: ByteArray,
+  private val p2pConfig: P2P?,


If p2pConfig is null, why would we create a P2PManager?

Filter94 · 2025-05-09T09:57:37Z

app/src/main/kotlin/maru/app/MaruApp.kt

    log.info(config.toString())
  }

+  val p2pManager =


I don't think we can create a P2PManager if config.p2pConfig is null

Filter94 · 2025-05-09T09:59:53Z

app/src/main/kotlin/maru/app/MaruApp.kt

+                privateKeyBytes =
+                  privateKeyBytes
+                    .slice(
+                      privateKeyBytes.size - 32..privateKeyBytes.size - 1,


Can you please explain, why do we take the last 32 bytes of the private key here? It should also be reflected in the name, because we have privateKeyBytes above that we read from a file and we're passing modified privateKeyBytes here.

Filter94 · 2025-05-09T10:02:51Z

app/src/test/kotlin/maru/testutils/MaruFactory.kt

+      appConfig
+        .domainFriendly()
+        .persistence.dataPath
+        .resolve("private-key")


I feel like it's a convention over configuration approach, which is arguable. We can use ./$dataPath/private-key as the default private key location in case private key's location isn't specified, but I feel like the software would be much more usable if we'd also allow an arbitrary private key path

Filter94 · 2025-05-09T10:07:49Z

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

 )

 data class ApiEndpointConfig(
  val endpoint: URL,
  val jwtSecretPath: String? = null,
-)
+) {
+  override fun equals(other: Any?): Boolean {


Overriden equals and hashCode shouldn't be necessary without ByteArray amongst the fields. Default implementation should be exactly the same

Filter94 · 2025-05-09T10:10:57Z

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

@@ -73,7 +96,7 @@ data class MaruConfig(
  val persistence: Persistence,
  val sotNode: ApiEndpointConfig,
  val qbftOptions: QbftOptions,
-  val p2pConfig: P2P?,
+  val p2pConfig: P2P,


Raising this again since this seems to be non-nullable still in the latest version of the code

Filter94 · 2025-05-09T11:22:20Z

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

+
+private const val LINEA = "linea"
+
+class MaruRpcMethod : RpcMethod<MaruOutgoingRpcRequestHandler, Bytes, MaruRpcResponseHandler> {


This class isn't ready to be in main sourceset yet

Filter94 · 2025-05-09T11:23:21Z

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

+    privateKey: PrivKey,
+    ipv4Address: Multiaddr,
+  ): P2PNetwork<Peer> {
+    val rpcMethod = MaruRpcMethod()


I think this has to be moved away from here and RpcHandler probably has to be injected here

Filter94 · 2025-05-09T11:25:53Z

p2p/src/test/kotlin/maru/p2p/P2PTest.kt

+private const val PEER_ADDRESS_NODE_2: String = "/ip4/$IPV4/tcp/$PORT2/p2p/$PEER_ID_NODE_2"
+private const val PEER_ADDRESS_NODE_3: String = "/ip4/$IPV4/tcp/$PORT3/p2p/$PEER_ID_NODE_3"
+
+class P2PTest {


As is, this test was useful to learn how to use LibP2PNetwork, but right now the useful part that it's testing is the creation of LibP2PNetwork by P2PNetworkFactory. I think we should change the test to test Maru's wrapper around LibP2PNetwork. The class that implements P2PNetwork

Filter94 · 2025-05-09T11:26:56Z

p2p/src/test/kotlin/maru/p2p/P2PTest.kt

+        }
+      assertThat(p2pNetwork1.peerCount).isEqualTo(1)
+      assertThat(p2pNetwork2.peerCount).isEqualTo(2)
+      assertThat(p2pNetwork3.peerCount).isEqualTo(1)


From the peer counts, it follows that p2pNetwork1 and 3 are probably not connected directly, but we could use a direct check

Filter94 · 2025-05-09T11:27:24Z

p2p/src/test/kotlin/maru/p2p/P2PTest.kt

+      val testTopicHandler3 = TestTopicHandler()
+      p2pNetwork3.subscribe("topic", testTopicHandler3)
+
+      Awaitility


Consider factoring this assertion out and use it here with a concise and descriptive name

pinges added 8 commits April 2, 2025 20:34

for Enrico and Stefan

edfd7fa

Signed-off-by: [email protected] <[email protected]>

make gossiping work

f97c398

Signed-off-by: [email protected] <[email protected]>

make it work with latest teku and add peer.sendRequest() test

692d9d9

Signed-off-by: [email protected] <[email protected]>

some clean up

0aef78e

Signed-off-by: [email protected] <[email protected]>

tests working

89c8a84

Signed-off-by: [email protected] <[email protected]>

test p2p on it's own

7423602

Signed-off-by: [email protected] <[email protected]>

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

60cea6d

tests for p2p working

7a2cc47

Signed-off-by: [email protected] <[email protected]>

pinges requested a review from Copilot April 30, 2025 08:41

Copilot AI reviewed Apr 30, 2025

View reviewed changes

Filter94 reviewed Apr 30, 2025

View reviewed changes

jframe reviewed May 1, 2025

View reviewed changes

changes after comments from Roman and Jason

574d8e7

Signed-off-by: [email protected] <[email protected]>