Skip to content

network: improve peer management #3911

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

network: improve peer management #3911

wants to merge 3 commits into from

Conversation

AnnaShaleva
Copy link
Member

@AnnaShaleva AnnaShaleva commented May 20, 2025 •
edited
Loading

Close #3914, close #3915.

@AnnaShaleva AnnaShaleva requested a review from roman-khimov as a code owner May 20, 2025 19:07
@AnnaShaleva AnnaShaleva added bug Something isn't working network P2P layer labels May 20, 2025
@AnnaShaleva AnnaShaleva marked this pull request as draft May 20, 2025 19:07
@codecov Codecov
Copy link

codecov bot commented May 20, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.86%. Comparing base (c39fa7a) to head (8ecc928).
Report is 13 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3911      +/-   ##
==========================================
- Coverage   82.58%   81.86%   -0.73%     
==========================================
  Files         342      345       +3     
  Lines       48370    48857     +487     
==========================================
+ Hits        39948    39995      +47     
- Misses       6771     7191     +420     
- Partials     1651     1671      +20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AnnaShaleva AnnaShaleva changed the title network: remove good peer from disconnected on registration network: improve discovery for custom network setup May 22, 2025
@AnnaShaleva AnnaShaleva removed bug Something isn't working network P2P layer labels May 22, 2025
@AnnaShaleva AnnaShaleva force-pushed the fix-disconnected branch 2 times, most recently from c588f5f to b1fd639 Compare May 22, 2025 11:55
@AnnaShaleva AnnaShaleva changed the title network: improve discovery for custom network setup network: improve peer management May 22, 2025
@AnnaShaleva
network: remove good peer from disconnected on registration
c6aa354 
Peer should not be marked as "connected" and "disconnected" at the same
time. Fixes the first part of #3914.

Reproduced and checked on custom privnet. Before:
```
anna@kiwi:~/Documents/GitProjects/nspcc-dev/neo-go$ curl -d '{ "jsonrpc": "2.0", "id": 1, "method": "getpeers", "params": ["0x398dbf659b3ea356845aafdc54227833550855e8956f5a9eae338c025dfa8a77"] }' http://localhost:30336 | json_pp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   862  100   729  100   133  65363  11925 --:--:-- --:--:-- --:--:-- 78363
{
   "id" : 1,
   "jsonrpc" : "2.0",
   "result" : {
      "bad" : [
         {
            "address" : "172.17.0.1",
            "port" : 50336
         },
         {
            "address" : "172.200.0.254",
            "port" : 47766
         },
         {
            "address" : "172.200.0.254",
            "port" : 50336
         },
         {
            "address" : "172.17.0.1",
            "port" : 50333
         }
      ],
      "connected" : [
         {
            "address" : "172.17.0.1",
            "port" : 50334,
            "useragent" : "/NEO-GO:0.109.2-pre-11-gc39f7f7f/"
         },
         {
            "address" : "172.17.0.1",
            "lastknownheight" : 1,
            "port" : 50335,
            "useragent" : "/NEO-GO:0.109.2-pre-11-gc39f7f7f/"
         },
         {
            "address" : "172.200.0.254",
            "port" : 50333,
            "useragent" : "/NEO-GO:0.109.2-pre-11-gc39f7f7f/"
         },
         {
            "address" : "172.200.0.254",
            "port" : 50334,
            "useragent" : "/NEO-GO:0.109.2-pre-11-gc39f7f7f/"
         },
         {
            "address" : "172.200.0.254",
            "port" : 50335,
            "useragent" : "/NEO-GO:0.109.2-pre-11-gc39f7f7f/"
         }
      ],
      "unconnected" : [
         {
            "address" : "172.17.0.1",
            "port" : 50335
         }
      ]
   }
}
```

After:
```
anna@kiwi:~/Documents/GitProjects/nspcc-dev/neo-go$ curl -d '{ "jsonrpc": "2.0", "id": 1, "method": "getpeers", "params": ["0x398dbf659b3ea356845aafdc54227833550855e8956f5a9eae338c025dfa8a77"] }' http://localhost:30336 | json_pp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   926  100   793  100   133   723k   121k --:--:-- --:--:-- --:--:--  904k
{
   "id" : 1,
   "jsonrpc" : "2.0",
   "result" : {
      "bad" : [
         {
            "address" : "172.200.0.254",
            "port" : 45608
         },
         {
            "address" : "172.200.0.254",
            "port" : 50336
         },
         {
            "address" : "172.17.0.1",
            "port" : 50336
         },
         {
            "address" : "172.17.0.1",
            "port" : 50333
         },
         {
            "address" : "172.200.0.254",
            "port" : 50335
         }
      ],
      "connected" : [
         {
            "address" : "172.17.0.1",
            "port" : 50334,
            "useragent" : "/NEO-GO:0.109.2-pre-12-ge11a83d8/"
         },
         {
            "address" : "172.200.0.254",
            "lastknownheight" : 1,
            "port" : 50335,
            "useragent" : "/NEO-GO:0.109.2-pre-12-ge11a83d8/"
         },
         {
            "address" : "172.200.0.254",
            "lastknownheight" : 1,
            "port" : 50334,
            "useragent" : "/NEO-GO:0.109.2-pre-12-ge11a83d8/"
         },
         {
            "address" : "172.200.0.254",
            "lastknownheight" : 1,
            "port" : 50333,
            "useragent" : "/NEO-GO:0.109.2-pre-12-ge11a83d8/"
         },
         {
            "address" : "172.17.0.1",
            "lastknownheight" : 1,
            "port" : 50335,
            "useragent" : "/NEO-GO:0.109.2-pre-12-ge11a83d8/"
         }
      ],
      "unconnected" : []
   }
}
```

Signed-off-by: Anna Shaleva <[email protected]>
@AnnaShaleva
network: remove good peer connection addr from unconnected/bad
65d3d9b 
Fix the second part of #3914.

Signed-off-by: Anna Shaleva <[email protected]>
@AnnaShaleva
network: drop duplicated peer connections by server nonce
8ecc928 
On dockerized private network with non-default ext/int ports mapping
there's no way to distinguish connections by underlying connecion
address prior to handshake. Even after handshake, if the peer doesn't
declare announced address/port the same way as it is declared in the
node's local peer record, there's still no other way than server nonce
to distinguish duplicating connections.

For example, on privnet with configuration from #3915, node will
maintain two connection to every seed:
```
// Start the node, wait for some connections to be established.

...

// Try to connect to seed nodes, in particular, with the seed 172.17.0.1:20334:
2025-05-22T10:44:55.779Z	INFO	choosing peer from seeds	{"addr": "172.17.0.1:20334", "requested": 1}
...
2025-05-22T10:44:55.779Z	INFO	dealing address	{"addr": "172.17.0.1:20334"}

...

// New peer is connected (incoming connection), it's the seed node
// 172.17.0.1:20334 in fact, start protocol with this node:
2025-05-22T10:44:55.908Z	INFO	new peer connected	{"peer addr": "172.200.0.254:42654", "fake conn addr": "172.200.0.254:42654", "conn addr": "172.200.0.254:42654", "peerCount": 1}
2025-05-22T10:44:55.909Z	INFO	started protocol	{"addr": "172.200.0.254:42654", "userAgent": "/NEO-GO:0.109.2-pre-16-g953c3580/", "startHeight": 0, "id": 2422991527}

...

// Outgoing seed connection succeeds, there's no way to distinguish this
// connection from incoming one based on any available address:
2025-05-22T10:44:55.994Z	INFO	updating seed address	{"seed": "172.17.0.1:20334", "old": "", "new": "172.17.0.1:20334"}
2025-05-22T10:44:55.994Z	INFO	register connected	{"addr": "172.17.0.1:20334"}
2025-05-22T10:44:55.994Z	INFO	new peer connected	{"peer addr": "172.17.0.1:20334", "fake conn addr": "172.17.0.1:20334", "conn addr": "172.17.0.1:20334", "peerCount": 2}

...

// Start protocol with the same seed based on outgoing connection:
2025-05-22T10:44:55.996Z	INFO	started protocol	{"addr": "172.17.0.1:20334", "userAgent": "/NEO-GO:0.109.2-pre-16-g953c3580/", "startHeight": 0, "id": 2422991527}

```

That's it, addresses can't tell us anything in this case. So we either
live with duplicating connections or filter out connections by server
ID. The latter is not the best option sice an invironment is untrusted
and peer may intentionally fake its ID, but we don't have more reliable
information.

Close #3915.

Signed-off-by: Anna Shaleva <[email protected]>
@AnnaShaleva AnnaShaleva marked this pull request as ready for review May 22, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roman-khimov roman-khimov Awaiting requested review from roman-khimov roman-khimov is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Duplicating peer connections Peer in "connected" and "disconnected" state simultaneously
1 participant
@AnnaShaleva