feat: bump nim-libp2p to v2.0.0 - #3929
Conversation
* bump libp2p pin to release/v2.0.0 (c43199378) * pin nimble.lock: lsquic/websock/boringssl/protobuf_serialization/npeg/jwt * add libp2p_mix dep and point libp2p/protocols/mix -> libp2p_mix * migrate rng to libp2p Rng type (prod, channels, noise, tests) * noise: take Rng, extract bearSslDrbg internally * waku_switch: TransportConfig factory; withMaxInOut; local MaxConnections * waku_relay/rendezvous/discv5/kademlia: v2.0.0 API (rng, config, ServiceDiscovery) * tests: newStandardSwitch shim; PeerId.random(rng); common.rng()/crypto.newRng() * drop libp2p/utils/semaphore (use chronos AsyncSemaphore) * add waku/compat/option_valueor shim where needed * add std/options where transitive re-export dropped
* deps: json_rpc + web3 via websock-relaxed forks (WIP: revert on upstream fix) * fix: setup() post-build switch services (autonat nil segfault) * fix: unified 2.0.0 connection limits * test: 2.0.0 defaults (identify-push, agent string) * test: drop reboot/reconnect (unsupported Switch restart)
* add yamux to our newStandardSwitch to match prod behavior * test: rng instantiation cleanup
|
This PR may contain changes to database schema of one of the drivers. If you are introducing any changes to the schema, make sure the upgrade from the latest release to this change passes without any errors/issues. Please make sure the label |
|
You can find the image built from this PR at Built from f8fb050 |
* json_rpc: to v0.6.1 tag (latest) * lsquic: v0.5.1 (latest; QUIC connection-handling fixes) * boringssl: v0.0.8 (latest)
Ivansete-status
left a comment
There was a problem hiding this comment.
LGTM in general, thanks so much for it!
The only reason why I don't approve yet is because we are removing one test.
Besides, we can replace Option (std/options) with Opt (results) everywhere now or can be done in a separate PR.
| ## Polyfill: `valueOr` / `withValue` templates for `std/options.Option[T]`. | ||
| ## | ||
| ## Previously provided transitively by `libp2p/utility`, removed in | ||
| ## nim-libp2p PR #2162 (commit 8a9943145). logos-delivery uses these |
There was a problem hiding this comment.
We can avoid having that file by only using Opt instead of Option, similar to how has been done in vacp2p/nim-libp2p#2162.
Opt is defined in nim-results ( we usually get it with import result .)
There was a problem hiding this comment.
I think swapping Option for Opt everywhere is going to be its own Issue/PR yep
| url = "https://github.com/vacp2p/nim-jwt.git"; | ||
| rev = "18f8378de52b241f321c1f9ea905456e89b95c6f"; | ||
| sha256 = "1986czmszdxj6g9yr7xn1fx8y2y9mwpb3f1bn9nc6973qawsdm0p"; | ||
| rev = "057ec95eb5af0eea9c49bfe9025b3312c95dc5f2"; |
There was a problem hiding this comment.
I would double-check that file as maybe it contains too many changes.
There was a problem hiding this comment.
The nix/deps.nix file is auto-generated from the nimble.lock file. It can be redone locally to check that the result is identical (might need some extra nix tooling installed):
./tools/gen-nix-deps.sh nimble.lock /tmp/deps.regen.nix
git diff --no-index nix/deps.nix /tmp/deps.regen.nix
The question then is whether the nimble.lock is correct. It's at least very similar to what Prem did in his PR.
| await allFutures(otherSwitch.stop(), otherNode.stop()) | ||
|
|
||
| suite "Security and Privacy": | ||
| asyncTest "Relay can receive messages after reboot and reconnect": |
There was a problem hiding this comment.
Why removing that test?
There was a problem hiding this comment.
It's the "drop relay reboot/reconnect test (asserted a Switch restart capability that is simply not supported)" item in the PR desc.
It's basically that. Restarting a switch is not actually supported.
| NIM_PARAMS := $(NIM_PARAMS) -d:libp2p_mix_experimental_exit_is_dest | ||
|
|
||
| # enable libp2p's QUIC transport | ||
| NIM_PARAMS := $(NIM_PARAMS) -d:libp2p_quic_support |
There was a problem hiding this comment.
Maybe this line in particular should go for QUIC adoption. Not a big deal, though.
There was a problem hiding this comment.
Right; I don't think "libp2p without quic" is a thing that makes much sense, we want it enabled always so I rolled this into the bump.
jm-clius
left a comment
There was a problem hiding this comment.
I gave this only an overview, but satisfied myself that I have answers to the questions that came up (e.g. how setup() vs start9) is addressed, why MaxConnections are now explicit, etc.)
|
We will merge now but any later comments/reviews here are appreciated and will be addressed. |
Description
This PR bumps nim-libp2p to v2.0.0, which paves the way for adding QUIC support in logos-delivery.
This PR uses the libp2p v2 bump work in PR #3931 as a reference. Thanks @chaitanyaprem!
Changes
ref HmacDrbgContext-> libp2pRngacross prod/channels/tests (interface-only; same DRBG)Issue
iterates logos-messaging/pm#404
iterates logos-messaging/pm#324
closes #3634 (since nim-libp2p v2.0.0 contains vacp2p/nim-libp2p#2359)