fix(nip46): always include Amber-friendly fallback relays in the pairing set - #74
Merged
Merged
Conversation
…ing set
Hoot subscribed to kind:24133 events only on relays from the
user's relays.txt. If that list was sparse (the user's
relays.txt has only purplerelay.com, nos.lol, damus.io,
relay.nostr.band — no nostr.wine) or contained relays Amber
didn't recognise, Amber published the connect ack to its own
preferred relay (wss://relay.damus.io / wss://nostr.wine) and
hoot never saw it. Symptom: scan approved in Amber, no
response in hoot — the TUI spins forever on the QR screen.
Fix: nip46.PairingRelays() always appendes a hard-coded set of
fallback relays (wss://relay.damus.io, wss://nostr.wine) to
whatever the user configured, dedup'd. Both the nostrconnect://
URI and hoot's subscription set use this combined list, so:
- Amber always sees a relay it trusts in the URI
(so it picks one to publish to), AND
- hoot always subscribes to a relay Amber is willing to use
(so the connect ack actually arrives)
Also: ConnectRelays now skips relays whose connection died
between dial and subscribe (IsConnected() == false). Previously
we'd subscribe against a closed relay, producing a never-firing
event channel that fooled CheckConnection into 'no events yet'
forever.
CheckConnection now distinguishes 'all subscriptions closed'
from 'no events yet' — the former returns an actionable error
pointing at the network so users don't re-scan fruitlessly.
Tests:
- TestPairingRelaysIncludesFallbacks / PreservesUserOrder /
Dedups / EmptyUser pin the new helper's behaviour.
- TestConnectRelaysSkipsDisconnectedRelays exercises the
defensive skip path with an unresolvable host.
hoot.go's OnInitQR now calls nip46.PairingRelays(getRelayList())
instead of getRelayList() directly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After PR #73 (refuse to dial through inherited
*_PROXYenv vars), the user reported a second failure mode:The proxy fix correctly unblocks the initial sign-in dial. The new failure is in the post-pairing subscribe/recv path: Amber approves the QR, publishes the connect ack to one of the relays in the URI, but hoot never sees the event.
Root cause
The user's
relays.txtcontains:Neither
purplerelay.comnornos.lolare relays Amber is willing to publish to, and the user's list omitswss://nostr.wine(Amber's documented default bunker relay). Amber's published fallback chain iswss://relay.damus.io→wss://nostr.band→wss://nostr.wine. If hoot's subscription is on none of those (because the user's relays.txt made the dial fail or be skipped), the connect ack arrives at a relay hoot isn't listening on.Fix
nip46.PairingRelays(userRelays)always merges a hard-coded fallback set (wss://relay.damus.io,wss://nostr.wine) into whatever the user configured, dedup'd, preserving user order. Both thenostrconnect://URI and hoot'sSession.RelayURLsuse this combined list, so:Also:
ConnectRelaysnow skips relays whose connection died between dial and subscribe (IsConnected() == false). Previously we'd subscribe against a closed relay, producing a never-firing event channel that fooledCheckConnectioninto "no events yet" forever.CheckConnectionnow distinguishes "all subscriptions closed" from "no events yet" — the former returns an actionable error pointing at the network so users don't re-scan fruitlessly.Tests
TestPairingRelaysIncludesFallbacks/PreservesUserOrder/Dedups/EmptyUserpin the new helper.TestConnectRelaysSkipsDisconnectedRelaysexercises the defensive skip path with an unresolvable host.All existing tests still pass (4 master + 11 PR#73 + 5 new = 20 nip46 tests).
CI: 7/7 (test + 6 cross-platform builds).