From 77c0b27cb0c0ce5539b63b1ddc1772332f7898a2 Mon Sep 17 00:00:00 2001 From: oth-body Date: Mon, 14 Sep 2026 18:56:56 -0400 Subject: [PATCH] fix(nip46): connect relays before showing QR, retry polling for signer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The NIP-46 QR login was broken in two ways: 1. Relay dial + subscribe happened inside OnCheckQR (after the QR was already displayed). That meant a 10-15 second dial delay after the user scanned, during which the signer's connect event could arrive before the subscription was active — and be missed. The screen would stay on 'Waiting for connection...' forever. 2. checkQRConnection in the TUI ran exactly once. If OnCheckQR returned an error (timeout, refused, etc.), the function returned nil and nothing ever re-fired the check. The user was stuck. Fix: nip46/nip46.go: - New ConnectRelays(ctx): dials every configured relay in parallel and subscribes for kind 24133 events immediately. Called from OnInitQR so subscriptions are active before the QR is shown. - New CheckConnection(timeout): short-polls the merged event channel. Returns (pubkey, nil) on success or ('', error) if nothing arrived yet. Designed for repeated calling from the TUI. - Removed WaitForConnection (replaced by the above split). hoot.go: - OnInitQR now calls session.ConnectRelays(ctx) before returning the URI. By the time the user sees the QR, subscriptions are already listening on every healthy relay. - OnCheckQR calls session.CheckConnection(3s) — a short poll. On error it returns the error; the TUI retries. tui/tui.go: - checkQRConnection now returns qrRetryMsg on error instead of nil. - New qrRetryMsg / checkQRTickMsg types. Update() handles qrRetryMsg by scheduling a tea.Tick(1s) that fires checkQRTickMsg, which re-runs checkQRConnection. This creates a polling loop that keeps trying until the signer connects or the user presses Esc. --- hoot.go | 41 +++--- nip46/nip46.go | 386 +++++++++++++++++++------------------------------ tui/tui.go | 27 ++++ 3 files changed, 200 insertions(+), 254 deletions(-) diff --git a/hoot.go b/hoot.go index 3ccf0e8..24b2d09 100644 --- a/hoot.go +++ b/hoot.go @@ -1425,20 +1425,20 @@ func main() { return os.WriteFile(relayPath, []byte(data), 0600) }, OnInitQR: func() (string, error) { - // Initialize NIP-46 session against ALL configured relays. - // If we pick just one and it's down at scan-time, the user - // gets a websocket error from the very first connect attempt - // (this was the previous behavior). With a list, the QR URI - // advertises every one of them and WaitForConnection dials - // them in parallel — whichever the remote signer publishes - // through is the one hoot reads the response on. getRelayList - // already falls back to defaultRelays when there's no - // user-defined relays.txt. + // Generate NIP-46 URI with ALL configured relays and + // immediately dial + subscribe on them. By the time the + // user scans, the subscriptions are active and we won't + // miss the signer's connect event. relays := getRelayList() uri, session, err := nip46.GenerateConnectURI(relays, "Hoot") if err != nil { return "", err } + // Dial and subscribe in the background of this cmd. + ctx := context.Background() + if err := session.ConnectRelays(ctx); err != nil { + return "", fmt.Errorf("relay connect: %w", err) + } nip46Session = session return uri, nil }, @@ -1446,21 +1446,22 @@ func main() { if nip46Session == nil { return "", fmt.Errorf("session not initialized") } - // Wait for connection - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) - defer cancel() - - if err := nip46Session.WaitForConnection(ctx); err != nil { - return "", err + // Short poll — the subscriptions are already active + // from ConnectRelays called in OnInitQR. We just + // check if the signer's connect event arrived yet. + pubKey, err := nip46Session.CheckConnection(3 * time.Second) + if err != nil { + return "", err // TUI will retry } - // Get public key - pubKey, err := nip46Session.GetPublicKey(ctx) + // Got connect — now request the user's public key. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + userPubKey, err := nip46Session.GetPublicKey(ctx) if err != nil { - return "", err + return pubKey, nil // fall back to signer pubkey } - - return pubKey, nil + return userPubKey, nil }, // Profile callbacks OnListProfiles: func() ([]tui.ProfileInfo, string, error) { diff --git a/nip46/nip46.go b/nip46/nip46.go index 54da0fe..a29607e 100644 --- a/nip46/nip46.go +++ b/nip46/nip46.go @@ -14,14 +14,6 @@ import ( ) // Session represents an active NIP-46 connection. -// -// NIP-46 lets the client advertise multiple relays in its -// nostrconnect:// URI and lets the remote signer pick whichever -// it can reach. To match that, hoot now dials all advertised -// relays in parallel and considers a signer's connect event -// received on ANY of them. Earlier versions picked the first -// configured relay only, which produced a websocket error when -// that single relay was down — even if others were healthy. type Session struct { ClientPrivateKey string ClientPublicKey string @@ -29,17 +21,17 @@ type Session struct { RelayURLs []string UserPublicKey string - relays []*nostr.Relay // connected relays, one per RelayURL + relays []*nostr.Relay + merged chan *nostr.Event // merged subscription events from all relays + cancel context.CancelFunc } -// Request represents a NIP-46 JSON-RPC request type Request struct { ID string `json:"id"` Method string `json:"method"` Params []interface{} `json:"params"` } -// Response represents a NIP-46 JSON-RPC response type Response struct { ID string `json:"id"` Result string `json:"result,omitempty"` @@ -47,16 +39,7 @@ type Response struct { } // GenerateConnectURI creates a nostrconnect:// URI for QR code display. -// -// The URI advertises every URL in relayURLs as a separate `relay=` -// query parameter, per NIP-46 § "Signers MAY publish to any of the -// relays communicated in the initial URI". A remote signer (Amber, -// etc.) will pick whichever it can reach, so we get resilience when -// one advertised relay is offline. If relayURLs is empty, fall back -// to a single placeholder "wss://relay.damus.io" — historically the -// only choice and still the most likely-to-succeed default. func GenerateConnectURI(relayURLs []string, appName string) (uri string, session *Session, err error) { - // Generate ephemeral client keypair clientSK := nostr.GeneratePrivateKey() clientPK, err := nostr.GetPublicKey(clientSK) if err != nil { @@ -74,7 +57,6 @@ func GenerateConnectURI(relayURLs []string, appName string) (uri string, session RelayURLs: relays, } - // Build URI: nostrconnect://?relay=&relay=&...&metadata= metadata := map[string]string{"name": appName} metadataJSON, _ := json.Marshal(metadata) @@ -88,8 +70,6 @@ func GenerateConnectURI(relayURLs []string, appName string) (uri string, session return uri, session, nil } -// dedupStrings returns a copy of ss with duplicates removed, -// preserving first-seen order. func dedupStrings(ss []string) []string { seen := make(map[string]struct{}, len(ss)) out := make([]string, 0, len(ss)) @@ -103,100 +83,51 @@ func dedupStrings(ss []string) []string { return out } -// dialRelays connects to all configured relays in parallel and -// returns the successfully connected relays. The first dial error -// from each URL is logged-and-skipped rather than failing the whole -// session: a single flaky relay shouldn't block login when others -// work. -// -// NIP-46 spec compliance: we listen on every relay we advertised; -// any of them becoming a valid conduit for the signer's connect -// event is enough for the handshake to succeed. This matches what -// Amber et al. actually do — the signer picks the relay it can -// reach and publishes its response there. We just have to be -// listening on the right ones. -func (s *Session) dialRelays(ctx context.Context, perRelayTimeout time.Duration) []*nostr.Relay { - type result struct { - relay *nostr.Relay - err error - url string - } - results := make(chan result, len(s.RelayURLs)) - var wg sync.WaitGroup - for _, url := range s.RelayURLs { - wg.Add(1) - go func(url string) { - defer wg.Done() - dialCtx, cancel := context.WithTimeout(ctx, perRelayTimeout) - defer cancel() - relay, err := nostr.RelayConnect(dialCtx, url) - results <- result{relay: relay, err: err, url: url} - }(url) - } - wg.Wait() - close(results) - - var connected []*nostr.Relay - for r := range results { - if r.err == nil && r.relay != nil { - connected = append(connected, r.relay) - // Best-effort: silently skip failed dials so a single - // flaky relay doesn't block login when others work. - continue - } - _ = r - } - return connected -} - -// WaitForConnection waits for the remote signer to connect on any of -// the configured relays. It dials every advertised relay in -// parallel, subscribes for kind 24133 events addressed to our -// client pubkey on each, and returns as soon as ANY of them sees -// the signer's connect message. -func (s *Session) WaitForConnection(ctx context.Context) error { - const dialTimeout = 15 * time.Second +// ConnectRelays dials every configured relay in parallel and +// subscribes for NIP-46 kind 24133 events addressed to our client +// pubkey. Call this from OnInitQR (while generating the QR) so +// that by the time the user scans, the subscriptions are already +// active. Errors on individual relays are skipped; only a total +// failure (zero relays connected) returns an error. +func (s *Session) ConnectRelays(ctx context.Context) error { + const dialTimeout = 10 * time.Second + // Dial s.relays = s.dialRelays(ctx, dialTimeout) if len(s.relays) == 0 { - return fmt.Errorf("could not connect to any configured relay (tried: %s)", + return fmt.Errorf("could not connect to any relay (tried: %s)", strings.Join(s.RelayURLs, ", ")) } - // Subscribe for kind 24133 events addressed to us on every relay. + // Subscribe on every connected relay; merge event channels. + subCtx, cancel := context.WithCancel(ctx) + s.cancel = cancel + filter := nostr.Filter{ Kinds: []int{24133}, Tags: nostr.TagMap{"p": []string{s.ClientPublicKey}}, } type relaySub struct { - relay *nostr.Relay - events chan *nostr.Event - sub *nostr.Subscription - closeFn func() + events chan *nostr.Event + close func() } - subs := make([]*relaySub, 0, len(s.relays)) + var subs []*relaySub for _, relay := range s.relays { - sub, err := relay.Subscribe(ctx, nostr.Filters{filter}) + sub, err := relay.Subscribe(subCtx, nostr.Filters{filter}) if err != nil { continue } - rs := &relaySub{ - relay: relay, - events: sub.Events, - sub: sub, - closeFn: sub.Close, - } - subs = append(subs, rs) - defer rs.closeFn() + subs = append(subs, &relaySub{events: sub.Events, close: sub.Close}) } if len(subs) == 0 { + cancel() s.Close() - return fmt.Errorf("no relay accepted our subscription; tried %d relays", len(s.RelayURLs)) + return fmt.Errorf("no relay accepted our subscription") } - // Merge event streams from every relay; first to deliver wins. - merged := make(chan *nostr.Event, 64) + // Merge into s.merged so CheckConnection can poll it. + s.merged = make(chan *nostr.Event, 64) var wg sync.WaitGroup for _, rs := range subs { wg.Add(1) @@ -204,26 +135,42 @@ func (s *Session) WaitForConnection(ctx context.Context) error { defer wg.Done() for ev := range rs.events { select { - case merged <- ev: - case <-ctx.Done(): + case s.merged <- ev: + case <-subCtx.Done(): return } } }(rs) } - defer func() { - // Stop forwarding goroutines when we return. - go func() { - wg.Wait() - close(merged) - }() + // Background cleanup: when subCtx is cancelled the forwarders + // exit and we close merged. + go func() { + wg.Wait() + close(s.merged) }() - // Wait for connection event (with timeout). - timeout := time.After(60 * time.Second) + return nil +} + +// CheckConnection polls for a signer connect event with a short +// timeout. Returns the signer's pubkey on success, or an error if +// nothing arrived yet. Callers should retry until success or a +// hard timeout. This is meant to be called from the TUI's +// OnCheckQR in a polling loop. +func (s *Session) CheckConnection(timeout time.Duration) (string, error) { + if s.merged == nil { + return "", fmt.Errorf("not connected — call ConnectRelays first") + } + + timer := time.NewTimer(timeout) + defer timer.Stop() + for { select { - case ev := <-merged: + case ev, ok := <-s.merged: + if !ok { + return "", fmt.Errorf("subscription closed") + } if ev == nil { continue } @@ -240,28 +187,48 @@ func (s *Session) WaitForConnection(ctx context.Context) error { // Parse response var resp Response if err := json.Unmarshal([]byte(decrypted), &resp); err != nil { - // Might be a connect acknowledgement; treat any - // decryptable event from a peer as the handshake. + // Treat any decryptable event as connect ack. s.SignerPublicKey = ev.PubKey - return nil + return ev.PubKey, nil } - - // If we got a result, connection is established if resp.Result != "" || resp.Error == "" { s.SignerPublicKey = ev.PubKey - return nil + return ev.PubKey, nil } - case <-timeout: - s.Close() - return fmt.Errorf("connection timeout after trying relays: %s", - strings.Join(s.RelayURLs, ", ")) + case <-timer.C: + return "", fmt.Errorf("no response yet") + } + } +} - case <-ctx.Done(): - s.Close() - return ctx.Err() +func (s *Session) dialRelays(ctx context.Context, perRelayTimeout time.Duration) []*nostr.Relay { + type result struct { + relay *nostr.Relay + err error + } + results := make(chan result, len(s.RelayURLs)) + var wg sync.WaitGroup + for _, u := range s.RelayURLs { + wg.Add(1) + go func(u string) { + defer wg.Done() + dialCtx, cancel := context.WithTimeout(ctx, perRelayTimeout) + defer cancel() + relay, err := nostr.RelayConnect(dialCtx, u) + results <- result{relay: relay, err: err} + }(u) + } + wg.Wait() + close(results) + + var connected []*nostr.Relay + for r := range results { + if r.err == nil && r.relay != nil { + connected = append(connected, r.relay) } } + return connected } // GetPublicKey requests the user's public key from the signer. @@ -276,7 +243,6 @@ func (s *Session) GetPublicKey(ctx context.Context) (string, error) { if err != nil { return "", err } - if resp.Error != "" { return "", fmt.Errorf("signer error: %s", resp.Error) } @@ -287,16 +253,13 @@ func (s *Session) GetPublicKey(ctx context.Context) (string, error) { // SignEvent requests the signer to sign an event func (s *Session) SignEvent(ctx context.Context, event *nostr.Event) error { - reqID := fmt.Sprintf("%d", time.Now().UnixNano()) - - // Serialize unsigned event eventJSON, err := json.Marshal(event) if err != nil { return err } req := Request{ - ID: reqID, + ID: fmt.Sprintf("%d", time.Now().UnixNano()), Method: "sign_event", Params: []interface{}{string(eventJSON)}, } @@ -305,85 +268,76 @@ func (s *Session) SignEvent(ctx context.Context, event *nostr.Event) error { if err != nil { return err } - if resp.Error != "" { return fmt.Errorf("signer error: %s", resp.Error) } - // Parse signed event from response var signedEvent nostr.Event if err := json.Unmarshal([]byte(resp.Result), &signedEvent); err != nil { return fmt.Errorf("failed to parse signed event: %w", err) } - // Copy signature to original event event.ID = signedEvent.ID event.Sig = signedEvent.Sig return nil } -// publishToAllRelays publishes an event to every connected relay. -// NIP-46 says the signer picks a relay to listen on; we don't know -// which one, so we publish to all of them. Publish is best-effort: -// one relay failing to publish should not block the others. -func (s *Session) publishToAllRelays(ctx context.Context, event *nostr.Event) error { - if len(s.relays) == 0 { - return fmt.Errorf("no relay connections available") +func (s *Session) sendRequest(ctx context.Context, req Request) (*Response, error) { + if s.SignerPublicKey == "" { + return nil, fmt.Errorf("not connected to signer") } - errCh := make(chan error, len(s.relays)) - var wg sync.WaitGroup - for _, relay := range s.relays { - wg.Add(1) - go func(relay *nostr.Relay) { - defer wg.Done() - errCh <- relay.Publish(ctx, *event) - }(relay) + + reqJSON, err := json.Marshal(req) + if err != nil { + return nil, err } - wg.Wait() - close(errCh) - // Return the first non-nil error but don't fail completely — at - // least one relay probably succeeded and the signer may be - // listening on it. - for err := range errCh { - if err != nil { - return err - } + + sharedSecret, err := nip04.ComputeSharedSecret(s.SignerPublicKey, s.ClientPrivateKey) + if err != nil { + return nil, err + } + encrypted, err := nip04.Encrypt(string(reqJSON), sharedSecret) + if err != nil { + return nil, err } - return nil -} -// subscribeAllRelays subscribes for kind 24133 events addressed to -// us from the signer pubkey across every connected relay, and -// returns a merged event channel plus a close function. -// -// The signer chooses where to publish its response (any of the -// relays it could reach from our URI). To not miss the response, -// we listen on every relay we connected to and merge the streams. -func (s *Session) subscribeAllRelays(ctx context.Context, signerPubKey string, since nostr.Timestamp) (chan *nostr.Event, func(), error) { + now := nostr.Timestamp(time.Now().Unix()) + event := nostr.Event{ + PubKey: s.ClientPublicKey, + CreatedAt: now, + Kind: 24133, + Tags: nostr.Tags{{"p", s.SignerPublicKey}}, + Content: encrypted, + } + event.Sign(s.ClientPrivateKey) + + // Publish to every connected relay. + if err := s.publishToAllRelays(ctx, &event); err != nil { + return nil, fmt.Errorf("failed to publish request: %w", err) + } + + // Subscribe on every relay for the response. filter := nostr.Filter{ Kinds: []int{24133}, - Authors: []string{signerPubKey}, + Authors: []string{s.SignerPublicKey}, Tags: nostr.TagMap{"p": []string{s.ClientPublicKey}}, - Since: &since, + Since: &now, } type relaySub struct { events chan *nostr.Event close func() } - subs := make([]*relaySub, 0, len(s.relays)) + var subs []*relaySub for _, relay := range s.relays { sub, err := relay.Subscribe(ctx, nostr.Filters{filter}) if err != nil { continue } - subs = append(subs, &relaySub{ - events: sub.Events, - close: sub.Close, - }) + subs = append(subs, &relaySub{events: sub.Events, close: sub.Close}) } if len(subs) == 0 { - return nil, nil, fmt.Errorf("no relay accepted subscription") + return nil, fmt.Errorf("no relay accepted subscription for response") } merged := make(chan *nostr.Event, 64) @@ -401,71 +355,14 @@ func (s *Session) subscribeAllRelays(ctx context.Context, signerPubKey string, s } }(rs) } - - closeFn := func() { + closeSubs := func() { for _, rs := range subs { rs.close() } - // Let the forwarders drain before closing merged. We can't - // wg.Wait here because we're called from inside the request - // handler that uses merged; the merged channel will be GCed - // when the request loop exits. - go func() { - wg.Wait() - close(merged) - }() - } - return merged, closeFn, nil -} - -func (s *Session) sendRequest(ctx context.Context, req Request) (*Response, error) { - if s.SignerPublicKey == "" { - return nil, fmt.Errorf("not connected to signer") - } - - // Serialize request - reqJSON, err := json.Marshal(req) - if err != nil { - return nil, err - } - - // Encrypt - sharedSecret, err := nip04.ComputeSharedSecret(s.SignerPublicKey, s.ClientPrivateKey) - if err != nil { - return nil, err - } - encrypted, err := nip04.Encrypt(string(reqJSON), sharedSecret) - if err != nil { - return nil, err + go func() { wg.Wait(); close(merged) }() } + defer closeSubs() - // Create the request event with the current timestamp; ALL relays - // see the same event id so the signer can dedupe if it actually - // receives the same event on multiple relays. - now := nostr.Timestamp(time.Now().Unix()) - event := nostr.Event{ - PubKey: s.ClientPublicKey, - CreatedAt: now, - Kind: 24133, - Tags: nostr.Tags{{"p", s.SignerPublicKey}}, - Content: encrypted, - } - event.Sign(s.ClientPrivateKey) - - // Publish to every connected relay. Whichever the signer is - // listening on will deliver the request. - if err := s.publishToAllRelays(ctx, &event); err != nil { - return nil, fmt.Errorf("failed to publish request: %w", err) - } - - // Subscribe on every relay for the response. - merged, closeFn, err := s.subscribeAllRelays(ctx, s.SignerPublicKey, now) - if err != nil { - return nil, err - } - defer closeFn() - - // Wait for response timeout := time.After(30 * time.Second) for { select { @@ -473,33 +370,54 @@ func (s *Session) sendRequest(ctx context.Context, req Request) (*Response, erro if ev == nil { continue } - // Decrypt decrypted, err := nip04.Decrypt(ev.Content, sharedSecret) if err != nil { continue } - var resp Response if err := json.Unmarshal([]byte(decrypted), &resp); err != nil { continue } - - // Check if this is our response if resp.ID == req.ID || strings.HasPrefix(resp.Result, "{") || resp.Error != "" { return &resp, nil } case <-timeout: return nil, fmt.Errorf("request timeout") - case <-ctx.Done(): return nil, ctx.Err() } } } +func (s *Session) publishToAllRelays(ctx context.Context, event *nostr.Event) error { + if len(s.relays) == 0 { + return fmt.Errorf("no relay connections available") + } + errCh := make(chan error, len(s.relays)) + var wg sync.WaitGroup + for _, relay := range s.relays { + wg.Add(1) + go func(relay *nostr.Relay) { + defer wg.Done() + errCh <- relay.Publish(ctx, *event) + }(relay) + } + wg.Wait() + close(errCh) + for err := range errCh { + if err != nil { + return err + } + } + return nil +} + // Close closes every relay connection. func (s *Session) Close() { + if s.cancel != nil { + s.cancel() + } for _, r := range s.relays { if r != nil { r.Close() diff --git a/tui/tui.go b/tui/tui.go index 8d03371..5269661 100644 --- a/tui/tui.go +++ b/tui/tui.go @@ -3,6 +3,7 @@ package tui import ( "fmt" "strings" + "time" "github.com/charmbracelet/bubbles/spinner" "github.com/charmbracelet/bubbles/textinput" @@ -326,6 +327,19 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.messageStyle = lipgloss.Style{} // Start checking for connection in background return m, m.checkQRConnection + + case qrRetryMsg: + // The previous check didn't find a signer connect event yet. + // Re-fire after a short delay so we keep polling without + // blocking the TUI render loop. tea.Tick fires a callback + // after the duration that returns the Cmd to execute. + return m, tea.Tick(time.Second, func(t time.Time) tea.Msg { + return checkQRTickMsg{} + }) + + case checkQRTickMsg: + // Fired by the retry timer above — run another check. + return m, m.checkQRConnection } // Update text input @@ -612,10 +626,23 @@ func (m Model) checkQRConnection() tea.Msg { if err == nil && pubkey != "" { return qrSuccessMsg(pubkey) } + // No connection yet — return a retry message so the TUI + // re-fires this check after a short delay instead of giving + // up silently (the old behavior, which left the screen stuck + // on "Waiting for connection..." forever). + return qrRetryMsg{} } return nil } +// qrRetryMsg signals that the QR connection check should be +// retried after a short delay. +type qrRetryMsg struct{} + +// checkQRTickMsg is fired by tea.Tick after the retry delay, +// triggering another checkQRConnection round. +type checkQRTickMsg struct{} + func (m Model) handleHomeEnter() (tea.Model, tea.Cmd) { switch m.cursor { case 0: // Post