Skip to content
This repository was archived by the owner on Jan 16, 2026. It is now read-only.

chore(rpc): Connect Peer RPC Request#2034

Closed
varun-doshi wants to merge 1 commit intoop-rs:mainfrom
varun-doshi:varun/conenct-peer
Closed

chore(rpc): Connect Peer RPC Request#2034
varun-doshi wants to merge 1 commit intoop-rs:mainfrom
varun-doshi:varun/conenct-peer

Conversation

@varun-doshi
Copy link
Contributor

@varun-doshi varun-doshi commented Jun 6, 2025

Description

Implements the opp2p_connectPeer rpc request.

Closes #1575

Copilot AI review requested due to automatic review settings June 6, 2025 15:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Implements the opp2p_connectPeer JSON-RPC request, parsing the peer ID and forwarding a ConnectPeer command through the P2P subsystem.

  • Adds opp2p_connect_peer handler in the RPC server to parse the incoming string and send a ConnectPeer request.
  • Introduces a new ConnectPeer variant in P2pRpcRequest and its corresponding connect_peer handler.
  • All existing P2P RPC routes are now symmetric for connect/disconnect operations.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/node/rpc/src/p2p.rs Implemented opp2p_connect_peer, parsing input and sending P2pRpcRequest::ConnectPeer.
crates/node/p2p/src/rpc/request.rs Added ConnectPeer enum variant and connect_peer method to dial a peer.
Comments suppressed due to low confidence (1)

crates/node/rpc/src/p2p.rs:145

  • Add unit or integration tests for opp2p_connect_peer covering both successful connections and failure cases (invalid peer ID parsing and internal send errors).
async fn opp2p_connect_peer(&self, peer_id: String) -> RpcResult<()> {

Comment on lines +46 to 51
/// The peer id to connect.
peer_id: PeerId,
},
/// Request to disconnect the specified peer.
DisconnectPeer {
/// The peer id to disconnect.
Copy link

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] In the doc comment, consider changing "peer id" to "peer ID" for consistency with other code and logs.

Suggested change
/// The peer id to connect.
peer_id: PeerId,
},
/// Request to disconnect the specified peer.
DisconnectPeer {
/// The peer id to disconnect.
/// The peer ID to connect.
peer_id: PeerId,
},
/// Request to disconnect the specified peer.
DisconnectPeer {
/// The peer ID to disconnect.

Copilot uses AI. Check for mistakes.
}
}

fn connect_peer(peer_id: PeerId, gossip: &mut GossipDriver) {
Copy link

Copilot AI Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Logging targets differ between the RPC server (target: "rpc") and this handler (target: "p2p::rpc"); consider unifying or documenting the distinction for consistent observability.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Jun 6, 2025

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 83.0%. Comparing base (0244757) to head (7ec279d).
Report is 5 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/node/rpc/src/p2p.rs 0.0% 10 Missing ⚠️
crates/node/p2p/src/rpc/request.rs 0.0% 6 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@refcell
Copy link
Contributor

refcell commented Jun 6, 2025

I actually already had this drafted and just put up a pr with it. We want to parse the string as a multiaddress like how the optimism monorepo parses the string using go libp2p's AddrInfoFromString method.

@varun-doshi
Copy link
Contributor Author

I actually already had this drafted and just put up a pr with it. We want to parse the string as a multiaddress like how the optimism monorepo parses the string using go libp2p's AddrInfoFromString method.

Ahh understood

I'll close this PR

@varun-doshi varun-doshi closed this Jun 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(node/p2p): opp2p_connectPeer RPC Method Support

3 participants