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.

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

Improve DiversifyPeersBySubnetTask #1898

Merged
merged 1 commit into from
Dec 21, 2020

Conversation

twoeths
Copy link
Contributor

@twoeths twoeths commented Dec 18, 2020

resolves #1896
Improve DiversifyPeersBySubnetTask by saving calls to network.getPeers()

  • get connected peers from the beginning and use it through out apis
  • searchSubnetPeers() now works with an array of subnets instead of 1 subnet in order to save network.getPeers() call

@github-actions github-actions bot added Api scope-networking All issues related to networking, gossip, and libp2p. labels Dec 18, 2020
@codeclimate
Copy link

codeclimate bot commented Dec 18, 2020

Code Climate has analyzed commit aaeda6e and detected 0 issues on this pull request.

View more on Code Climate.

@twoeths twoeths changed the title DiversifyPeersBySubnetTask: save calls to network.getPeers() Improve DiversifyPeersBySubnetTask Dec 18, 2020
@twoeths twoeths marked this pull request as ready for review December 18, 2020 10:25
@@ -50,8 +61,8 @@ export async function handlePeerMetadataSequence(
/**
* Find subnets that we don't have at least 1 connected peer.
*/
export function findMissingSubnets(network: INetwork): number[] {
const peers = network.getPeers({connected: true}).map((peer) => peer.id);
export function findMissingSubnets(connectedPeers: LibP2p.Peer[] = [], network: INetwork): number[] {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use type PeerId for connectedPeers param here and below?
Also, connectedPeers shouldn't have default value of []

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I changed to PeerId[] for possible apis. Some still need to access the internal protocols so we have to go with Libp2p.Peer[].
also I removing the default [] from all of them

@twoeths twoeths force-pushed the tuyen/improve-DiversifyPeersBySubnetTask branch from 7ee14e2 to aaeda6e Compare December 18, 2020 23:29
@wemeetagain wemeetagain merged commit 87f5901 into master Dec 21, 2020
@wemeetagain wemeetagain deleted the tuyen/improve-DiversifyPeersBySubnetTask branch December 21, 2020 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope-networking All issues related to networking, gossip, and libp2p.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve DiversifyPeersBySubnetTask
2 participants