Skip to content

Commit 37de823

Browse files
iamquang95iamquang95
authored
refactor(p2p): not allow builder to set p2p_context (#348)
* refactor(p2p): not allow builder to set p2p_context * fix: test nodesigs * fix: address comments
1 parent 4b46b02 commit 37de823

11 files changed

Lines changed: 88 additions & 79 deletions

File tree

‎crates/dkg/examples/bcast.rs‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ use pluto_p2p::{
8282
config::P2PConfig,
8383
gater, k1,
8484
p2p::{Node, NodeType},
85+
p2p_context::P2PContext,
8586
relay::{MutableRelayReservation, RelayRouter},
8687
};
8788
use pluto_tracing::TracingConfig;
@@ -579,13 +580,14 @@ async fn main() -> Result<()> {
579580
disable_reuse_port: args.disable_reuse_port,
580581
};
581582

583+
let p2p_context = P2PContext::new(known_peers.clone());
582584
let mut component = None;
583585
let mut node: Node<ExampleBehaviour> = Node::new(
584586
p2p_config,
585587
key.clone(),
586588
NodeType::QUIC,
587589
args.filter_private_addrs,
588-
known_peers,
590+
p2p_context,
589591
|builder, keypair, relay_client| {
590592
let p2p_context = builder.p2p_context();
591593
let local_peer_id = keypair.public().to_peer_id();
@@ -594,15 +596,12 @@ async fn main() -> Result<()> {
594596
bcast::Behaviour::new(cluster_peers.clone(), p2p_context.clone(), key.clone());
595597
component = Some(c);
596598

597-
builder
598-
.with_p2p_context(p2p_context.clone())
599-
.with_gater(conn_gater)
600-
.with_inner(ExampleBehaviour {
601-
relay: relay_client,
602-
relay_reservation: MutableRelayReservation::new(relays.clone()),
603-
relay_router: RelayRouter::new(relays.clone(), p2p_context, local_peer_id),
604-
bcast: bcast_behaviour,
605-
})
599+
builder.with_gater(conn_gater).with_inner(ExampleBehaviour {
600+
relay: relay_client,
601+
relay_reservation: MutableRelayReservation::new(relays.clone()),
602+
relay_router: RelayRouter::new(relays.clone(), p2p_context, local_peer_id),
603+
bcast: bcast_behaviour,
604+
})
606605
},
607606
)?;
608607

‎crates/dkg/src/bcast/behaviour.rs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -700,12 +700,8 @@ mod tests {
700700
key,
701701
NodeType::TCP,
702702
false,
703-
peer_ids.clone(),
704-
move |builder, _keypair| {
705-
builder
706-
.with_p2p_context(p2p_context.clone())
707-
.with_inner(behaviour)
708-
},
703+
p2p_context.clone(),
704+
move |builder, _keypair| builder.with_inner(behaviour),
709705
)?;
710706
let addr: Multiaddr = format!("/ip4/127.0.0.1/tcp/{}", ports[index]).parse()?;
711707
nodes.push(LocalNode {

‎crates/dkg/src/nodesigs.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -569,8 +569,8 @@ mod tests {
569569
key.clone(),
570570
NodeType::TCP,
571571
false,
572-
peer_ids.clone(),
573-
move |builder, _| builder.with_p2p_context(p2p_context).with_inner(behaviour),
572+
p2p_context,
573+
move |builder, _| builder.with_inner(behaviour),
574574
)?;
575575

576576
let addr: Multiaddr = format!("/ip4/127.0.0.1/tcp/{}", ports[index]).parse()?;

‎crates/p2p/examples/bootnode.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ use pluto_p2p::{
4242
config::P2PConfig,
4343
gater, k1,
4444
p2p::{Node, NodeType},
45+
p2p_context::P2PContext,
4546
relay::{MutableRelayReservation, RelayRouter},
4647
};
4748
use pluto_tracing::TracingConfig;
@@ -143,7 +144,7 @@ pub async fn main() -> Result<()> {
143144
pk,
144145
NodeType::QUIC,
145146
false,
146-
known_peers.clone(),
147+
P2PContext::new(known_peers.clone()),
147148
|builder, keypair, relay_client| {
148149
let p2p_context = builder.p2p_context();
149150
let local_peer_id = keypair.public().to_peer_id();

‎crates/p2p/examples/p2p.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use pluto_p2p::{
2020
behaviours::pluto::PlutoBehaviourEvent,
2121
config::P2PConfig,
2222
p2p::{Node, NodeType},
23+
p2p_context::P2PContext,
2324
};
2425
use tokio::signal;
2526

@@ -73,13 +74,13 @@ async fn main() -> Result<()> {
7374

7475
// Create node with composed behaviour
7576
// No known cluster peers in this example
76-
let known_peers: Vec<libp2p::PeerId> = vec![];
77+
let p2p_context = P2PContext::default();
7778
let mut p2p = Node::new(
7879
P2PConfig::default(),
7980
key.clone(),
8081
NodeType::QUIC,
8182
false,
82-
known_peers,
83+
p2p_context,
8384
|builder, keypair, relay_client| {
8485
builder
8586
.with_user_agent("pluto-p2p-example/1.0.0")

‎crates/p2p/examples/quic_upgrade.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ use pluto_p2p::{
5959
behaviours::pluto::PlutoBehaviourEvent,
6060
config::P2PConfig,
6161
p2p::{Node, NodeType},
62+
p2p_context::P2PContext,
6263
quic_upgrade::QuicUpgradeEvent,
6364
};
6465
use tokio::signal;
@@ -112,7 +113,7 @@ async fn main() -> Result<()> {
112113
key,
113114
NodeType::QUIC, // Enable QUIC transport
114115
false, // Don't filter private addresses (for local testing)
115-
known_peers,
116+
P2PContext::new(known_peers),
116117
|builder, _keypair, relay_client| {
117118
builder
118119
.with_user_agent("quic-upgrade-example/1.0.0")

‎crates/p2p/src/behaviours/pluto.rs‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ pub struct PlutoBehaviour<B: NetworkBehaviour> {
6262

6363
impl<B: NetworkBehaviour> PlutoBehaviour<B> {
6464
/// Returns a new builder for configuring a PlutoBehaviour.
65-
pub fn builder() -> PlutoBehaviourBuilder<B> {
66-
PlutoBehaviourBuilder::default()
65+
pub fn builder(p2p_context: P2PContext) -> PlutoBehaviourBuilder<B> {
66+
PlutoBehaviourBuilder::new(p2p_context)
6767
}
6868
}
6969

@@ -96,25 +96,20 @@ pub struct PlutoBehaviourBuilder<B> {
9696
inner: Option<B>,
9797
}
9898

99-
impl<B> Default for PlutoBehaviourBuilder<B> {
100-
fn default() -> Self {
99+
impl<B: NetworkBehaviour> PlutoBehaviourBuilder<B> {
100+
/// Creates a new builder with default configuration and the provided shared
101+
/// P2P context.
102+
pub fn new(p2p_context: P2PContext) -> Self {
101103
Self {
102104
gater: None,
103105
identify_protocol: DEFAULT_IDENTIFY_PROTOCOL.clone(),
104106
user_agent: DEFAULT_USER_AGENT.clone(),
105107
autonat_config: autonat::Config::default(),
106-
p2p_context: P2PContext::default(),
108+
p2p_context,
107109
quic_enabled: false,
108110
inner: None,
109111
}
110112
}
111-
}
112-
113-
impl<B: NetworkBehaviour> PlutoBehaviourBuilder<B> {
114-
/// Creates a new builder with default configuration.
115-
pub fn new() -> Self {
116-
Self::default()
117-
}
118113

119114
/// Returns the cloned P2P context.
120115
pub fn p2p_context(&self) -> P2PContext {
@@ -167,14 +162,6 @@ impl<B: NetworkBehaviour> PlutoBehaviourBuilder<B> {
167162
self
168163
}
169164

170-
/// Sets the global context.
171-
///
172-
/// The global context is used to store the peer store.
173-
pub fn with_p2p_context(mut self, p2p_context: P2PContext) -> Self {
174-
self.p2p_context = p2p_context;
175-
self
176-
}
177-
178165
/// Sets whether QUIC is enabled.
179166
///
180167
/// When enabled, the behaviour will periodically attempt to upgrade
@@ -191,7 +178,6 @@ impl<B: NetworkBehaviour> PlutoBehaviourBuilder<B> {
191178
/// * `key` - The keypair for this node, used for identify and autonat
192179
pub fn build(self, key: &Keypair) -> PlutoBehaviour<B> {
193180
let local_peer_id = key.public().to_peer_id();
194-
self.p2p_context.set_local_peer_id(local_peer_id);
195181

196182
let identify_config = identify::Config::new(self.identify_protocol, key.public())
197183
.with_agent_version(self.user_agent)

‎crates/p2p/src/p2p.rs‎

Lines changed: 50 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
//! secret_key,
2424
//! NodeType::QUIC,
2525
//! false, // filter_private_addrs
26-
//! vec![], // known_peers
27-
//! |builder, _p2p_ctx, _keypair, relay_client| {
26+
//! P2PContext::default(),
27+
//! |builder, _keypair, relay_client| {
2828
//! builder
2929
//! .with_user_agent("my-app/1.0.0")
3030
//! .with_inner(relay_client)
@@ -42,8 +42,8 @@
4242
//! secret_key,
4343
//! NodeType::QUIC,
4444
//! false, // filter_private_addrs
45-
//! vec![], // known_peers
46-
//! |builder, _p2p_ctx, keypair, relay_client| {
45+
//! P2PContext::default(),
46+
//! |builder, keypair, relay_client| {
4747
//! builder
4848
//! .with_user_agent("my-app/1.0.0")
4949
//! .with_inner(MyBehaviour {
@@ -67,8 +67,8 @@
6767
//! secret_key,
6868
//! NodeType::TCP,
6969
//! false, // filter_private_addrs
70-
//! vec![], // known_peers
71-
//! |builder, _p2p_ctx, keypair| {
70+
//! P2PContext::default(),
71+
//! |builder, keypair| {
7272
//! builder.with_inner(
7373
//! relay::Behaviour::new(keypair.public().to_peer_id(), relay_config)
7474
//! )
@@ -148,6 +148,15 @@ pub enum P2PError {
148148
/// Failed to parse IP address.
149149
#[error("Failed to parse IP address: {0}")]
150150
FailedToParseIpAddress(#[from] std::net::AddrParseError),
151+
152+
/// The provided P2P context is already bound to a different local peer ID.
153+
#[error("P2P context local peer ID mismatch: expected {expected}, got {actual}")]
154+
LocalPeerIdMismatch {
155+
/// Local peer ID derived from the node keypair.
156+
expected: Box<PeerId>,
157+
/// Local peer ID already bound in the shared P2P context.
158+
actual: Box<PeerId>,
159+
},
151160
}
152161

153162
impl P2PError {
@@ -183,18 +192,18 @@ pub struct Node<B: NetworkBehaviour> {
183192
impl<B: NetworkBehaviour> Node<B> {
184193
/// Creates a new client node with relay client support.
185194
///
186-
/// The `behaviour_fn` receives a default `PlutoBehaviourBuilder`, the P2P
187-
/// context, keypair, and relay client. It should configure the builder
188-
/// (e.g., set user agent, inner behaviour) and return it. The builder
189-
/// will then be finalized internally.
195+
/// The `behaviour_fn` receives a `PlutoBehaviourBuilder`, keypair, and
196+
/// relay client. It should configure the builder (e.g., set user agent,
197+
/// inner behaviour) and return it. The builder will then be finalized
198+
/// internally.
190199
///
191200
/// # Arguments
192201
///
193202
/// * `cfg` - P2P configuration for addresses and networking
194203
/// * `key` - Secret key for node identity
195204
/// * `node_type` - Transport type (TCP or QUIC)
196205
/// * `filter_private_addrs` - Whether to filter private addresses
197-
/// * `known_peers` - List of known cluster peer IDs for metrics tracking
206+
/// * `p2p_context` - Shared P2P runtime context for this node
198207
/// * `behaviour_fn` - Closure that configures and returns the behaviour
199208
/// builder
200209
///
@@ -206,8 +215,8 @@ impl<B: NetworkBehaviour> Node<B> {
206215
/// secret_key,
207216
/// NodeType::QUIC,
208217
/// false,
209-
/// vec![peer1, peer2], // known cluster peers
210-
/// |builder, _p2p_ctx, _keypair, relay_client| {
218+
/// P2PContext::new(vec![peer1, peer2]),
219+
/// |builder, _keypair, relay_client| {
211220
/// builder
212221
/// .with_user_agent("my-app/1.0.0")
213222
/// .with_inner(MyBehaviour { relay_client, peerinfo: ... })
@@ -219,7 +228,7 @@ impl<B: NetworkBehaviour> Node<B> {
219228
key: k256::SecretKey,
220229
node_type: NodeType,
221230
filter_private_addrs: bool,
222-
known_peers: impl IntoIterator<Item = PeerId>,
231+
p2p_context: P2PContext,
223232
behaviour_fn: F,
224233
) -> Result<Self>
225234
where
@@ -230,7 +239,7 @@ impl<B: NetworkBehaviour> Node<B> {
230239
) -> PlutoBehaviourBuilder<B>,
231240
{
232241
let keypair = utils::keypair_from_secret_key(key)?;
233-
let p2p_context = P2PContext::new(known_peers);
242+
Self::bind_local_peer_id(&p2p_context, keypair.public().to_peer_id())?;
234243

235244
let mut node = match node_type {
236245
NodeType::TCP => Self::build_tcp_client(keypair, p2p_context, behaviour_fn),
@@ -247,22 +256,22 @@ impl<B: NetworkBehaviour> Node<B> {
247256
/// Server nodes (like relay servers) don't include relay client support
248257
/// since they are expected to be publicly reachable.
249258
///
250-
/// The `behaviour_fn` receives a default `PlutoBehaviourBuilder`, the P2P
251-
/// context, and keypair. It should configure the builder (e.g., set user
252-
/// agent, inner behaviour) and return it.
259+
/// The `behaviour_fn` receives a `PlutoBehaviourBuilder` and keypair. It
260+
/// should configure the builder (e.g., set user agent, inner behaviour)
261+
/// and return it.
253262
pub fn new_server<F>(
254263
cfg: P2PConfig,
255264
key: k256::SecretKey,
256265
node_type: NodeType,
257266
filter_private_addrs: bool,
258-
known_peers: impl IntoIterator<Item = PeerId>,
267+
p2p_context: P2PContext,
259268
behaviour_fn: F,
260269
) -> Result<Self>
261270
where
262271
F: FnOnce(PlutoBehaviourBuilder<B>, &Keypair) -> PlutoBehaviourBuilder<B>,
263272
{
264273
let keypair = utils::keypair_from_secret_key(key)?;
265-
let p2p_context = P2PContext::new(known_peers);
274+
Self::bind_local_peer_id(&p2p_context, keypair.public().to_peer_id())?;
266275

267276
let mut node = match node_type {
268277
NodeType::TCP => Self::build_tcp_server(keypair, p2p_context, behaviour_fn),
@@ -317,6 +326,22 @@ impl<B: NetworkBehaviour> Node<B> {
317326
Ok(())
318327
}
319328

329+
fn bind_local_peer_id(p2p_context: &P2PContext, local_peer_id: PeerId) -> Result<()> {
330+
match p2p_context.local_peer_id() {
331+
Some(existing_peer_id) if existing_peer_id != local_peer_id => {
332+
Err(P2PError::LocalPeerIdMismatch {
333+
expected: Box::new(local_peer_id),
334+
actual: Box::new(existing_peer_id),
335+
})
336+
}
337+
Some(_) => Ok(()),
338+
None => {
339+
p2p_context.set_local_peer_id(local_peer_id);
340+
Ok(())
341+
}
342+
}
343+
}
344+
320345
fn build_quic_client<F>(
321346
keypair: Keypair,
322347
p2p_context: P2PContext,
@@ -339,9 +364,8 @@ impl<B: NetworkBehaviour> Node<B> {
339364
.with_relay_client(noise::Config::new, yamux_config)
340365
.map_err(P2PError::failed_to_build_swarm)?
341366
.with_behaviour(|key, relay_client| {
342-
let builder = PlutoBehaviourBuilder::default()
343-
.with_p2p_context(p2p_context.clone())
344-
.with_quic_enabled(true);
367+
let builder =
368+
PlutoBehaviourBuilder::new(p2p_context.clone()).with_quic_enabled(true);
345369
behaviour_fn(builder, key, relay_client).build(key)
346370
})
347371
.map_err(P2PError::failed_to_build_swarm)?
@@ -376,8 +400,7 @@ impl<B: NetworkBehaviour> Node<B> {
376400
.with_relay_client(noise::Config::new, yamux_config)
377401
.map_err(P2PError::failed_to_build_swarm)?
378402
.with_behaviour(|key, relay_client| {
379-
let builder =
380-
PlutoBehaviourBuilder::default().with_p2p_context(p2p_context.clone());
403+
let builder = PlutoBehaviourBuilder::new(p2p_context.clone());
381404
behaviour_fn(builder, key, relay_client).build(key)
382405
})
383406
.map_err(P2PError::failed_to_build_swarm)?
@@ -407,8 +430,7 @@ impl<B: NetworkBehaviour> Node<B> {
407430
.with_dns()
408431
.map_err(P2PError::failed_to_build_swarm)?
409432
.with_behaviour(|key| {
410-
let builder =
411-
PlutoBehaviourBuilder::default().with_p2p_context(p2p_context.clone());
433+
let builder = PlutoBehaviourBuilder::new(p2p_context.clone());
412434
behaviour_fn(builder, key).build(key)
413435
})
414436
.map_err(P2PError::failed_to_build_swarm)?
@@ -438,8 +460,7 @@ impl<B: NetworkBehaviour> Node<B> {
438460
.with_dns()
439461
.map_err(P2PError::failed_to_build_swarm)?
440462
.with_behaviour(|key| {
441-
let builder =
442-
PlutoBehaviourBuilder::default().with_p2p_context(p2p_context.clone());
463+
let builder = PlutoBehaviourBuilder::new(p2p_context.clone());
443464
behaviour_fn(builder, key).build(key)
444465
})
445466
.map_err(P2PError::failed_to_build_swarm)?

0 commit comments

Comments
 (0)