feat(swarm): introduce ConnectionId::new_unchecked constructor

In earlier iterations of the design for generic connection management, we removed the `ConnectionId::new` constructor because it would have allowed users to create `ConnectionId`s that are already taken, thus breaking invariants that `NetworkBehaviour`s rely on. Later, we incorporated the creation of `ConnectionId` in `DialOpts` which mitigates this risk altogether.

Thus, it is reasonably safe to introduce a public, non-deprecated constructor for `ConnectionId` that can be used for tests.

Related https://github.com/libp2p/rust-libp2p/pull/3327#issuecomment-1469870307.

Pull-Request: #3652.
This commit is contained in:
Thomas Eizinger
2023-03-21 20:28:46 +01:00
committed by GitHub
parent ab9555c8b6
commit 3fa10be0d5
4 changed files with 30 additions and 26 deletions

View File

@ -222,17 +222,19 @@ where
}
};
#[allow(deprecated)]
gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
peer_id: peer,
connection_id: ConnectionId::DUMMY,
connection_id: ConnectionId::new_unchecked(0),
endpoint: &endpoint,
failed_addresses: &[],
other_established: 0, // first connection
}));
if let Some(kind) = kind {
#[allow(deprecated)]
gs.on_connection_handler_event(peer, ConnectionId::DUMMY, HandlerEvent::PeerKind(kind));
gs.on_connection_handler_event(
peer,
ConnectionId::new_unchecked(0),
HandlerEvent::PeerKind(kind),
);
}
if explicit {
gs.add_explicit_peer(&peer);
@ -582,10 +584,9 @@ fn test_join() {
for _ in 0..3 {
let random_peer = PeerId::random();
// inform the behaviour of a new peer
#[allow(deprecated)]
gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
peer_id: random_peer,
connection_id: ConnectionId::DUMMY,
connection_id: ConnectionId::new_unchecked(0),
endpoint: &ConnectedPoint::Dialer {
address: "/ip4/127.0.0.1".parse::<Multiaddr>().unwrap(),
role_override: Endpoint::Dialer,
@ -965,10 +966,7 @@ fn test_get_random_peers() {
*p,
PeerConnections {
kind: PeerKind::Gossipsubv1_1,
connections: vec![
#[allow(deprecated)]
ConnectionId::DUMMY,
],
connections: vec![ConnectionId::new_unchecked(0)],
},
)
})
@ -2998,8 +2996,7 @@ fn test_ignore_rpc_from_peers_below_graylist_threshold() {
//receive from p1
gs.on_connection_handler_event(
p1,
#[allow(deprecated)]
ConnectionId::DUMMY,
ConnectionId::new_unchecked(0),
HandlerEvent::Message {
rpc: Rpc {
messages: vec![raw_message1],
@ -3025,8 +3022,7 @@ fn test_ignore_rpc_from_peers_below_graylist_threshold() {
//receive from p2
gs.on_connection_handler_event(
p2,
#[allow(deprecated)]
ConnectionId::DUMMY,
ConnectionId::new_unchecked(0),
HandlerEvent::Message {
rpc: Rpc {
messages: vec![raw_message3],
@ -3632,8 +3628,7 @@ fn test_scoring_p4_invalid_signature() {
gs.on_connection_handler_event(
peers[0],
#[allow(deprecated)]
ConnectionId::DUMMY,
ConnectionId::new_unchecked(0),
HandlerEvent::Message {
rpc: Rpc {
messages: vec![],
@ -4114,11 +4109,10 @@ fn test_scoring_p6() {
}
//add additional connection for 3 others with addr
#[allow(deprecated)]
for id in others.iter().take(3) {
gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
peer_id: *id,
connection_id: ConnectionId::DUMMY,
connection_id: ConnectionId::new_unchecked(0),
endpoint: &ConnectedPoint::Dialer {
address: addr.clone(),
role_override: Endpoint::Dialer,
@ -4137,10 +4131,9 @@ fn test_scoring_p6() {
//add additional connection for 3 of the peers to addr2
for peer in peers.iter().take(3) {
#[allow(deprecated)]
gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
peer_id: *peer,
connection_id: ConnectionId::DUMMY,
connection_id: ConnectionId::new_unchecked(0),
endpoint: &ConnectedPoint::Dialer {
address: addr2.clone(),
role_override: Endpoint::Dialer,
@ -4168,10 +4161,9 @@ fn test_scoring_p6() {
);
//two times same ip doesn't count twice
#[allow(deprecated)]
gs.on_swarm_event(FromSwarm::ConnectionEstablished(ConnectionEstablished {
peer_id: peers[0],
connection_id: ConnectionId::DUMMY,
connection_id: ConnectionId::new_unchecked(0),
endpoint: &ConnectedPoint::Dialer {
address: addr,
role_override: Endpoint::Dialer,
@ -5179,8 +5171,7 @@ fn test_subscribe_and_graft_with_negative_score() {
let (mut gs2, _, _) = inject_nodes1().create_network();
#[allow(deprecated)]
let connection_id = ConnectionId::DUMMY;
let connection_id = ConnectionId::new_unchecked(0);
let topic = Topic::new("test");

View File

@ -1295,8 +1295,7 @@ fn network_behaviour_on_address_change() {
let local_peer_id = PeerId::random();
let remote_peer_id = PeerId::random();
#[allow(deprecated)]
let connection_id = ConnectionId::DUMMY;
let connection_id = ConnectionId::new_unchecked(0);
let old_address: Multiaddr = Protocol::Memory(1).into();
let new_address: Multiaddr = Protocol::Memory(2).into();

View File

@ -3,7 +3,11 @@
- Deprecate `ConnectionLimits` in favor of `libp2p::connection_limits`.
See [PR 3386].
- Introduce `ConnectionId::new_unchecked` to allow for more sophisticated, manual tests of `NetworkBehaviour`.
See [PR 3652].
[PR 3386]: https://github.com/libp2p/rust-libp2p/pull/3386
[PR 3652]: https://github.com/libp2p/rust-libp2p/pull/3652
# 0.42.0

View File

@ -67,6 +67,16 @@ impl ConnectionId {
)]
pub const DUMMY: ConnectionId = ConnectionId(0);
/// Creates an _unchecked_ [`ConnectionId`].
///
/// [`Swarm`](crate::Swarm) enforces that [`ConnectionId`]s are unique and not reused.
/// This constructor does not, hence the _unchecked_.
///
/// It is primarily meant for allowing manual tests of [`NetworkBehaviour`](crate::NetworkBehaviour)s.
pub fn new_unchecked(id: usize) -> Self {
Self(id)
}
/// Returns the next available [`ConnectionId`].
pub(crate) fn next() -> Self {
Self(NEXT_CONNECTION_ID.fetch_add(1, Ordering::SeqCst))