fix(nip46): accept secret-echo connect response and send remote-signer-pubkey as first param per NIP-46

[DocNR]

Summary

NDKNip46Signer.blockUntilReady has two NIP-46 spec violations on the bunker:// path that prevent connecting to spec-compliant signers (e.g. Clave, nsec.app's bunker, anything matching nostr-tools BunkerSigner.fromURI semantics). Both bugs are present in every released NDK from 2.12.x through 3.0.3.

Per the NIP-46 spec:

connect: [<remote-signer-pubkey>, <optional_secret>, <optional_requested_perms>]
result: "ack" OR <required-secret-value>

Bug 1 — first connect param is the user-pubkey instead of the remote-signer-pubkey

Line 347 sends [this.userPubkey ?? ""]. For bunker:// URIs without ?pubkey= (the typical single-user signer case), userPubkey is null → an empty string lands as the first param. Per spec the first param is the remote-signer-pubkey, which for a bunker:// URI is the URI hostname (= bunkerPubkey).

Fixed by falling through to bunkerPubkey when userPubkey is null.

Bug 2 — only "ack" is accepted as a valid connect response

Line 354 only accepts response.result === "ack". Per spec, "ack" OR the URI secret echoed back are both valid. Spec-compliant signers like Clave and any signer matching nostr-tools BunkerSigner.fromURI semantics return the secret-echo form. NDK rejects with response.error which is undefined (no error in the response) — leaving downstream catch handlers to throw TypeError: Cannot read properties of undefined (reading 'message').

Fixed by accepting response.result === "ack" || response.result === this.secret.

Drive-by: rejection is now wrapped in a real Error so downstream handlers get a useful message instead of undefined.

Note: blockUntilReadyNostrConnect already does this right

Line 287 of the nostrconnect:// flow already matches response.result === this.nostrConnectSecret. This PR aligns the bunker:// path with that and with the spec.

Real-world failure

stacker.news's NIP-46 bunker login hangs indefinitely when paired with a Clave bunker URI for exactly these reasons. Companion PR (stackernews/stacker.news#2947) extends their existing NDKNip46SignerURLPatch workaround with the same fix; once this PR ships, that workaround can be deleted on next NDK bump.

Tests

3 new tests in core/src/signers/nip46/index.test.ts (following the existing mock pattern):

• blockUntilReady resolves user on URI secret echoed back (NIP-46 spec) — covers Bug 2
• blockUntilReady sends bunkerPubkey as first connect param when no userPubkey — covers Bug 1 (URI without ?pubkey=)
• blockUntilReady sends bunkerPubkey (not userPubkey) as first connect param when the URI carries ?pubkey= — covers Bug 1 for multi-user signer URIs (added after review feedback — the earlier fix still sent userPubkey as params[0] whenever the URI carried ?pubkey=)
• blockUntilReady rejects with a real Error (not undefined) for unexpected connect responses — covers the drive-by

Manually verified the fix against:

• A Clave-emulating mock bunker (returns secret echo) on a local nak serve relay → resolves
• A real Clave bunker URI from an iOS TestFlight build on relay.powr.build → resolves in ~2s

Test plan

• New unit tests not run locally. NDK is a bun workspace; I don't have bun available, and npm install inside core/ doesn't reproduce the workspace layout (vitest isn't hoisted into core/node_modules). The 4 new tests are written to the exact pattern of the existing blockUntilReady resolves user on ack test — same createMockRpc() harness, same mockImplementation callback shape. A maintainer running bun test is the validation gate here; flagging explicitly rather than claiming a green run I didn't do.
• Fix verified end-to-end against a Clave-emulating mock bunker (returns the secret echo) on a local nak serve relay and against a real Clave bunker URI from an iOS TestFlight build on relay.powr.build (resolves in ~2s). The mock-bunker harness drives the real NDKNip46Signer code path, not a reimplementation.

🤖 Generated with Claude Code