feat(l1): add rpc endpoint admin_peers #2732

fmoletta · 2025-05-09T18:50:21Z

Motivation
Support rpc endpoint admin_peers

Description

Add rpc endpoint admin_peers
Track inbound connections
Store peer node version when starting a connection
Add peer_handler: PeerHandler field to RpcContext so we can access peers from the rpc
(Misc) Syncer & SyncManager now receive a PeerHandler upon creation instead of a KademliaTable
(Misc) Fix common typo across the project

Data missing compared to geth implementation:

The local address of each connection
Whether a connection is trusted, static (we have no notion of this yet)

Closes #2671

github-actions · 2025-05-09T19:27:22Z

Lines of code report

Total lines added: 151
Total lines removed: 4
Total lines changed: 155

Detailed view

+-------------------------------------------------+-------+------+
| File                                            | Lines | Diff |
+-------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/initializers.rs               | 373   | +3   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/kademlia.rs        | 495   | +4   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/peer_handler.rs    | 557   | +14  |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/connection.rs | 543   | +6   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/p2p.rs        | 269   | +9   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync.rs            | 570   | -4   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/types.rs           | 408   | +2   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/admin/mod.rs       | 53    | +2   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/admin/peers.rs     | 103   | +103 |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/rpc.rs             | 629   | +5   |
+-------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/utils.rs           | 408   | +3   |
+-------------------------------------------------+-------+------+

mpaulucci · 2025-05-12T15:05:03Z

crates/networking/rpc/rpc.rs

@@ -170,6 +173,7 @@ pub async fn start_api(
        active_filters: active_filters.clone(),
        syncer: Arc::new(syncer),
        client_version,
+        peer_handler,


Could we refactor this to group a bunch of p2p related concepts like local_p2p_node, local_node_record , peer_handler , maybe syncer into something like Node or similar? Maybe out of scope of this PR, but RpcApiContext is like a bag were we just throw random things, would be nice to be a bit more thoughtful in this regard.

I think syncer and peer handler should remain separate from the rest as they have their own api and are not used in conjunction. I think readable data such as jwt_secret and local node data could be grouped together

Ill do this in a separate PR to be cleaner

PR opened #2752

Since the PR has been merged, the peer handler should be in the Node now

NodeData contains static data about our node, while the PeerHandler actually has its own behaviour and has nothing to do with the data stored in NodeData, that's why Id rather keep them separate

**Motivation** `RpcContext` has become quite extensive lately, so we need to group some of its fields into individual structures that hold data used for similar purposes. This PR groups static data related to the node (such as p2p address, version, etc) into a `NodeData` struct in order to simplify it. **Description** * Group static data about the node held in `RpcContext` into `NodeData` * (Misc) Remove duplicated code between test functions   Addresses review comment: #2732 (comment)

github-actions · 2025-05-15T20:19:22Z

Benchmark for `fd726ec`

Click to view benchmark

Test	Base	PR	%
Trie/cita-trie insert 10k	36.3±0.77ms	36.5±0.71ms	+0.55%
Trie/cita-trie insert 1k	3.6±0.05ms	3.8±0.04ms	+5.56%
Trie/ethrex-trie insert 10k	127.9±1.66ms	129.9±3.30ms	+1.56%
Trie/ethrex-trie insert 1k	11.0±0.06ms	11.3±0.21ms	+2.73%

github-actions · 2025-05-15T21:06:42Z

Benchmark for `7d331aa`

Click to view benchmark

Test	Base	PR	%
Trie/cita-trie insert 10k	41.0±2.15ms	40.0±2.33ms	-2.44%
Trie/cita-trie insert 1k	3.6±0.04ms	3.7±0.04ms	+2.78%
Trie/ethrex-trie insert 10k	134.8±1.96ms	134.5±3.47ms	-0.22%
Trie/ethrex-trie insert 1k	11.2±0.11ms	11.2±0.13ms	0.00%

github-actions · 2025-05-15T21:48:03Z

Benchmark for `e70f8b2`

Click to view benchmark

Test	Base	PR	%
Trie/cita-trie insert 10k	39.6±1.88ms	37.3±1.73ms	-5.81%
Trie/cita-trie insert 1k	3.8±0.05ms	3.6±0.04ms	-5.26%
Trie/ethrex-trie insert 10k	133.3±2.35ms	137.9±3.53ms	+3.45%
Trie/ethrex-trie insert 1k	11.1±0.07ms	11.2±0.12ms	+0.90%

fedacking

We should integrate the peer table into the Node. Rest looks good

fedacking · 2025-05-16T13:55:55Z

crates/networking/rpc/rpc.rs

@@ -170,6 +173,7 @@ pub async fn start_api(
        active_filters: active_filters.clone(),
        syncer: Arc::new(syncer),
        client_version,
+        peer_handler,


Since the PR has been merged, the peer handler should be in the Node now

fedacking · 2025-05-16T13:58:15Z

crates/networking/p2p/kademlia.rs

    ) {
        let peer = self.get_by_node_id_mut(node_id);
        if let Some(peer) = peer {
            peer.channels = Some(channels);
            peer.supported_capabilities = capabilities;
            peer.is_connected = true;
+            peer.is_connection_inbound = inbound;
        } else {
            debug!(
                "[PEERS] Peer with node_id {:?} not found in the kademlia table when trying to init backend communication",


Should the debug information also include if it's inbound?

This looks like a very edge case rather than something we want to actively look for

mpaulucci · 2025-05-16T17:14:07Z

crates/networking/p2p/peer_handler.rs

+
+    /// Creates a dummy PeerHandler for tests where interacting with peers is not needed
+    /// This should only be used in tests as it won't be able to interact with the node's connected peers
+    pub fn dummy() -> PeerHandler {


nit: keep this inside test flag?

I would like to but the test feature doesn't behave well when importing across crates

mpaulucci

LGTM

fedacking

LGTM

…to rpc-admin-peers

github-actions · 2025-05-16T22:16:10Z

Benchmark for `700f068`

Click to view benchmark

Test	Base	PR	%
Trie/cita-trie insert 10k	38.1±1.40ms	38.4±1.92ms	+0.79%
Trie/cita-trie insert 1k	3.6±0.04ms	3.7±0.03ms	+2.78%
Trie/ethrex-trie insert 10k	131.4±1.52ms	133.0±3.89ms	+1.22%
Trie/ethrex-trie insert 1k	11.2±0.06ms	11.2±0.08ms	0.00%

mpaulucci reviewed May 12, 2025

View reviewed changes

fmoletta mentioned this pull request May 12, 2025

refactor(l1): group node data in RpcContext #2752

Merged

fmoletta added 10 commits May 15, 2025 16:47

feat: add rpc endpoint admin_peers

f735fea

push file

8489f12

fix serde + remove name parameter (as we do not know it)

ccc2852

Add doc

e47e629

Track wether connections are inbound

4c25ed2

Minor fixes + test"

e0e4f8a

Fix comparison string format

a8edd33

intergate node id refactor changes"

6f2c229

remove unused import

c178b93

fixes

b771dfd

fmoletta force-pushed the rpc-admin-peers branch from ca4479c to b771dfd Compare May 15, 2025 20:08

Fix common typo

0ad77e6

fmoletta marked this pull request as ready for review May 15, 2025 20:17

fmoletta requested a review from a team as a code owner May 15, 2025 20:17

fmoletta added 3 commits May 15, 2025 18:01

feat(l1): store peer node version when starting a connection

544b991

fmt

faffca2

Merge branch 'store-peer-node-version' into rpc-admin-peers

e2686d0

Add name field to peer data returned

6065c5e

fedacking requested changes May 16, 2025

View reviewed changes

mpaulucci reviewed May 16, 2025

View reviewed changes

mpaulucci approved these changes May 16, 2025

View reviewed changes

fedacking approved these changes May 16, 2025

View reviewed changes

Merge branch 'main' of github.com:lambdaclass/lambda_ethereum_rust in…

9e0de7f

…to rpc-admin-peers

fmoletta enabled auto-merge May 16, 2025 22:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(l1): add rpc endpoint admin_peers #2732

feat(l1): add rpc endpoint admin_peers #2732

fmoletta commented May 9, 2025 •

edited

Loading

github-actions bot commented May 9, 2025 •

edited

Loading

mpaulucci May 12, 2025

fmoletta May 12, 2025

fmoletta May 12, 2025

fmoletta May 12, 2025

fedacking May 16, 2025

fmoletta May 16, 2025

github-actions bot commented May 15, 2025

github-actions bot commented May 15, 2025

github-actions bot commented May 15, 2025

fedacking left a comment

fedacking May 16, 2025

fedacking May 16, 2025

fmoletta May 16, 2025

mpaulucci May 16, 2025

fmoletta May 16, 2025

mpaulucci left a comment

fedacking left a comment

github-actions bot commented May 16, 2025

feat(l1): add rpc endpoint admin_peers #2732

Are you sure you want to change the base?

feat(l1): add rpc endpoint admin_peers #2732

Conversation

fmoletta commented May 9, 2025 • edited Loading

github-actions bot commented May 9, 2025 • edited Loading

Lines of code report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 15, 2025

Benchmark for fd726ec

github-actions bot commented May 15, 2025

Benchmark for 7d331aa

github-actions bot commented May 15, 2025

Benchmark for e70f8b2

fedacking left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpaulucci left a comment

Choose a reason for hiding this comment

fedacking left a comment

Choose a reason for hiding this comment

github-actions bot commented May 16, 2025

Benchmark for 700f068

fmoletta commented May 9, 2025 •

edited

Loading

github-actions bot commented May 9, 2025 •

edited

Loading

Benchmark for `fd726ec`

Benchmark for `7d331aa`

Benchmark for `e70f8b2`

Benchmark for `700f068`