Skip to content

feat: service discovery integration#3772

Closed
SionoiS wants to merge 34 commits into
masterfrom
feat--service-discovery
Closed

feat: service discovery integration#3772
SionoiS wants to merge 34 commits into
masterfrom
feat--service-discovery

Conversation

@SionoiS

@SionoiS SionoiS commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Service discovery integration for testing purposes. Will not be merged.

@SionoiS SionoiS self-assigned this Mar 20, 2026
Comment thread tools/confutils/cli_args.nim
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:3772

Built from 94ba1eb

@SionoiS
SionoiS force-pushed the feat--service-discovery branch from 4d21fba to f94efda Compare May 5, 2026 16:25
@SionoiS SionoiS mentioned this pull request May 6, 2026
@SionoiS
SionoiS force-pushed the feat--service-discovery branch from 5ec18fb to 55e752c Compare May 12, 2026 11:57
SionoiS and others added 4 commits June 5, 2026 08:09
… to compile chat2disco

- waku.nimble: libp2p #f54c7150a7ccbc4e9871bb8b56ecfd7e3e59f7de; also pin protobuf_serialization#ce97ba0 and websock#fb8ba71 to match new libp2p reqs; mix remains on 6c5f43 (its declared pins lag)
- nimble.lock + nix/deps.nix updated (libp2p rev/sha)
- Source fixes for new libp2p (object configs, removed utility module -> libp2p/utils/opt, rendezvous nil, kademlia no longer imports mix_protocol to reduce bad dep surface)
- nph on touched .nim
- chat2disco builds+starts successfully against the updated libp2p (with in-nimbledeps patches to mix for its removed symbols like sequninit/utility and withValue(Opt) sites; run make update will require similar or upstream mix bump)

Refs the 106-commit libp2p delta with kademlia/service-disco fixes (e.g. ticket time, record sizes, registration).
…89 on start

Root cause (post libp2p f54c715 update + rv/kad/service-disco changes):
- chat2disco (and waku factory for circuitRelayClient) do direct
  `node.switch.services = @[Service(hp/autonat)]` (bypassing the
  deprecated switch.add that called .setup).
- AutonatService (enableAddressMapper=true by default) + HP require
  their .setup(switch) to populate .addressMapper (and handlers)
  before start().
- In switch.start: services.start → autonat.start does
  addressMappers.add(nil); await peerInfo.update() → for m in ...:
  await m(...) → nil proc deref (Defect/SEGV in release).
- Surfaces at the `await node.switch.start()` (waku_node:589).
- Secondary: wakuKademlia.start() + waku mapper (capturing node,
  returning announced which hp mutates) scheduled before switch.start
  (which activates the mounted kad via ms + runs the hp mappers/updates).

Fix:
- After services= in apps/chat2disco/chat2disco.nim and in
  waku/factory/waku.nim (both hp and bare autonat branches), explicitly
  call the .setup(node.switch) (or hp.setup) and handle error.
- Move `if not node.wakuKademlia.isNil(): ...start()` to after
  switch.start() + reconnectRelayPeers (correct ordering for mounted
  protocol user loops).
- Harden waku addressMapper (nil/empty guard, return listenAddrs) and
  set peerInfo.announcedAddrs (short-circuit) at the add site, in
  updateAnnounced..., and in the onReservation callbacks (chat2disco +
  factory) so expandAddrs prefers it.
- Minor: lookup/periodic guards in waku_kademlia; doc in
  autonat_service.

Also nph reformats on touched files.

Reuses: the .setup methods, existing post-switch init patterns,
isNil guards, CatchableError handling, make chat2disco + nph.

Verified: make chat2disco (twice, pre/post nph) SuccessX; no SEGV in
multiple start-path runs (only expected thread EOF on pipe close);
diff only our 5 files.

Builds on 82d87cfa (libp2p update) without touching pins or vendored.
Caveat: clean `make update` still requires the mix temp patches in
pkgs2 (as documented in the update).

Fixes the reported chat2disco startup segfault.
@SionoiS

SionoiS commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Can't make it compile anymore with latest libp2p. Closing in favor of something else that works...

@SionoiS SionoiS closed this Jun 11, 2026
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.

2 participants