You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Nonces within edges are used for nodes to determine the status of the communication between a pair of nodes. The higher the nonce, the more recent the information is. Nodes should update nonce +2 on each creation, so there is no problem with overflow using u64, but there was an issue that nodes were accepting higher nonce without checking edge with previous nonce existed.
The fix is just not accepting edge proposals with higher nonce, honest nodes will never try to send higher nonce than expected since the way syncing edges is working today new nodes joining the network, even if they loose the information about old connections they will learn nonce immediately after connecting to other nodes via RoutingTable Syncing
Test plan
pytest/tests/sanity/controlled_edge_nonce.py should pass
Are we certain it will not cause issues?
An alternative would be to allow nonces to grow no more than 1000 at a time, which still renders overflows impossible, but will handle gracefully cases when two nodes fail out of sync wrt nonces.
Are we certain it will not cause issues?
An alternative would be to allow nonces to grow no more than 1000 at a time, which still renders overflows impossible, but will handle gracefully cases when two nodes fail out of sync wrt nonces.
It shouldn't be a problem, since nodes will be out of sync very short amount of time ~HIGHEST_LATENCY between pair of nodes and after that they should be able to connect, but anyway I like your proposal, will use 1000 instead and then there shouldn't be any problems between honest nodes ever.
On Hanndshake peers checked to larger nonce
than previously known nonce to them, but they were not
checking and controlling larger nonce, hence accepting
every edge as long as the nonce was increasing. While
this is not fundamentally wrong it can be abused,
provoking overflow and eventually crashing the node.
Test plan
=========
This behavior was created in
- tests/spec/network/controlled_edge_nonce
Now this test should pass
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nonces within edges are used for nodes to determine the status of the communication between a pair of nodes. The higher the nonce, the more recent the information is. Nodes should update nonce +2 on each creation, so there is no problem with overflow using u64, but there was an issue that nodes were accepting higher nonce without checking edge with previous nonce existed.
The fix is just not accepting edge proposals with higher nonce, honest nodes will never try to send higher nonce than expected since the way syncing edges is working today new nodes joining the network, even if they loose the information about old connections they will learn nonce immediately after connecting to other nodes via RoutingTable Syncing
Test plan
pytest/tests/sanity/controlled_edge_nonce.py should pass