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

Comments

bridge: add all PeerIds of an AuthorityId to the gossip topology#4621

Closed
ordian wants to merge 2 commits intomasterfrom
ao-network-bridge-fix
Closed

bridge: add all PeerIds of an AuthorityId to the gossip topology#4621
ordian wants to merge 2 commits intomasterfrom
ao-network-bridge-fix

Conversation

@ordian
Copy link

@ordian ordian commented Dec 28, 2021

Fixes #4615

@ordian ordian added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Dec 28, 2021
@ordian ordian requested review from bkchr and eskimor December 28, 2021 10:02
NetworkService::add_known_address(&*self, peer_id.clone(), multiaddr);
found_peer_id = Some(peer_id);
}
found_peer_id
Copy link
Author

Choose a reason for hiding this comment

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

hmm, looking at this code again, req/resp still assumes one peer_id per authorityid?

@ordian
Copy link
Author

ordian commented Dec 28, 2021

Now that @eskimor reminded me of paritytech/substrate#10259 (comment), I'm convinced we shouldn't do this and instead ensure one PeerId per AuthorityId via type-system maybe.

@ordian ordian closed this Dec 28, 2021
@ordian ordian deleted the ao-network-bridge-fix branch December 28, 2021 10:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gossip: use all PeerIds per AuthorityId

2 participants