*: Rework reporting of invalid and wrong PeerIds #2441
Conversation
Previously, the negotiated PeerId was included in the swarm event and inject_dial_failure’s arguments while the expected one was absent. This patch adds the negotiated PeerId to the DialError and includes the expected one in the notifications.
Co-authored-by: Max Inden <mail@max-inden.de>
also add changelog entries
|
Thanks a lot for pushing this over the line, @mxinden! |
There was a problem hiding this comment.
In general I think this pr broke something. We have this check that prints a warning when we connect to a bootnode and that provides a different than expected peer id. Any idea what could be broken. See the following issue: paritytech/substrate#11709
| if self.local_id == obtained_peer_id { | ||
| Err(PendingConnectionError::WrongPeerId { | ||
| obtained: obtained_peer_id, | ||
| endpoint: endpoint.clone(), | ||
| }) |
There was a problem hiding this comment.
You're connecting to yourself, which isn't allowed. It's true that the error is unclear, but this was an InvalidPeerId error before this PR and in general has been an error for a very long time now, so I don't see how this specific change could have broken anything.
There was a problem hiding this comment.
Yeah, I also looked over this changeset and didn't find anything that could cause the issue we are seeing. However, I think that after the corresponding libp2p update in Substrate this error started to appear. There is also some internal parity devops issue about the Kusama bootnodes reporting a wrong peerid, but devops assured to me multiple times that all keys are correct. So yeah, I'm assuming now that maybe something changed it libp2p 🤷
Replaces #2428.