Skip to content

FretService.start() resolves before its RPC handler registrations fully complete (NOT URGENT) #1

Description

@aarashrestha

Affected versions: confirmed in p2p-fret@0.4.0, and STILL PRESENT in p2p-fret@0.6.0
(re-confirmed by direct inspection of the installed 0.6.0 dist, 2026-07-15 — service/fret-service.js
start() still calls this.registerRpcHandlers() without await, and registerRpcHandlers()
itself remains a synchronous, non-awaited fire-and-forget wrapper around all four register*
calls; each rpc/{ping,leave,maybe-act,neighbors}.js register* function still calls
void node.handle(...)) against the resolved runtime peer libp2p@3.3.2 (whose
Registrar.handle() this issue's mechanism depends on). Unlike the sibling
runOnLimitedConnection defect (fixed upstream in 0.6.0 — see the companion issue), this gap
survived the 0.6.0 release and stays file-worthy, re-pointed at 0.6.0.

Note — not a downstream blocker. This is filed as a correctness
report, not because it is blocking a consumer. In the VoteTorrent stack the strand-cohort
discovery path runs green on p2p-fret@0.6.0 without any fix for this gap (a multi-peer
relay harness forms the cohort with an all-zero error histogram — no rejected early dials
observed), consistent with this being a latent readiness race that widens under real relay-hop
RTT rather than a deterministic failure. It is a real defect worth fixing upstream; it is not
urgent for any consumer we are aware of.

Summary

CoreFretService.start() does not await its own registerRpcHandlers() call, and each of the four
register* functions (registerPing, registerNeighbors, registerMaybeAct, registerLeave)
registers its handler via fire-and-forget void node.handle(...). libp2p's Registrar.handle()
synchronously inserts the handler into its internal map but then awaits peerStore.merge() to
advertise the protocol to peers via identify — so the Startable chain that libp2p.start() awaits
before reporting the node "started" can resolve while that advertisement is still in flight. This
is a real async-completeness gap: a sibling peer that connects and dials a FRET sub-protocol very
soon after this node reports itself started can race the still-in-flight registration, in a window
that widens under real network latency (relay-hop RTT) versus a local loopback test.

This is a defect in this package: start() should not report readiness before its own protocol
handlers are fully registered (map insert and protocol advertisement), not something the
calling application can work around from outside the service.

Reproduction

The defect is a timing/completeness gap rather than a deterministic crash, so it is best understood
mechanically. CoreFretService.start() (pre-fix):

async start() {
    await this.seedFromPeerStore();
    this.registerRpcHandlers();   // NOT awaited
    this.startStabilizationLoop();
    if (this.mode === 'active') ...
}

registerRpcHandlers() {
    registerNeighbors(this.node, ...);   // fire-and-forget internally (see below)
    registerMaybeAct(this.node, ...);
    registerLeave(this.node, ...);
    registerPing(this.node, ...);
}

Each register* function (e.g. rpc/ping.js):

export function registerPing(node, protocol = PROTOCOL_PING, getSizeEstimate) {
    void node.handle(protocol, async (stream) => { ... });   // fire-and-forget
}

Because node.handle()'s promise is never returned or awaited anywhere in the call chain,
FretService.start() — and therefore the whole libp2p Startable chain components.start()
awaits before the node is reported "started" and begins accepting connections / being discovered —
can resolve before the four underlying peerStore.merge() calls (see Root cause) have actually
settled.

Reference the FRET-NEGOTIATION SMOKE section of packages/p2p-probe-host/relay-multi-peer-smoke.mjs,
and note honestly that this timing gap is wall-clock-widened on a real network (relay-hop RTT) versus
a Node loopback harness — so it is a source-cited correctness defect rather than one that
deterministically crashes on every run. The mechanism trace above is the primary evidence for the
defect; a live capture of a rejected early dial is a probabilistic, not guaranteed, reproduction.

Expected: start() resolves only after all four RPC handlers are fully registered — both the
registrar map insert and the peerStore protocol advertisement.

Actual: start() can resolve while one or more peerStore.merge() calls from handler
registration are still in flight, so an early dial from a freshly-connected sibling can be rejected
even though the handler will shortly be live.

Root cause

dist/src/service/fret-service.js (0.4.0 line numbers; re-confirmed against 0.6.0, whose
start()/registerRpcHandlers() are unchanged in shape, only shifted to :234/:369):

  • start() (:176-179 in 0.4.0, :234-239 in 0.6.0): calls this.registerRpcHandlers()
    synchronously, without await.
  • registerRpcHandlers() (:248-255 in 0.4.0, :369-376 in 0.6.0): calls all four register*
    functions synchronously; none of their return values are collected or awaited.

The four void node.handle(...) call sites (line numbers identical in both 0.4.0 and 0.6.0):

  • rpc/ping.js:9
  • rpc/neighbors.js:6,17 (two handlers: the primary neighbors protocol and its announce variant)
  • rpc/maybe-act.js:6
  • rpc/leave.js:20

The libp2p mechanism this races against — Registrar.handle() (libp2p/dist/src/registrar.js:65-80):
inserts the handler into this.handlers synchronously at :69 (before its first await), then
awaits this.components.peerStore.merge(this.components.peerId, { protocols: [protocol] }, opts)
at :78-80. That peerStore.merge() call is what advertises the protocol to remote peers via
identify's outgoing protocol list — the step a fire-and-forget caller allows to still be pending
when start() returns.

Fix

Make each register* function return its node.handle(...) promise instead of void-ing it,
make registerRpcHandlers() async and await Promise.all([...]) all four registrations, and make
start() await this.registerRpcHandlers().

rpc/ping.js:

export function registerPing(node, protocol = PROTOCOL_PING, getSizeEstimate) {
    return node.handle(protocol, async (stream) => { ... });
}

rpc/neighbors.js (both the primary and announce handlers, collected and awaited together):

export async function registerNeighbors(node, getSnapshot, onAnnounce, protocols, maxBytes) {
    const registrations = [
        node.handle(protocols.PROTOCOL_NEIGHBORS, async (stream) => { ... }),
    ];
    if (onAnnounce) {
        registrations.push(node.handle(protocols.PROTOCOL_NEIGHBORS_ANNOUNCE, async (stream) => { ... }));
    }
    await Promise.all(registrations);
}

rpc/maybe-act.js and rpc/leave.js mirror rpc/ping.js's change (return instead of void).

service/fret-service.js:

async start() {
    await this.seedFromPeerStore();
    await this.registerRpcHandlers();
    this.startStabilizationLoop();
    if (this.mode === 'active') ...
}

async registerRpcHandlers() {
    await Promise.all([
        registerNeighbors(this.node, ..., this.protocols, this.maxBytesNeighbors()),
        registerMaybeAct(this.node, ..., this.protocols.PROTOCOL_MAYBE_ACT, this.maxBytesMaybeAct()),
        registerLeave(this.node, ..., this.protocols.PROTOCOL_LEAVE),
        registerPing(this.node, this.protocols.PROTOCOL_PING, ...),
    ]);
}

Test reference

relay-multi-peer-smoke.js

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions