Update selection mechanism to connect to peers - Closes #3346 #4022
Conversation
diego-G
force-pushed
the
3346-update_peer_selection_multiple
branch
from
1f8baed
to
b6d73a1
Compare
There was a problem hiding this comment.
I think these calculations need to be moved to peer_book, we can expose a combined list of new/triedPeer using this calculation using a function (something like peerBook.getPeersForSelection) in this way we don't need to pass around new/triedPeers logic to peer_pool, peer_selection, etc. The peer_selection function for connection can be used for more higher-level logic for selection like peers having the same modules or greater height, etc.
Maybe this PR should go after #4030 so that we have more clarity.
|
@ishantiw as discussed, let's work on this idea after we merge the current PR and we review yours. |
diego-G
force-pushed
the
3346-update_peer_selection_multiple
branch
from
2469918
to
0c71956
Compare
| } | ||
|
|
||
| return shuffledTriedPeers.splice(0, 1)[0]; | ||
| return shuffledTriedPeers.pop() as P2PPeerInfo; |
There was a problem hiding this comment.
This one could be undefined if the shuffledTriedPeers and shuffledNewPeers lengths are both 0.
There was a problem hiding this comment.
Or if shuffledNewPeers is 0 and the if (Math.random() < r) block is not executed for shuffledTriedPeers.
There was a problem hiding this comment.
Those two cases are covered above. Have a look! Also there are unit test double checking it.
| } | ||
|
|
||
| return shuffledTriedPeers.splice(0, 1)[0]; | ||
| return shuffledTriedPeers.pop() as P2PPeerInfo; |
There was a problem hiding this comment.
Or if shuffledNewPeers is 0 and the if (Math.random() < r) block is not executed for shuffledTriedPeers.
diego-G
requested review from
pablitovicente
and removed request for
mitsuaki-u
How to manually test it?
npm testReview checklist