Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(identify)!: report what we pushed to a remote peer #4335

Closed
wants to merge 5 commits into from
Closed

feat(identify)!: report what we pushed to a remote peer #4335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a7ae23c
added behavior event when local protocols change
dhuseby Aug 17, 2023
4a0012b
feat!: adding Info to pushed event to detect when local protocols change
dhuseby Aug 18, 2023
d096076
bumped crate version and started CHANGELIG entry
dhuseby Aug 18, 2023
cc02b21
Update protocols/identify/CHANGELOG.md
dhuseby Aug 18, 2023
a657b7c
Merge branch 'master' into report-local-protocols-changed
thomaseizinger Sep 13, 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
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ libp2p-deflate = { version = "0.40.0", path = "transports/deflate" }
libp2p-dns = { version = "0.40.0", path = "transports/dns" }
libp2p-floodsub = { version = "0.43.0", path = "protocols/floodsub" }
libp2p-gossipsub = { version = "0.45.1", path = "protocols/gossipsub" }
libp2p-identify = { version = "0.43.0", path = "protocols/identify" }
libp2p-identify = { version = "0.44.0", path = "protocols/identify" }
libp2p-identity = { version = "0.2.3" }
libp2p-kad = { version = "0.44.4", path = "protocols/kad" }
libp2p-mdns = { version = "0.44.0", path = "protocols/mdns" }
Expand Down
6 changes: 5 additions & 1 deletion protocols/identify/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
## 0.43.0
## 0.44.0 - unreleased

- Added `Info` to the `libp2p-identify::Event::Pushed` to report what was pushed.
dhuseby marked this conversation as resolved.
Show resolved Hide resolved

## 0.43.0

- 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`.
Expand Down
2 changes: 1 addition & 1 deletion protocols/identify/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name = "libp2p-identify"
edition = "2021"
rust-version = { workspace = true }
description = "Nodes identifcation protocol for libp2p"
version = "0.43.0"
version = "0.44.0"
authors = ["Parity Technologies <admin@parity.io>"]
license = "MIT"
repository = "https://github.com/libp2p/rust-libp2p"
Expand Down
6 changes: 4 additions & 2 deletions protocols/identify/src/behaviour.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,9 @@ impl NetworkBehaviour for Behaviour {
self.events
.push_back(ToSwarm::GenerateEvent(Event::Sent { peer_id }));
}
handler::Event::IdentificationPushed => {
handler::Event::IdentificationPushed(info) => {
self.events
.push_back(ToSwarm::GenerateEvent(Event::Pushed { peer_id }));
.push_back(ToSwarm::GenerateEvent(Event::Pushed { peer_id, info }));
}
handler::Event::IdentificationError(error) => {
self.events
Expand Down Expand Up @@ -431,6 +431,8 @@ pub enum Event {
Pushed {
/// The peer that the information has been sent to.
peer_id: PeerId,
/// The information pushed to the peer.
info: Info,
dhuseby marked this conversation as resolved.
Show resolved Hide resolved
},
/// Error while attempting to identify the remote.
Error {
Expand Down
11 changes: 7 additions & 4 deletions protocols/identify/src/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub enum Event {
/// We replied to an identification request from the remote.
Identification,
/// We actively pushed our identification information to the remote.
IdentificationPushed,
IdentificationPushed(Info),
/// Failed to identify the remote, or to reply to an identification request.
IdentificationError(StreamUpgradeError<UpgradeError>),
}
Expand Down Expand Up @@ -182,9 +182,12 @@ impl Handler {
remote_info,
)));
}
future::Either::Right(()) => self.events.push(ConnectionHandlerEvent::NotifyBehaviour(
Event::IdentificationPushed,
)),
future::Either::Right(()) => {
let local_info = self.build_info();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it guaranteed that this is actually the info we pushed? What if our state changed since we sent it to the other peer?

I'd prefer if we changed the OutboundUpgrade implementation such that it emits the Info that it sent over the wire so we can carry it forward here.

self.events.push(ConnectionHandlerEvent::NotifyBehaviour(
Event::IdentificationPushed(local_info),
));
}
}
}

Expand Down