search

CityOfZion / neon-js

Javascript libraries that allow the applications to interact with NEO blockchain
https://docs.coz.io/neo3/neon-js/index.html
MIT License
184 stars 166 forks source link

Potential risk of frequent spamming of one/few nodes #270

Closed f27d closed 6 years ago

f27d commented 6 years ago

Really like the implementation of this!

As far as the code works, it loops through the nodes and checks their blockheight, looking for ones which are the highest or 1 block behind.

However, if the last node in the loop is the highest, then it overrides any of the nodes that were in the list.

So instead you just get one node rather than some options to ping race.

Some examples: If the order of the nodes examined (referenced by blockheight) is: 4, 5, 4, 6 Then only the last node would be eligible. If the order was:

4, 6, 4, 5

Then both 2nd and last nodes would be eligible.

https://github.com/CityOfZion/neon-js/blob/c6a169a82a4d037e00dccd424f53cdc818d6b3ae/src/api/neonDB.js#L90

I guess from what I can see there is a lot of time when one node is ahead of the others for a 10% of the time and so chance of this happening above is 10% * 1/# Nodes (Nodes returned from the initial /nodes api call).

snowypowers commented 6 years ago

The logic now does a sort and slice of the array instead of iterate and storing an external array so it should be cleaner to read and avoids this bug. Thanks for the issue!