Skip to content

chore: migrate to nim-libp2p v2.2.0 - #4102

Draft
fcecin wants to merge 8 commits into
masterfrom
integration/libp2p-2.2.0
Draft

chore: migrate to nim-libp2p v2.2.0#4102
fcecin wants to merge 8 commits into
masterfrom
integration/libp2p-2.2.0

Conversation

@fcecin

@fcecin fcecin commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

PR Description

NOTE: I'm working on an alternative version of the NAT feature; if that pans out, this PR will be replaced.

This PR bumps nim-libp2p to v2.2.0.

The work is split across the 8 PRs below, which can be reviewed individually.

Most changes in this PR are about NAT. v2.2.0 adds a NATConfig covering port mapping (UPnP, NAT-PMP or a static external IP), reachability (autonat), and hole punching. Each is built with its own helper (upnpConfig, natPmpConfig, explicitIpConfig) and attached to a switch with withNAT, which can be called once per concern. A NATService on the switch owns gateway discovery, the mappings and their lease renewal, and publishes the mapped addresses through the switch's address mapper chain.

--nat keeps its contract: none, any, upnp, pmp, extip:<ip>. It is parsed once at the CLI edge into a typed NatStrategy.

The point at which mappings are known also moved. setupNat ran before the switch existed, so an external IP and ports were available while the announced addresses were still being built. Under the NATService they exist only once the switch has started, so the announced set is now resolved at start: port 0 placeholders are replaced with the ports the sockets actually bound, and the NATService's mapped addresses are read off the address mapper chain and added to the announced set. The ENR is rebuilt from that same set, and again whenever a later mapping changes the set.

Announced addresses now follow the mappings that exist rather than the ones we assume. The external port used to be guessed from the bind port; now a transport without a mapping announces no external address for that transport, a mapping that changes port is followed, and one that lapses stops being announced.

discv5 is mapped separately. Its UDP socket is not owned by the switch, so the NATService does not map it; the node requests that mapping itself and renews it before the lease expires, and the port the gateway grants is the one the ENR advertises.

What configuration asked for is kept separately as a base, and the announced set is recomputed from that base plus whatever mappings are live rather than edited in place. Mappings can appear, move to another port or lapse without taking the configured entries with them, so an operator's extip: endpoint or dns4 announce stays in the set regardless of what the gateway is doing.

Notes

  • AutonatV2 is not included in this bump work.
  • CI job send-api-e2e-tests is failing in master, and this PR makes it slightly worse (e.g. NAT changes make e2e tests take longer, which is solved if we increase timeouts from e.g. 20s to 45s); fixes for this CI job should be in a separate PR.
  • nat_traversal is no longer a direct dependency, but libp2p's NATService links the same miniupnpc and libnatpmp static libraries. Nat.mk, the iOS vendor build in logos_delivery.nimble and library/ios_natpmp_stubs.c therefore stay, although Bump nim-libp2p to v2.2.0 and replace nat_traversal with libp2p NATConfig #4041 expects all three to go with the dependency.

Changes

Superseded PRs

closes #4055

closes #4060

Issue

Closes #4041

fcecin added 8 commits August 1, 2026 01:42
* bump nim-libp2p
* require nimble 0.24.1 for the 2.2.0 dependency graph
* drop the direct nat_traversal requirement
* update mix revision
* pin lsquic
* refresh dependency locks
* update imports
* adapt API changes
* update protocol metadata
* read the relay message payload once per callback
* store peer public keys as bytes and validate them on decode
* run the relay bad-peers test over TCP only
* fix affected tests
)

* detect zero-port placeholders
* announce switch-resolved addresses
* install the announce mapper after the switch has started
* refresh the signed peer record from the resolved addresses
* model NAT user intent
* parse existing CLI values
* thread strategy through config
* treat an empty nat value as not specified
* add NAT strategy tests
* configure NATService via the switch builder
* select UPnP / NAT-PMP port mappers per strategy
* support UPnP-then-NAT-PMP fallback for --nat any
* remove legacy nim-eth switch mapping (setupNat)
* preserve builder-attached NATService among waku switch services
* migrate chat2, chat2bridge, chat2mix off setupNat
* default test nodes to --nat none
* capture NAT-mapped addresses from the address mapper chain
* recompute announced addresses from the configured base and the mappings
* keep every mapped endpoint when resyncing from the recomputed config
* stop announcing external endpoints once their mapping lapses
* feed NAT results into recomputed NetConfig and ENR
* announce no guessed external quic port without a mapping
* bound gateway discovery
* recover rejected mappings
* re-elect fallback mappers
* map discv5 UDP and renew it
* refresh the ENR when discv5 renews on a different external port
* project the discv5 external port only with a switch external address
* reject a zero NAT discovery timeout
* refresh ENR on announced-address changes
* announce no guessed external websocket port without a mapping
* extend NAT coverage
…ost (#4100)

* raise the test job timeout from 60 to 80 minutes
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

You can find the images built from this PR at

quay.io/wakuorg/nwaku-pr:4102
quay.io/wakuorg/nwaku-pr:4102-logosdeliverynode

Built from b944495

@stubbsta

stubbsta commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Hi, here's what Claude tells me:

Issues

Correctness / behavior

  1. discv5 renewal failure is silent and unboundedlogos_delivery/waku/waku.nim (discv5NatRenewLoop). A failed renewal is only debug-logged and the loop just waits another half-lease. Two consecutive failures mean the mapping lapses while the ENR keeps advertising the dead external port indefinitely (until some later renewal succeeds). The switch-side fold drops lapsed mappings; discv5 has no equivalent. Consider warn once the lease has actually expired, and reverting discv5Conf.udpPort to the bind port (mirroring the "only projected when the switch also has an external address" guard at startup).

  2. RetryingPortMapper blindly deletes the occupying mappinglogos_delivery/waku/net/nat_config.nim (RetryingPortMapper.map). On permissive UPnP gateways (many IGDs accept DeletePortMapping from any LAN host), delete-then-retry can hijack a mapping legitimately owned by another device, not just reclaim a stale one from a previous run. The comment acknowledges the other-host case only as "cannot be reclaimed". Suggest logging the unmap at info/warn (not debug) so operators can diagnose it, and note the hijack possibility in the doc comment. Also, the alternate-port formula 49152 + port mod 16000 is deterministic, so two nodes behind one gateway requesting the same port collide again on the alternate — a single extra probe is fine, but worth a comment.

  3. Re-election leaks mappingsFallbackPortMapper (logos_delivery/waku/net/nat_config.nim): when the active mapper loses the gateway and another candidate is elected, mappings created through the old mapper are never unmapped and linger until lease expiry. Probably acceptable (the gateway likely rebooted anyway), but worth a doc-comment line.

  4. Operator address sharing the NAT external IP follows the mapping lifecyclesetBaseAnnouncedAddresses (logos_delivery/waku/node/waku_node.nim) strips every address whose IP equals the discovered external IP from the base. An --ext-multiaddr the operator configured with that same IP would be dropped when the mapping lapses, unlike other operator entries. Edge case; a mention in the doc comment would do.

  5. Per-renewal rediscovery — the strategy-based mapUdpPort overload constructs a fresh mapper and re-runs gateway discovery on every renewal tick (logos_delivery/waku/waku.nim, mapDiscv5Port). For --nat any where NAT-PMP won, every ~30 min renewal still burns the full UPnP SSDP timeout first. Caching one mapper in the renew loop (the first overload exists precisely for that) would fix it. Not blocking — worst case is ~1s of extra latency per tick at the chosen default timeout.

Configuration surface

  1. withNatDiscoveryTimeoutMs is unreachable — it exists on the conf builder (logos_delivery/waku/factory/conf_builder/waku_conf_builder.nim) and is validated/threaded through EndpointConf, but no CLI flag or TOML wiring calls it. Either add the --nat-discovery-timeout-ms flag or note it's library-only for now.

  2. cli_args.nim's if n.nat != "" guard changes empty --nat= from a runtime error to a warn-and-default — that's an improvement, but is a small contract change worth a line in the PR description.

JohnM549 commented Aug 6, 2026

Copy link
Copy Markdown

Correction: this comment was posted to the wrong repository while investigating a different PR numbered #4102. Please disregard it. No code or branch changes were made here.

@gmelodie

Copy link
Copy Markdown
Collaborator

nim-libp2p v2.3.0 will likely be released today, so maybe worth wait and integrate with that instead?

@fcecin

fcecin commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

nim-libp2p v2.3.0 will likely be released today, so maybe worth wait and integrate with that instead?

Thanks! Yes, I'll port the bump version I have now to it. This PR here won't be merged.

@fcecin
fcecin marked this pull request as draft August 10, 2026 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bump nim-libp2p to v2.2.0 and replace nat_traversal with libp2p NATConfig

4 participants