Skip to content

fix(service-disco): advertise a caller-supplied XPR - #99

Merged
gmelodie merged 4 commits into
masterfrom
fix/service-disco/advertise-full-xpr
Aug 11, 2026
Merged

fix(service-disco): advertise a caller-supplied XPR#99
gmelodie merged 4 commits into
masterfrom
fix/service-disco/advertise-full-xpr

Conversation

@gmelodie

@gmelodie gmelodie commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #98.

startAdvertising now takes the advertisement to publish, so a consuming module that runs on its own switch advertises its own peer ID and addresses. Before, the module encoded serviceInfo into an XPR built from its own connectable address, and discoverers reached the wrong node.

discoStartAdvertising(serviceId, serviceData, advertisement): advertisement is a base64 signed XPR, the same encoding createXpr returns and decodeXpr accepts. An empty string keeps the current behaviour, and the node advertises its own record. The module decodes the base64 and passes the bytes through; nim-libp2p's validateAdvert rejects a record that fails to decode, exceeds MaxXPRSize, or does not list the service, and the reason reaches the caller.

The libp2p input stays on master and moves to 00cab64, the merge of vacp2p/nim-libp2p#2917, which adds the advertisement: seq[byte] field to StartAdvertisingRequest and makes startAdvertising return Result[void, string].

That bump also picks up a week of nim-libp2p master, which renamed the generated entry points for {.ffiStatic.} procs. Two unrelated call sites follow the rename: libp2p_ctx_collect_metrics becomes libp2p_static_collect_metrics, and libp2p_ctx_new_private_key becomes libp2p_static_new_private_key. Both keep callSyncWith, so the existing context requirement does not change.

Tests:

  • disco_advertise_foreign_xpr builds an XPR on a node that mounts no service discovery, advertises it from a second node, and asserts that the discoverer resolves the first node's peer ID and address.
  • disco_advertise_rejects_bad_xpr covers bad base64, an XPR for a different service, and a tampered signature.
  • The bootstrap-plus-two-peers fixture the disco tests repeated three times is now startDiscoPeer.

Verified locally: nix build .#lgx succeeds on the merged master pin. The integration suite ran earlier on the pre-merge branch pin, whose cbind API is identical, and reported 84 passed, 3 failed. All disco and XPR tests pass, including the two new ones.

Two pre-existing issues found while verifying, both out of scope here:

  • CI never builds the integration suite. tests/CMakeLists.txt looks for libp2p.so in ../lib, the test derivation does not vendor it, and every job logs [Libp2pTests] libp2p not found in ../lib — skipping integration tests. The suite has to be built by hand to run at all.
  • The 3 failures are the consequence: custom_handlers_mount_without_emit_event_ok, protocol_bridge_request_accept_roundtrip and protocol_bridge_request_no_response all call mountProtocol on a node with no emitEvent, which src/custom_handlers.cpp:9 rejects by design. The tests contradict the implementation and nothing has been running them.
  • createXpr is missing from the generated LIDL contract because its declaration in plugin.h spans three lines and the header parser drops those.

@gmelodie
gmelodie force-pushed the fix/service-disco/advertise-full-xpr branch from cc83a90 to 4771153 Compare August 6, 2026 12:54
@gmelodie
gmelodie force-pushed the fix/service-disco/advertise-full-xpr branch from 4771153 to b0deb0a Compare August 6, 2026 13:10
@gmelodie
gmelodie force-pushed the fix/service-disco/advertise-full-xpr branch from b0deb0a to 8ff6384 Compare August 6, 2026 14:02
@gmelodie gmelodie self-assigned this Aug 6, 2026
@gmelodie
gmelodie requested a review from vladopajic August 6, 2026 15:07
@gmelodie
gmelodie marked this pull request as ready for review August 6, 2026 15:07
@gmelodie
gmelodie requested a review from jm-clius August 6, 2026 15:07
Comment thread flake.nix Outdated
@gmelodie
gmelodie force-pushed the fix/service-disco/advertise-full-xpr branch from df0a2eb to 0e54afa Compare August 10, 2026 13:26
@gmelodie
gmelodie merged commit bafe4b9 into master Aug 11, 2026
10 checks passed
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.

service-disco-api: Add ability to provide fully-formed XPR to advertise

3 participants