-
Notifications
You must be signed in to change notification settings - Fork 780
Description
While working on the same geographical region peering PR, I was able to recreate an issue that some of our node operators have complained about. With 0 inbound peers set and 1 outbound peer set as the max, my n_peers connected would jump to 25 when I started the node. This was on the osmo/v0.37.4 branch, which is not too divergent from mainline. After tracing it down, I found out that we are respecting the number of peers to dial in the ensurePeers logic branch, but the bug occurs here:
cometbft/p2p/pex/pex_reactor.go
Lines 370 to 385 in f4c622f
| // If this address came from a seed node, try to connect to it without | |
| // waiting (#2093) | |
| if srcIsSeed { | |
| go func(addr *p2p.NetAddress) { | |
| err := r.dialPeer(addr) | |
| if err != nil { | |
| switch err.(type) { | |
| case ErrMaxAttemptsToDial, ErrTooEarlyToDial, p2p.ErrCurrentlyDialingOrExistingAddress: | |
| r.Logger.Debug(err.Error(), "addr", addr) | |
| default: | |
| r.Logger.Debug(err.Error(), "addr", addr) | |
| } | |
| } | |
| }(netAddr) | |
| } | |
| } |
This logic branch disregards any user defined max limits and dials peers here. In my branch i just commented out this entire logic branch and the limits were once again respected, but made an issue here to discuss what the real desired solution is. Sadly Reactor doesn't have access to user configs as it stands so it's not as simple as checking if we are at the max number of peers already.