Purge addresses that fail to reach a peer #8843
Conversation
tomaka
added
A0-please_review
| @@ -287,12 +291,14 @@ impl DiscoveryBehaviour { | |||
| /// | |||
| /// If we didn't know this address before, also generates a `Discovered` event. | |||
| pub fn add_known_address(&mut self, peer_id: PeerId, addr: Multiaddr) { | |||
| if self.user_defined.iter().all(|(p, a)| *p != peer_id && *a != addr) { | |||
| let addrs_list = self.ephemeral_addresses.entry(peer_id).or_default(); | |||
| if !addrs_list.iter().any(|a| *a == addr) { | |||
There was a problem hiding this comment.
Why you don't check in permanent_addresses too?
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
|
Yeah, this was just waiting to be merged. |
stale
bot
removed
the
A5-stale
|
bot merge |
|
Waiting for commit status. |
|
Merge failed: Could not recover from: |
|
Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions. |
stale
bot
added
the
A5-stale
gilescope
removed
the
A5-stale
|
Yes |
|
@kpp could you merge master so that we can merge this pr? |
|
Yes. In an hour |
|
bot merge |
|
Waiting for commit status. |
|
ty @kpp |
|
Merge aborted: Checks failed for 9dba155 |
|
bot merge |
|
Waiting for commit status. |
|
Merge aborted: Checks failed for 9dba155 |
So far, the
add_known_addressmethod of the networking was meant to be used to inject addresses that are permanent. In other words, addresses for reserved nodes or bootnodes that the user has explicitly configured.However, Polkadot now uses this method to add the addresses reported by the authority-discovery system. In other words, addresses reported by the nodes themselves.
Before this PR, these addresses would never be purged. After this PR, addresses that fail to connect to someone will be removed and not tried again until they are re-added.
cc paritytech/polkadot-sdk#540
This PR is sub-optimal, so I don't think we should be closing paritytech/polkadot-sdk#540. I'm doing this change to try to quickly improve the situation with parachains.
Ideally, we would have a proper story for addresses management, including properly documenting the behaviour, and removing addresses for peers whose reputation drops to 0, but these are way bigger changes.