Skip to content

Commit

Permalink
feat(peer_db)!: more accurate peer stats per address (#5142)
Browse files Browse the repository at this point in the history
Description
---

Removes a lot of duplicate stats and online/offline fields on peers. 

Motivation
---
Due to a recent bug, wallets would get a new address every time they restarted. There was some handling of peers with multiple addresses, but in general only the latest address signed by the peer was used. This prevents a node from being a bridge between tor and IP or even IP and DNS. This PR controversially (I'm sure) removes the signed address and instead allows a node to try an address to see if the correct public key is on the other end. If so, the peer can choose the best latency according to communicate with it.

IMO, this is no less secure, since the worst case is that a node can force another node to contact an address that is not correct, only to find out the node at the other end has a different public key, This already can be done by generating a new peer id, using the address and signing it with the new peer id (i.e a sybil address).

Previously a peer may have been banned for answering an address with the wrong public key, but this can also be manipulated. Now, only the address is marked as bad.

Also reworked the general sorting of peer addresses, although in most previous cases, there would only be one address. 

Also reworked the order in which the peer tries neighbours. In order to make it more predictable and prevent looping, the connectivity service will try to find it's neighbours in a strict distance away from it. Previously it took into account a last_attempted_at or last_seen_at, which I am not convinced was working.

You'll note that there are some tokio improvements as well.

During the testing, I noticed that the peer query was, and still is, extremely heavy. In  tokio console, queries took in the order of 10s to 100s. Moving this to a `block_in_place` frees up the tokio worker threads. In a future PR I will improve this even further by removing unnecessary calls.

Also:
- Removed some dead code OptionalBoundedExecutor and replaced the re-export of runtime from tokio
- Used standard tokio spawns for BoundedExecutor instead of holding a runtime reference.

BREAKING CHANGE: Note that because the Peer struct has changed so dramatically, a migration is not really pragmatic. The peer_db for wallets and base nodes **must** be deleted on before starting up
  • Loading branch information
stringhandler authored Mar 3, 2023
1 parent 7fc3265 commit fdad1c6
Show file tree
Hide file tree
Showing 175 changed files with 2,761 additions and 2,801 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,5 @@ clients/base_node_grpc_client/package-lock.json
clients/validator_node_grpc_client/package-lock.json
clients/wallet_grpc_client/package-lock.json
pie/
/integration_tests/.husky/_/husky.sh
/integration_tests/tests/temp
Loading

0 comments on commit fdad1c6

Please sign in to comment.