[Merged by Bors] - P2P decentralization improvements #5329
Closed
Conversation
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
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #5329 +/- ##
=========================================
- Coverage 77.5% 77.3% -0.2%
=========================================
Files 252 257 +5
Lines 29695 30441 +746
=========================================
+ Hits 23026 23546 +520
- Misses 5211 5401 +190
- Partials 1458 1494 +36 ☔ View full report in Codecov by Sentry. |
fasmat
reviewed
Dec 5, 2023
ivan4th
force-pushed
the
p2p-improvements
branch
from
d2223d4
to
c8adf86
Compare
fasmat
reviewed
Dec 11, 2023
fasmat
reviewed
Dec 11, 2023
ivan4th
force-pushed
the
p2p-improvements
branch
3 times, most recently
from
36fbe1c
to
a1a2e40
Compare
ivan4th
force-pushed
the
p2p-improvements
branch
from
97faa75
to
3cc024c
Compare
poszu
reviewed
Dec 15, 2023
| select { | ||
| case <-ctx.Done(): | ||
| return nil | ||
| case p, ok := <-peerCh: |
There was a problem hiding this comment.
peerCh could be nil at this point (if d.disc.FindPeers(ctx, ns) failed). Reading from it would block forever then.
There was a problem hiding this comment.
Forgot to continue the loop in the if above, fixed, thanks
ivan4th
force-pushed
the
p2p-improvements
branch
2 times, most recently
from
51db827
to
3f7e8ba
Compare
|
Fixed the conflict, created https://github.com/spacemeshos/api/releases/tag/v1.25.0 (also https://github.com/spacemeshos/api/releases/tag/release%2Fgo%2Fv1.25.0) |
Allow multiple values
ivan4th
force-pushed
the
p2p-improvements
branch
from
3f7e8ba
to
c531ea6
Compare
|
Disabled routing discovery advertisement by default, and made the nodes use bootnodes as relays when routing discovery is disabled for even better consistency with existing behaviour when an old config is being used. |
|
@poszu pls check if the README / changelog changes are sufficient (or if they're excessive) |
fasmat
reviewed
Dec 19, 2023
ivan4th
force-pushed
the
p2p-improvements
branch
from
c531ea6
to
3cea314
Compare
fasmat
approved these changes
Dec 19, 2023
|
bors merge |
spacemesh-bors bot
pushed a commit
that referenced
this pull request
Dec 19, 2023
## Motivation This PR implements changes needed for spacemeshos/pm#275, except for measurement ## Changes * Introduce Routing Discovery to contact peers behind NATs * Introduce dynamic v2 relay discovery which is needed for hole punching. The idea is to have a wider array of circuit-v2 passive relays which should be much safer than old libp2p active relays (which were disabled in e.g. Filecoin due to security concerns) * Introduce QUIC transport to improve chances at hole punching, with testnet-mainnet "crosstalk" protection based on a transport-level handshake mechanism * the handshake is not used on mainnet. That way, connections between mainnet and testnet nodes are still prevented, as testnet peers expect the handshake, but if/when my libp2p changes are merged (libp2p/go-libp2p#2658) or libp2p gets private network support * Make it possible to listen on multiple addresses and advertise multiple addresses * Extend DebugService with additional P2P info needed for hole punching diagnostics (needs spacemeshos/api#285) * Add `ping-peers` config option to facilitate P2P network issue diagnostics * Add `force-dht-server` config option that is useful during troubleshooting DHT and hole-punching issues `ping-peers` and `force-dht-server` were initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios. All of the changes are disabled in the config by default, except for: * libp2p Ping service is enabled by default to make diagnostics easier * DHT Values and Providers as these will make DHT Routing Peer discovery work efficiently from the beginning when we enable this feature in the configs * Bootnodes aren't used as relays by default anymore. v2 relays have very limited capacity by default and bootnode relay servers' reservations are very quickly exhausted. Need to either specify a static relay list or enable routing discovery, which searches for more available relays as needed ## Test Plan * Tested using k8s several clusters with cone NATs enabled via `bridge` CNI plugin (via Multus) -- backported to v1.2.8 * Added a Mac node for testing ## TODO - [x] Have spacemeshos/api#285 merged and updated to the new `api` release - [x] Retest using an image based on this branch (not backport) - [ ] Decide on whether/how to extend systests to include NAT testing - [ ] To check: TCP holepunching tends to happen more than QUIC (might be related to the handshake mechanism) - [ ] ~~To consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery~~ (doesn't work too well, need something more involved for that) Maybe as a follow-up (depending on how soon this gets reviewed): - Include new metrics / check if they're already present - NAT type (UDP / TCP) - Cone / Symmetric / Unknown - Reachability - Public / Private / Unknown - N of "advertised" peers found via routing discovery - N of TCP and UDP (QUIC) peers - N of peers reached via relayed connections (these being present for a long time may indicate hole-punching troubles, usually relayed connections go away relatively quickly) - N of relay reservations this node managed to obtain - Whether routing discovery is active or suspended (e.g. b/c `low-peers` N of peers has been reached) - Whether DHT is in the `Server` or `Client` mode - systests checking NATed connections Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
|
Pull request successfully merged into develop. Build succeeded: |
spacemesh-bors
bot
changed the title
ivan4th
added a commit
that referenced
this pull request
Dec 20, 2023
This PR implements changes needed for spacemeshos/pm#275, except for measurement * Introduce Routing Discovery to contact peers behind NATs * Introduce dynamic v2 relay discovery which is needed for hole punching. The idea is to have a wider array of circuit-v2 passive relays which should be much safer than old libp2p active relays (which were disabled in e.g. Filecoin due to security concerns) * Introduce QUIC transport to improve chances at hole punching, with testnet-mainnet "crosstalk" protection based on a transport-level handshake mechanism * the handshake is not used on mainnet. That way, connections between mainnet and testnet nodes are still prevented, as testnet peers expect the handshake, but if/when my libp2p changes are merged (libp2p/go-libp2p#2658) or libp2p gets private network support * Make it possible to listen on multiple addresses and advertise multiple addresses * Extend DebugService with additional P2P info needed for hole punching diagnostics (needs spacemeshos/api#285) * Add `ping-peers` config option to facilitate P2P network issue diagnostics * Add `force-dht-server` config option that is useful during troubleshooting DHT and hole-punching issues `ping-peers` and `force-dht-server` were initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios. All of the changes are disabled in the config by default, except for: * libp2p Ping service is enabled by default to make diagnostics easier * DHT Values and Providers as these will make DHT Routing Peer discovery work efficiently from the beginning when we enable this feature in the configs * Bootnodes aren't used as relays by default anymore. v2 relays have very limited capacity by default and bootnode relay servers' reservations are very quickly exhausted. Need to either specify a static relay list or enable routing discovery, which searches for more available relays as needed * Tested using k8s several clusters with cone NATs enabled via `bridge` CNI plugin (via Multus) -- backported to v1.2.8 * Added a Mac node for testing - [x] Have spacemeshos/api#285 merged and updated to the new `api` release - [x] Retest using an image based on this branch (not backport) - [ ] Decide on whether/how to extend systests to include NAT testing - [ ] To check: TCP holepunching tends to happen more than QUIC (might be related to the handshake mechanism) - [ ] ~~To consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery~~ (doesn't work too well, need something more involved for that) Maybe as a follow-up (depending on how soon this gets reviewed): - Include new metrics / check if they're already present - NAT type (UDP / TCP) - Cone / Symmetric / Unknown - Reachability - Public / Private / Unknown - N of "advertised" peers found via routing discovery - N of TCP and UDP (QUIC) peers - N of peers reached via relayed connections (these being present for a long time may indicate hole-punching troubles, usually relayed connections go away relatively quickly) - N of relay reservations this node managed to obtain - Whether routing discovery is active or suspended (e.g. b/c `low-peers` N of peers has been reached) - Whether DHT is in the `Server` or `Client` mode - systests checking NATed connections Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
ivan4th
added a commit
that referenced
this pull request
Dec 20, 2023
This PR implements changes needed for spacemeshos/pm#275, except for measurement * Introduce Routing Discovery to contact peers behind NATs * Introduce dynamic v2 relay discovery which is needed for hole punching. The idea is to have a wider array of circuit-v2 passive relays which should be much safer than old libp2p active relays (which were disabled in e.g. Filecoin due to security concerns) * Introduce QUIC transport to improve chances at hole punching, with testnet-mainnet "crosstalk" protection based on a transport-level handshake mechanism * the handshake is not used on mainnet. That way, connections between mainnet and testnet nodes are still prevented, as testnet peers expect the handshake, but if/when my libp2p changes are merged (libp2p/go-libp2p#2658) or libp2p gets private network support * Make it possible to listen on multiple addresses and advertise multiple addresses * Extend DebugService with additional P2P info needed for hole punching diagnostics (needs spacemeshos/api#285) * Add `ping-peers` config option to facilitate P2P network issue diagnostics * Add `force-dht-server` config option that is useful during troubleshooting DHT and hole-punching issues `ping-peers` and `force-dht-server` were initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios. All of the changes are disabled in the config by default, except for: * libp2p Ping service is enabled by default to make diagnostics easier * DHT Values and Providers as these will make DHT Routing Peer discovery work efficiently from the beginning when we enable this feature in the configs * Bootnodes aren't used as relays by default anymore. v2 relays have very limited capacity by default and bootnode relay servers' reservations are very quickly exhausted. Need to either specify a static relay list or enable routing discovery, which searches for more available relays as needed * Tested using k8s several clusters with cone NATs enabled via `bridge` CNI plugin (via Multus) -- backported to v1.2.8 * Added a Mac node for testing - [x] Have spacemeshos/api#285 merged and updated to the new `api` release - [x] Retest using an image based on this branch (not backport) - [ ] Decide on whether/how to extend systests to include NAT testing - [ ] To check: TCP holepunching tends to happen more than QUIC (might be related to the handshake mechanism) - [ ] ~~To consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery~~ (doesn't work too well, need something more involved for that) Maybe as a follow-up (depending on how soon this gets reviewed): - Include new metrics / check if they're already present - NAT type (UDP / TCP) - Cone / Symmetric / Unknown - Reachability - Public / Private / Unknown - N of "advertised" peers found via routing discovery - N of TCP and UDP (QUIC) peers - N of peers reached via relayed connections (these being present for a long time may indicate hole-punching troubles, usually relayed connections go away relatively quickly) - N of relay reservations this node managed to obtain - Whether routing discovery is active or suspended (e.g. b/c `low-peers` N of peers has been reached) - Whether DHT is in the `Server` or `Client` mode - systests checking NATed connections Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
ivan4th
added a commit
that referenced
this pull request
Dec 25, 2023
This PR implements changes needed for spacemeshos/pm#275, except for measurement * Introduce Routing Discovery to contact peers behind NATs * Introduce dynamic v2 relay discovery which is needed for hole punching. The idea is to have a wider array of circuit-v2 passive relays which should be much safer than old libp2p active relays (which were disabled in e.g. Filecoin due to security concerns) * Introduce QUIC transport to improve chances at hole punching, with testnet-mainnet "crosstalk" protection based on a transport-level handshake mechanism * the handshake is not used on mainnet. That way, connections between mainnet and testnet nodes are still prevented, as testnet peers expect the handshake, but if/when my libp2p changes are merged (libp2p/go-libp2p#2658) or libp2p gets private network support * Make it possible to listen on multiple addresses and advertise multiple addresses * Extend DebugService with additional P2P info needed for hole punching diagnostics (needs spacemeshos/api#285) * Add `ping-peers` config option to facilitate P2P network issue diagnostics * Add `force-dht-server` config option that is useful during troubleshooting DHT and hole-punching issues `ping-peers` and `force-dht-server` were initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios. All of the changes are disabled in the config by default, except for: * libp2p Ping service is enabled by default to make diagnostics easier * DHT Values and Providers as these will make DHT Routing Peer discovery work efficiently from the beginning when we enable this feature in the configs * Bootnodes aren't used as relays by default anymore. v2 relays have very limited capacity by default and bootnode relay servers' reservations are very quickly exhausted. Need to either specify a static relay list or enable routing discovery, which searches for more available relays as needed * Tested using k8s several clusters with cone NATs enabled via `bridge` CNI plugin (via Multus) -- backported to v1.2.8 * Added a Mac node for testing - [x] Have spacemeshos/api#285 merged and updated to the new `api` release - [x] Retest using an image based on this branch (not backport) - [ ] Decide on whether/how to extend systests to include NAT testing - [ ] To check: TCP holepunching tends to happen more than QUIC (might be related to the handshake mechanism) - [ ] ~~To consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery~~ (doesn't work too well, need something more involved for that) Maybe as a follow-up (depending on how soon this gets reviewed): - Include new metrics / check if they're already present - NAT type (UDP / TCP) - Cone / Symmetric / Unknown - Reachability - Public / Private / Unknown - N of "advertised" peers found via routing discovery - N of TCP and UDP (QUIC) peers - N of peers reached via relayed connections (these being present for a long time may indicate hole-punching troubles, usually relayed connections go away relatively quickly) - N of relay reservations this node managed to obtain - Whether routing discovery is active or suspended (e.g. b/c `low-peers` N of peers has been reached) - Whether DHT is in the `Server` or `Client` mode - systests checking NATed connections Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
ivan4th
added a commit
that referenced
this pull request
Dec 26, 2023
This PR implements changes needed for spacemeshos/pm#275, except for measurement * Introduce Routing Discovery to contact peers behind NATs * Introduce dynamic v2 relay discovery which is needed for hole punching. The idea is to have a wider array of circuit-v2 passive relays which should be much safer than old libp2p active relays (which were disabled in e.g. Filecoin due to security concerns) * Introduce QUIC transport to improve chances at hole punching, with testnet-mainnet "crosstalk" protection based on a transport-level handshake mechanism * the handshake is not used on mainnet. That way, connections between mainnet and testnet nodes are still prevented, as testnet peers expect the handshake, but if/when my libp2p changes are merged (libp2p/go-libp2p#2658) or libp2p gets private network support * Make it possible to listen on multiple addresses and advertise multiple addresses * Extend DebugService with additional P2P info needed for hole punching diagnostics (needs spacemeshos/api#285) * Add `ping-peers` config option to facilitate P2P network issue diagnostics * Add `force-dht-server` config option that is useful during troubleshooting DHT and hole-punching issues `ping-peers` and `force-dht-server` were initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios. All of the changes are disabled in the config by default, except for: * libp2p Ping service is enabled by default to make diagnostics easier * DHT Values and Providers as these will make DHT Routing Peer discovery work efficiently from the beginning when we enable this feature in the configs * Bootnodes aren't used as relays by default anymore. v2 relays have very limited capacity by default and bootnode relay servers' reservations are very quickly exhausted. Need to either specify a static relay list or enable routing discovery, which searches for more available relays as needed * Tested using k8s several clusters with cone NATs enabled via `bridge` CNI plugin (via Multus) -- backported to v1.2.8 * Added a Mac node for testing - [x] Have spacemeshos/api#285 merged and updated to the new `api` release - [x] Retest using an image based on this branch (not backport) - [ ] Decide on whether/how to extend systests to include NAT testing - [ ] To check: TCP holepunching tends to happen more than QUIC (might be related to the handshake mechanism) - [ ] ~~To consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery~~ (doesn't work too well, need something more involved for that) Maybe as a follow-up (depending on how soon this gets reviewed): - Include new metrics / check if they're already present - NAT type (UDP / TCP) - Cone / Symmetric / Unknown - Reachability - Public / Private / Unknown - N of "advertised" peers found via routing discovery - N of TCP and UDP (QUIC) peers - N of peers reached via relayed connections (these being present for a long time may indicate hole-punching troubles, usually relayed connections go away relatively quickly) - N of relay reservations this node managed to obtain - Whether routing discovery is active or suspended (e.g. b/c `low-peers` N of peers has been reached) - Whether DHT is in the `Server` or `Client` mode - systests checking NATed connections Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
dsmello
pushed a commit
that referenced
this pull request
Dec 28, 2023
## Motivation This PR implements changes needed for spacemeshos/pm#275, except for measurement ## Changes * Introduce Routing Discovery to contact peers behind NATs * Introduce dynamic v2 relay discovery which is needed for hole punching. The idea is to have a wider array of circuit-v2 passive relays which should be much safer than old libp2p active relays (which were disabled in e.g. Filecoin due to security concerns) * Introduce QUIC transport to improve chances at hole punching, with testnet-mainnet "crosstalk" protection based on a transport-level handshake mechanism * the handshake is not used on mainnet. That way, connections between mainnet and testnet nodes are still prevented, as testnet peers expect the handshake, but if/when my libp2p changes are merged (libp2p/go-libp2p#2658) or libp2p gets private network support * Make it possible to listen on multiple addresses and advertise multiple addresses * Extend DebugService with additional P2P info needed for hole punching diagnostics (needs spacemeshos/api#285) * Add `ping-peers` config option to facilitate P2P network issue diagnostics * Add `force-dht-server` config option that is useful during troubleshooting DHT and hole-punching issues `ping-peers` and `force-dht-server` were initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios. All of the changes are disabled in the config by default, except for: * libp2p Ping service is enabled by default to make diagnostics easier * DHT Values and Providers as these will make DHT Routing Peer discovery work efficiently from the beginning when we enable this feature in the configs * Bootnodes aren't used as relays by default anymore. v2 relays have very limited capacity by default and bootnode relay servers' reservations are very quickly exhausted. Need to either specify a static relay list or enable routing discovery, which searches for more available relays as needed ## Test Plan * Tested using k8s several clusters with cone NATs enabled via `bridge` CNI plugin (via Multus) -- backported to v1.2.8 * Added a Mac node for testing ## TODO - [x] Have spacemeshos/api#285 merged and updated to the new `api` release - [x] Retest using an image based on this branch (not backport) - [ ] Decide on whether/how to extend systests to include NAT testing - [ ] To check: TCP holepunching tends to happen more than QUIC (might be related to the handshake mechanism) - [ ] ~~To consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery~~ (doesn't work too well, need something more involved for that) Maybe as a follow-up (depending on how soon this gets reviewed): - Include new metrics / check if they're already present - NAT type (UDP / TCP) - Cone / Symmetric / Unknown - Reachability - Public / Private / Unknown - N of "advertised" peers found via routing discovery - N of TCP and UDP (QUIC) peers - N of peers reached via relayed connections (these being present for a long time may indicate hole-punching troubles, usually relayed connections go away relatively quickly) - N of relay reservations this node managed to obtain - Whether routing discovery is active or suspended (e.g. b/c `low-peers` N of peers has been reached) - Whether DHT is in the `Server` or `Client` mode - systests checking NATed connections Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
ivan4th
added a commit
that referenced
this pull request
Dec 29, 2023
This PR implements changes needed for spacemeshos/pm#275, except for measurement * Introduce Routing Discovery to contact peers behind NATs * Introduce dynamic v2 relay discovery which is needed for hole punching. The idea is to have a wider array of circuit-v2 passive relays which should be much safer than old libp2p active relays (which were disabled in e.g. Filecoin due to security concerns) * Introduce QUIC transport to improve chances at hole punching, with testnet-mainnet "crosstalk" protection based on a transport-level handshake mechanism * the handshake is not used on mainnet. That way, connections between mainnet and testnet nodes are still prevented, as testnet peers expect the handshake, but if/when my libp2p changes are merged (libp2p/go-libp2p#2658) or libp2p gets private network support * Make it possible to listen on multiple addresses and advertise multiple addresses * Extend DebugService with additional P2P info needed for hole punching diagnostics (needs spacemeshos/api#285) * Add `ping-peers` config option to facilitate P2P network issue diagnostics * Add `force-dht-server` config option that is useful during troubleshooting DHT and hole-punching issues `ping-peers` and `force-dht-server` were initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios. All of the changes are disabled in the config by default, except for: * libp2p Ping service is enabled by default to make diagnostics easier * DHT Values and Providers as these will make DHT Routing Peer discovery work efficiently from the beginning when we enable this feature in the configs * Bootnodes aren't used as relays by default anymore. v2 relays have very limited capacity by default and bootnode relay servers' reservations are very quickly exhausted. Need to either specify a static relay list or enable routing discovery, which searches for more available relays as needed * Tested using k8s several clusters with cone NATs enabled via `bridge` CNI plugin (via Multus) -- backported to v1.2.8 * Added a Mac node for testing - [x] Have spacemeshos/api#285 merged and updated to the new `api` release - [x] Retest using an image based on this branch (not backport) - [ ] Decide on whether/how to extend systests to include NAT testing - [ ] To check: TCP holepunching tends to happen more than QUIC (might be related to the handshake mechanism) - [ ] ~~To consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery~~ (doesn't work too well, need something more involved for that) Maybe as a follow-up (depending on how soon this gets reviewed): - Include new metrics / check if they're already present - NAT type (UDP / TCP) - Cone / Symmetric / Unknown - Reachability - Public / Private / Unknown - N of "advertised" peers found via routing discovery - N of TCP and UDP (QUIC) peers - N of peers reached via relayed connections (these being present for a long time may indicate hole-punching troubles, usually relayed connections go away relatively quickly) - N of relay reservations this node managed to obtain - Whether routing discovery is active or suspended (e.g. b/c `low-peers` N of peers has been reached) - Whether DHT is in the `Server` or `Client` mode - systests checking NATed connections Co-authored-by: Ivan Shvedunov <ivan4th@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
This PR implements changes needed for spacemeshos/pm#275, except for measurement
Changes
ping-peersconfig option to facilitate P2P network issue diagnosticsforce-dht-serverconfig option that is useful during troubleshooting DHT and hole-punching issuesping-peersandforce-dht-serverwere initially considered to be temporary features, but I think it might make sense to keep them for various P2P network troubleshooting scenarios.All of the changes are disabled in the config by default, except for:
Test Plan
bridgeCNI plugin (via Multus) -- backported to v1.2.8TODO
apireleaseTo consider: try picking up some % (e.g.: 50%) of non-infra peers during routing discovery(doesn't work too well, need something more involved for that)Maybe as a follow-up (depending on how soon this gets reviewed):
low-peersN of peers has been reached)ServerorClientmode