Sort peers & fix bug with filtered peer status #354
Conversation
|
Removing the filter as it's a copy of another (does it twice) |
alexbarnsley
changed the title
|
I believe I'm done making changes now. There was a bug where it filtered down peers based on status, but it didn't use this filtered down list when making the initial connection to a peer (on first load of the desktop client). Also now sorting peers based on Height DESC, Delay ASC |
| })); | ||
| findGoodPeer(response.peers, 0); | ||
| getFromPeer('/api/peers/version').then(function(versionResponse) { | ||
| let peers = response.peers.filter(function(peer) { |
There was a problem hiding this comment.
Does there need to be validation for response.success like the above api call here?
There was a problem hiding this comment.
Not a bad shout. It shouldn't be necessary but will add it anyway and will let it fallback on the stored peers if it fails 👍
There was a problem hiding this comment.
Pushed the change for you to take a look 👀
|
I think getting the same version from the current peer does not make sense because if it is using an earlier version the user will get stuck in that version |
|
@luciorubeens would that be possible without manually changing the networks? It all starts from the initial network which would be the master wouldn't it?: Maybe I've misunderstood, I though it connected to the master network first to get the peers. I'll double check when I'm at a machine. I'm happy to take it out though if you think it will be an issue! |
|
yeah, but suppose we updated the ark-node, and this default peer has not updated yet.. I'm just making sure that we don't have to manually update this. I'll merge but I think we can improve a bit more.. +5! 👍🏻 |
|
That's fair enough @luciorubeens 😁 I'll have a think about whether there's a better way to check version numbers. Perhaps any matching version or greater would work? That way you're not stuck on an old version. Does that sound like an option? |
* fix: button margin on alert message * fix: rounded on top of menu (wallet show page) * fix: copy to clipboard padding (wallet new page)
This is to improve the initial connection to the network. More recently, there's been a lot of connect/disconnect activity. It now looks for the best peers based on highest Block Height, and lowest Delay.
@luciorubeens I couldn't use orderBy - it just didn't work (silently failing). Instead, I did this. I also didn't store in the serviceProvider as suggested, so i check on the first index of getGoodPeer and sort then, once.
Feedback welcome!