Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

chore: update libp2p to 0.52.1 #14429

Merged
merged 53 commits into from
Jul 25, 2023
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
3102dbd
update libp2p to 0.52.0
melekes Jun 21, 2023
54778bd
proto name now must implement `AsRef<str>`
melekes Jun 21, 2023
839641b
update libp2p version everywhere
melekes Jun 21, 2023
45ff813
ToSwarm, FromBehaviour, ToBehaviour
melekes Jun 21, 2023
12ba133
new NetworkBehaviour invariants
melekes Jun 22, 2023
fe656e8
replace `Vec<u8>` with `StreamProtocol`
melekes Jun 22, 2023
37e7408
rename ConnectionHandlerEvent::Custom to NotifyBehaviour
melekes Jun 22, 2023
d5b8198
remove DialError & ListenError invariants
melekes Jun 23, 2023
81f5cd0
use connection_limits::Behaviour
melekes Jun 23, 2023
4d068dd
impl `void::Void` for `BehaviourOut`
melekes Jun 27, 2023
3e54dd1
KademliaHandler no longer public
melekes Jun 27, 2023
076635b
fix StreamProtocol construction
melekes Jun 27, 2023
d1fd05c
update libp2p-identify to 0.2.0
melekes Jun 27, 2023
f01de8b
remove non-existing methods from PollParameters
melekes Jun 28, 2023
bea250f
`P2p` now contains `PeerId`, not `Multihash`
melekes Jun 28, 2023
405ebcb
use multihash-codetable crate
melekes Jun 28, 2023
394eba1
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jun 28, 2023
ac5cfd3
update Cargo.lock
melekes Jun 28, 2023
3d04207
reformat text
melekes Jun 28, 2023
5f511cc
comment out tests for now
melekes Jun 29, 2023
f141f03
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jun 29, 2023
6970c11
remove `.into()` from P2p
melekes Jun 29, 2023
49e0ba7
confirm observed addr manually
melekes Jul 4, 2023
477ebc1
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 4, 2023
dd4a718
remove SwarmEvent::Banned
melekes Jul 4, 2023
c4f0a9c
fix imports
melekes Jul 4, 2023
350902c
replace `libp2p` with smaller deps in network-gossip
melekes Jul 4, 2023
b67be46
bring back tests
melekes Jul 4, 2023
3c179f0
finish rewriting tests
melekes Jul 4, 2023
720a068
uncomment handler tests
melekes Jul 5, 2023
7ffa62a
Revert "uncomment handler tests"
melekes Jul 6, 2023
07f2db8
add a fixme
melekes Jul 6, 2023
0370270
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 7, 2023
b25efe5
update Cargo.lock
melekes Jul 7, 2023
343a86e
remove extra From
melekes Jul 7, 2023
965cc2d
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 10, 2023
3b1c8f2
make void uninhabited
melekes Jul 17, 2023
a506670
fix discovery test
melekes Jul 17, 2023
6c78a74
use autonat protocols
melekes Jul 17, 2023
09f1ab6
fix SyncNotificationsClogged invariant
melekes Jul 17, 2023
e22dc73
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 17, 2023
2b1377c
only set server mode manually in tests
melekes Jul 17, 2023
6aa6c8d
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 17, 2023
8538215
Merge remote-tracking branch 'origin/master' into anton/libp2p-0.52.0
Jul 17, 2023
a6e2e25
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 18, 2023
c45b465
address @dmitry-markin comments
melekes Jul 18, 2023
d464408
remove autonat
melekes Jul 18, 2023
8b9309d
removed unused var
melekes Jul 18, 2023
73df4d5
fix EOL
melekes Jul 18, 2023
c34f538
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 19, 2023
d44efb5
update smallvec and sha2
melekes Jul 19, 2023
ff76e2a
bump k256
melekes Jul 19, 2023
925cad0
Merge branch 'master' into anton/libp2p-0.52.0
melekes Jul 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,645 changes: 308 additions & 1,337 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -340,4 +340,4 @@ inherits = "release"
# https://doc.rust-lang.org/rustc/linker-plugin-lto.html
lto = "fat"
# https://doc.rust-lang.org/rustc/codegen-options/index.html#codegen-units
codegen-units = 1
codegen-units = 1
melekes marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions client/authority-discovery/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ codec = { package = "parity-scale-codec", version = "3.6.1", default-features =
futures = "0.3.21"
futures-timer = "3.0.1"
ip_network = "0.4.1"
libp2p = { version = "0.51.3", features = ["kad", "ed25519"] }
multihash = { version = "0.17.0", default-features = false, features = ["std", "sha2"] }
libp2p = { version = "0.52.1", features = ["kad", "ed25519"] }
multihash-codetable = { version = "0.1.0", features = ["sha2", "digest"] }
log = "0.4.17"
prost = "0.11"
rand = "0.8.5"
Expand Down
2 changes: 1 addition & 1 deletion client/authority-discovery/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ fn get_addresses_and_authority_id() {
let remote_addr = "/ip6/2001:db8:0:0:0:0:0:2/tcp/30333"
.parse::<Multiaddr>()
.unwrap()
.with(Protocol::P2p(remote_peer_id.into()));
.with(Protocol::P2p(remote_peer_id));

let test_api = Arc::new(TestApi { authorities: vec![] });

Expand Down
8 changes: 4 additions & 4 deletions client/authority-discovery/src/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ use futures::{channel::mpsc, future, stream::Fuse, FutureExt, Stream, StreamExt}
use addr_cache::AddrCache;
use codec::{Decode, Encode};
use ip_network::IpNetwork;
use libp2p::{core::multiaddr, identity::PublicKey, multihash::Multihash, Multiaddr, PeerId};
use multihash::{Code, MultihashDigest};
use libp2p::{core::multiaddr, identity::PublicKey, Multiaddr};
use multihash_codetable::{Code, MultihashDigest};

use log::{debug, error, log_enabled};
use prometheus_endpoint::{register, Counter, CounterVec, Gauge, Opts, U64};
Expand Down Expand Up @@ -304,7 +304,7 @@ where
}

fn addresses_to_publish(&self) -> impl Iterator<Item = Multiaddr> {
let peer_id: Multihash = self.network.local_peer_id().into();
let peer_id = self.network.local_peer_id();
let publish_non_global_ips = self.publish_non_global_ips;
self.network
.external_addresses()
Expand Down Expand Up @@ -529,7 +529,7 @@ where
.map_err(Error::ParsingMultiaddress)?;

let get_peer_id = |a: &Multiaddr| match a.iter().last() {
Some(multiaddr::Protocol::P2p(key)) => PeerId::from_multihash(key).ok(),
Some(multiaddr::Protocol::P2p(peer_id)) => Some(peer_id),
_ => None,
};

Expand Down
29 changes: 14 additions & 15 deletions client/authority-discovery/src/worker/addr_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ impl AddrCache {

fn peer_id_from_multiaddr(addr: &Multiaddr) -> Option<PeerId> {
addr.iter().last().and_then(|protocol| {
if let Protocol::P2p(multihash) = protocol {
PeerId::from_multihash(multihash).ok()
if let Protocol::P2p(peer_id) = protocol {
Some(peer_id)
} else {
None
}
Expand All @@ -178,7 +178,8 @@ fn addresses_to_peer_ids(addresses: &HashSet<Multiaddr>) -> HashSet<PeerId> {
mod tests {
use super::*;

use libp2p::multihash::{self, Multihash};
use libp2p::multihash::Multihash;
use multihash_codetable::Code;
use quickcheck::{Arbitrary, Gen, QuickCheck, TestResult};

use sp_authority_discovery::{AuthorityId, AuthorityPair};
Expand All @@ -200,14 +201,13 @@ mod tests {
impl Arbitrary for TestMultiaddr {
fn arbitrary(g: &mut Gen) -> Self {
let seed = (0..32).map(|_| u8::arbitrary(g)).collect::<Vec<_>>();
let peer_id = PeerId::from_multihash(
Multihash::wrap(multihash::Code::Sha2_256.into(), &seed).unwrap(),
)
.unwrap();
let peer_id =
PeerId::from_multihash(Multihash::wrap(Code::Sha2_256.into(), &seed).unwrap())
.unwrap();
melekes marked this conversation as resolved.
Show resolved Hide resolved
let multiaddr = "/ip6/2001:db8:0:0:0:0:0:2/tcp/30333"
.parse::<Multiaddr>()
.unwrap()
.with(Protocol::P2p(peer_id.into()));
.with(Protocol::P2p(peer_id));

TestMultiaddr(multiaddr)
}
Expand All @@ -219,18 +219,17 @@ mod tests {
impl Arbitrary for TestMultiaddrsSamePeerCombo {
fn arbitrary(g: &mut Gen) -> Self {
let seed = (0..32).map(|_| u8::arbitrary(g)).collect::<Vec<_>>();
let peer_id = PeerId::from_multihash(
Multihash::wrap(multihash::Code::Sha2_256.into(), &seed).unwrap(),
)
.unwrap();
let peer_id =
PeerId::from_multihash(Multihash::wrap(Code::Sha2_256.into(), &seed).unwrap())
.unwrap();
let multiaddr1 = "/ip6/2001:db8:0:0:0:0:0:2/tcp/30333"
.parse::<Multiaddr>()
.unwrap()
.with(Protocol::P2p(peer_id.into()));
.with(Protocol::P2p(peer_id));
let multiaddr2 = "/ip6/2002:db8:0:0:0:0:0:2/tcp/30133"
.parse::<Multiaddr>()
.unwrap()
.with(Protocol::P2p(peer_id.into()));
.with(Protocol::P2p(peer_id));
TestMultiaddrsSamePeerCombo(multiaddr1, multiaddr2)
}
}
Expand Down Expand Up @@ -367,7 +366,7 @@ mod tests {
let mut addr_cache = AddrCache::new();

let peer_id = PeerId::random();
let addr = Multiaddr::empty().with(Protocol::P2p(peer_id.into()));
let addr = Multiaddr::empty().with(Protocol::P2p(peer_id));

let authority_id0 = AuthorityPair::generate().0.public();
let authority_id1 = AuthorityPair::generate().0.public();
Expand Down
6 changes: 3 additions & 3 deletions client/authority-discovery/src/worker/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ fn dont_stop_polling_dht_event_stream_after_bogus_event() {
let peer_id = PeerId::random();
let address: Multiaddr = "/ip6/2001:db8:0:0:0:0:0:1/tcp/30333".parse().unwrap();

address.with(multiaddr::Protocol::P2p(peer_id.into()))
address.with(multiaddr::Protocol::P2p(peer_id))
};
let remote_key_store = MemoryKeystore::new();
let remote_public_key: AuthorityId = remote_key_store
Expand Down Expand Up @@ -526,7 +526,7 @@ impl DhtValueFoundTester {
let address: Multiaddr =
format!("/ip6/2001:db8:0:0:0:0:0:{:x}/tcp/30333", idx).parse().unwrap();

address.with(multiaddr::Protocol::P2p(peer_id.into()))
address.with(multiaddr::Protocol::P2p(peer_id))
}

fn process_value_found(
Expand Down Expand Up @@ -749,7 +749,7 @@ fn lookup_throttling() {
let peer_id = PeerId::random();
let address: Multiaddr = "/ip6/2001:db8:0:0:0:0:0:1/tcp/30333".parse().unwrap();

address.with(multiaddr::Protocol::P2p(peer_id.into()))
address.with(multiaddr::Protocol::P2p(peer_id))
};
let remote_key_store = MemoryKeystore::new();
let remote_public_keys: Vec<AuthorityId> = (0..20)
Expand Down
2 changes: 1 addition & 1 deletion client/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ chrono = "0.4.10"
clap = { version = "4.2.5", features = ["derive", "string"] }
fdlimit = "0.2.1"
futures = "0.3.21"
libp2p-identity = { version = "0.1.2", features = ["peerid", "ed25519"]}
libp2p-identity = { version = "0.2.0", features = ["peerid", "ed25519"]}
log = "0.4.17"
names = { version = "0.13.0", default-features = false }
parity-scale-codec = "3.6.1"
Expand Down
2 changes: 1 addition & 1 deletion client/consensus/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"]
async-trait = "0.1.57"
futures = { version = "0.3.21", features = ["thread-pool"] }
futures-timer = "3.0.1"
libp2p-identity = { version = "0.1.2", features = ["peerid", "ed25519"] }
libp2p-identity = { version = "0.2.0", features = ["peerid", "ed25519"] }
log = "0.4.17"
mockall = "0.11.3"
parking_lot = "0.12.1"
Expand Down
3 changes: 2 additions & 1 deletion client/network-gossip/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ targets = ["x86_64-unknown-linux-gnu"]
ahash = "0.8.2"
futures = "0.3.21"
futures-timer = "3.0.1"
libp2p = "0.51.3"
libp2p-identity = { version = "0.2.0", features = ["peerid", "ed25519"]}
multiaddr = "0.18.0"
log = "0.4.17"
schnellru = "0.2.1"
tracing = "0.1.29"
Expand Down
5 changes: 3 additions & 2 deletions client/network-gossip/src/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use futures::{
channel::mpsc::{channel, Receiver, Sender},
prelude::*,
};
use libp2p::PeerId;
use libp2p_identity::PeerId;
use log::trace;
use prometheus_endpoint::Registry;
use sp_runtime::traits::Block as BlockT;
Expand Down Expand Up @@ -330,12 +330,13 @@ impl<B: BlockT> futures::future::FusedFuture for GossipEngine<B> {
#[cfg(test)]
mod tests {
use super::*;
use crate::{multiaddr::Multiaddr, ValidationResult, ValidatorContext};
use crate::{ValidationResult, ValidatorContext};
use futures::{
channel::mpsc::{unbounded, UnboundedSender},
executor::{block_on, block_on_stream},
future::poll_fn,
};
use multiaddr::Multiaddr;
use quickcheck::{Arbitrary, Gen, QuickCheck};
use sc_network::{
config::MultiaddrWithPeerId, NetworkBlock, NetworkEventStream, NetworkNotification,
Expand Down
6 changes: 3 additions & 3 deletions client/network-gossip/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ pub use self::{
validator::{DiscardAll, MessageIntent, ValidationResult, Validator, ValidatorContext},
};

use libp2p::{multiaddr, PeerId};
use libp2p_identity::PeerId;
use multiaddr::{Multiaddr, Protocol};
use sc_network::{
types::ProtocolName, NetworkBlock, NetworkEventStream, NetworkNotification, NetworkPeers,
};
Expand All @@ -82,8 +83,7 @@ mod validator;
/// Abstraction over a network.
pub trait Network<B: BlockT>: NetworkPeers + NetworkEventStream + NetworkNotification {
fn add_set_reserved(&self, who: PeerId, protocol: ProtocolName) {
let addr =
iter::once(multiaddr::Protocol::P2p(who.into())).collect::<multiaddr::Multiaddr>();
let addr = Multiaddr::empty().with(Protocol::P2p(who));
let result = self.add_peers_to_reserved_set(protocol, iter::once(addr).collect());
if let Err(err) = result {
log::error!(target: "gossip", "add_set_reserved failed: {}", err);
Expand Down
4 changes: 2 additions & 2 deletions client/network-gossip/src/state_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use crate::{MessageIntent, Network, ValidationResult, Validator, ValidatorContext};

use ahash::AHashSet;
use libp2p::PeerId;
use libp2p_identity::PeerId;
use schnellru::{ByLength, LruMap};

use prometheus_endpoint::{register, Counter, PrometheusError, Registry, U64};
Expand Down Expand Up @@ -521,8 +521,8 @@ impl Metrics {
#[cfg(test)]
mod tests {
use super::*;
use crate::multiaddr::Multiaddr;
use futures::prelude::*;
use multiaddr::Multiaddr;
use sc_network::{
config::MultiaddrWithPeerId, event::Event, NetworkBlock, NetworkEventStream,
NetworkNotification, NetworkPeers, NotificationSenderError,
Expand Down
2 changes: 1 addition & 1 deletion client/network-gossip/src/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use libp2p::PeerId;
use libp2p_identity::PeerId;
use sc_network_common::role::ObservedRole;
use sp_runtime::traits::Block as BlockT;

Expand Down
6 changes: 4 additions & 2 deletions client/network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ fnv = "1.0.6"
futures = "0.3.21"
futures-timer = "3.0.2"
ip_network = "0.4.1"
libp2p = { version = "0.51.3", features = ["dns", "identify", "kad", "macros", "mdns", "noise", "ping", "tcp", "tokio", "yamux", "websocket", "request-response"] }
libp2p = { version = "0.52.1", features = ["autonat", "dns", "identify", "kad", "macros", "mdns", "noise", "ping", "tcp", "tokio", "yamux", "websocket", "request-response"] }
libp2p-kad = { version = "0.44.2" }
linked_hash_set = "0.1.3"
log = "0.4.17"
mockall = "0.11.3"
Expand All @@ -38,6 +39,7 @@ serde_json = "1.0.85"
smallvec = "1.8.0"
thiserror = "1.0"
unsigned-varint = { version = "0.7.1", features = ["futures", "asynchronous_codec"] }
void = "1"
zeroize = "1.4.3"
prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0.10.0-dev", path = "../../utils/prometheus" }
sc-client-api = { version = "4.0.0-dev", path = "../api" }
Expand All @@ -52,7 +54,7 @@ wasm-timer = "0.2"
[dev-dependencies]
assert_matches = "1.3"
mockall = "0.11.3"
multistream-select = "0.12.1"
multistream-select = "0.13.0"
rand = "0.8.5"
tempfile = "3.1.0"
tokio = { version = "1.22.0", features = ["macros"] }
Expand Down
2 changes: 1 addition & 1 deletion client/network/bitswap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ prost-build = "0.11"
async-channel = "1.8.0"
cid = "0.9.0"
futures = "0.3.21"
libp2p-identity = { version = "0.1.2", features = ["peerid"] }
libp2p-identity = { version = "0.2.0", features = ["peerid"] }
log = "0.4.17"
prost = "0.11"
thiserror = "1.0"
Expand Down
2 changes: 1 addition & 1 deletion client/network/common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ codec = { package = "parity-scale-codec", version = "3.6.1", features = [
"derive",
] }
futures = "0.3.21"
libp2p-identity = { version = "0.1.2", features = ["peerid"] }
libp2p-identity = { version = "0.2.0", features = ["peerid"] }
sc-consensus = { version = "0.10.0-dev", path = "../../consensus/common" }
sp-consensus = { version = "0.10.0-dev", path = "../../../primitives/consensus/common" }
sp-consensus-grandpa = { version = "4.0.0-dev", path = "../../../primitives/consensus/grandpa" }
Expand Down
2 changes: 1 addition & 1 deletion client/network/light/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ codec = { package = "parity-scale-codec", version = "3.6.1", features = [
"derive",
] }
futures = "0.3.21"
libp2p-identity = { version = "0.1.2", features = ["peerid"] }
libp2p-identity = { version = "0.2.0", features = ["peerid"] }
log = "0.4.16"
prost = "0.11"
sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" }
Expand Down
31 changes: 27 additions & 4 deletions client/network/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ use crate::{
use bytes::Bytes;
use futures::channel::oneshot;
use libp2p::{
core::Multiaddr, identify::Info as IdentifyInfo, identity::PublicKey, kad::RecordKey,
swarm::NetworkBehaviour, PeerId,
autonat, connection_limits::ConnectionLimits, core::Multiaddr, identify::Info as IdentifyInfo,
identity::PublicKey, kad::RecordKey, swarm::NetworkBehaviour, PeerId,
};

use sc_network_common::role::{ObservedRole, Roles};
Expand All @@ -42,7 +42,7 @@ pub use crate::request_responses::{InboundFailure, OutboundFailure, RequestId, R

/// General behaviour of the network. Combines all protocols together.
#[derive(NetworkBehaviour)]
#[behaviour(out_event = "BehaviourOut")]
#[behaviour(to_swarm = "BehaviourOut")]
pub struct Behaviour<B: BlockT> {
/// All the substrate-specific protocols.
substrate: Protocol<B>,
Expand All @@ -51,8 +51,12 @@ pub struct Behaviour<B: BlockT> {
peer_info: peer_info::PeerInfoBehaviour,
/// Discovers nodes of the network.
discovery: DiscoveryBehaviour,
/// Connection limits.
connection_limits: libp2p::connection_limits::Behaviour,
Comment on lines +54 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀

/// Generic request-response protocols.
request_responses: request_responses::RequestResponsesBehaviour,
// AutoNAT.
auto_nat: autonat::Behaviour,
Copy link
Contributor

@tomaka tomaka Jul 18, 2023

Choose a reason for hiding this comment

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

Is adding this line intended? It modifies the networking protocols of Substrate/Polkadot, and is a pretty substantial change for what looks a dependency version bump PR.
If intended, this should IMO go through an RFC: https://github.com/polkadot-fellows/RFCs/pulls

Copy link
Contributor

@tomaka tomaka Jul 18, 2023

Choose a reason for hiding this comment

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

I find it a bit worrisome that modifying the networking protocol would have successfully passed review without raising alarms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you @melekes elaborate on why this is added in the first place?

Copy link
Contributor Author

@melekes melekes Jul 18, 2023

Choose a reason for hiding this comment

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

Thanks 🙏 Will create an RFC today. I've reverted the code to manually confirming the observed address (although it's not safe, I believe)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you @melekes elaborate on why this is added in the first place?

https://github.com/libp2p/rust-libp2p/blob/master/protocols/identify/CHANGELOG.md#0430

Observed addresses (aka. external address candidates) of the local node, reported by a remote node via libp2p-identify, are no longer automatically considered confirmed external addresses, in other words they are no longer trusted by default. Instead users need to confirm the reported observed address either manually, or by using libp2p-autonat. In trusted environments users can simply extract observed addresses from a libp2p-identify::Event::Received { info: libp2p_identify::Info { observed_addr }} and confirm them via Swarm::add_external_address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Kademlia switches its mode to server once it receives a confirmed external address. Of course, we can set mode to server ourselves (via set_mode like it is done in tests), but I don't think it's desirable.
  2. I've moved logging of new external addresses (Discovered new external address for our node) to FromSwarm::ExternalAddrConfirmed case, so we don't log all the candidates.
  3. AutoNAT makes sure external address are publicly reachable (currently we're confirming all external addresses d464408)

Copy link
Contributor

@tomaka tomaka Jul 18, 2023

Choose a reason for hiding this comment

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

(although it's not safe, I believe)

It's how it works today.
There's no safety aspect here. We must already consider every peer as potentially malicious. Increasing the chance that peers report accurate information should neither increase not decrease safety. If it does, it means that we don't actually consider every peer as potentially malicious and this should be fixed.

I'm not necessarily against the principle of AutoNAT. What I'm completely against is sneaking changes to the networking protocol in minor version bumps, and I'm quite annoyed about rust-libp2p if they do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note from the rust-libp2p side:

  • running libp2p-autonat is not required.
  • libp2p-autonat is not automatically enabled.
  • manually confirming the observed addresses is just fine. If I recall correctly there was a command line flag in Substrate where a user could even specify the external address.

Long story short, Substrate should be able to upgrade to libp2p v0.52.0 without the introduction of libp2p-autonat.

}

/// Event generated by `Behaviour`.
Expand Down Expand Up @@ -158,6 +162,9 @@ pub enum BehaviourOut {
/// request duration.
Dht(DhtEvent, Duration),

/// Event generated by the AutoNAT.
AutoNAT(autonat::Event),

/// Ignored event generated by lower layers.
None,
}
Expand All @@ -171,15 +178,19 @@ impl<B: BlockT> Behaviour<B> {
disco_config: DiscoveryConfig,
request_response_protocols: Vec<ProtocolConfig>,
peerset: PeersetHandle,
connection_limits: ConnectionLimits,
) -> Result<Self, request_responses::RegisterError> {
let local_peer_id = local_public_key.to_peer_id();
Ok(Self {
substrate,
peer_info: peer_info::PeerInfoBehaviour::new(user_agent, local_public_key),
discovery: disco_config.finish(),
connection_limits: libp2p::connection_limits::Behaviour::new(connection_limits),
request_responses: request_responses::RequestResponsesBehaviour::new(
request_response_protocols.into_iter(),
peerset,
)?,
auto_nat: autonat::Behaviour::new(local_peer_id, autonat::Config::default()),
})
}

Expand Down Expand Up @@ -247,7 +258,7 @@ impl<B: BlockT> Behaviour<B> {
pub fn add_self_reported_address_to_dht(
&mut self,
peer_id: &PeerId,
supported_protocols: &[impl AsRef<[u8]>],
supported_protocols: &[impl AsRef<str>],
addr: Multiaddr,
) {
self.discovery.add_self_reported_address(peer_id, supported_protocols, addr);
Expand Down Expand Up @@ -351,3 +362,15 @@ impl From<DiscoveryOut> for BehaviourOut {
}
}
}

impl From<autonat::Event> for BehaviourOut {
fn from(event: autonat::Event) -> Self {
BehaviourOut::AutoNAT(event)
}
}

impl From<void::Void> for BehaviourOut {
fn from(e: void::Void) -> Self {
void::unreachable(e)
}
}
Loading