[Kukks]

Follow-up to #1090: consolidate the ad-hoc Nostr usages (one-off pools, a hook living in the Settings screen, dead comparison helper) into one shared service. No UI or discovery-behavior change.

What this does

• \src/lib/solverCardRemote.ts\ becomes the single home for live-card reads: one app-lifetime pool, one shared transport fetcher (so its last-known-card memory survives across checks), and in-flight dedup so concurrent checks for the same solver share one relay round-trip.
• New \src/hooks/useRemoteSolverCard.ts: the detail-view hook moved out of \Solvers.tsx\ into the hooks folder, unchanged behavior.
• \Solvers.tsx\ drops ~40 lines and just consumes the hook; \swapMarkets.ts\ wiring untouched (already on the shared reader).
• Removes the now-unused \isSameCardContent\ comparator (nothing reads it since the Original/Current switch replaced the staleness derivation).

Validation

• 50 tests pass (incl. new: singleton-reader identity, shared in-flight read, re-read after settle).
• \ sc --noEmit, eslint, Prettier, and the repo pre-commit hooks green.
• No e2e impact expected (no UI change); existing detail-view e2e covers the flow.

Notes

• Stacked on Solver cards: detail view with original/current switch; watch bypasses market cache #1090 — retarget this PR to \master\ once Solver cards: detail view with original/current switch; watch bypasses market cache #1090 merges.
• Rollback: revert this one commit; the previous branch state is untouched.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 85cae4b4-18a3-47ad-b38b-4e5b6e5e760a

📥 Commits

Reviewing files that changed from the base of the PR and between e8675e2 and 31a72be.

📒 Files selected for processing: 9

• src/components/FlexRow.tsx
• src/hooks/useRemoteSolverCard.ts
• src/lib/nostr.ts
• src/lib/solverCardRemote.ts
• src/providers/notifications.tsx
• src/screens/Settings/Solvers.tsx
• src/test/lib/nostr.test.ts
• src/test/lib/solverCardRemote.test.ts
• src/test/lib/swapMarkets.test.ts

+-----------------------------------------------------------+
| GPU shortage? The only thing I fear is a carrot shortage. |
+-----------------------------------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cloudflare-workers-and-pages]

Deploying wallet-mutinynet with    Cloudflare Pages

Latest commit:	e12c6a6
Status:	✅  Deploy successful!
Preview URL:	https://c31f5bf1.arkade-wallet.pages.dev
Branch Preview URL:	https://refactor-nostr-card-service.arkade-wallet.pages.dev

View logs

[cloudflare-workers-and-pages]

Deploying wallet-bitcoin with    Cloudflare Pages

Latest commit:	e12c6a6
Status:	✅  Deploy successful!
Preview URL:	https://3c6bd06d.wallet-bitcoin.pages.dev
Branch Preview URL:	https://refactor-nostr-card-service.wallet-bitcoin.pages.dev

View logs

[arkana-ai-bot]

Arkana review — PR #1092

Clean infrastructure refactor. No VTXO, signing, forfeit, round, or exit paths touched; no protocol-critical review required. All findings are low-to-minor severity.

---

✅ What's correct

notifications.tsx dead-relay removal — the relay.current connected to wss://relay.primal.net on every nostrBackup toggle but was never subscribed to or published through (all notification functions call sendNotification, not the relay). Removing it stops a silent wasteful WebSocket that served no purpose.

Inflight dedup ordering (src/lib/solverCardRemote.ts:50–71) — The async IIFE starts executing synchronously up to the first await; control returns to the outer function, which calls inflight.set(key, task) before any microtask can run. Since fetch() is always async (network I/O), the finally { inflight.delete(key) } always fires as a microtask — after set. The ordering is safe under JS's single-threaded event loop. The try/finally guarantees no map leak on rejection. ✓

Pool singleton migration (src/lib/nostr.ts:13–23) — Moving SimplePool to sharedNostrPool() is correct. SimplePool multiplexes over relay URLs and is designed for concurrent subscriptions across different filter sets (backup uses kind 4 on the default relay list; solver-card discovery uses kind 38859 on solver-supplied relays). No cross-contamination possible.

isSameCardContent removal — grep across all repo clones finds zero consumers; safe to drop.

NostrStorage constructor (src/lib/nostr.ts:32–35) — The pool?: SimplePool second parameter is additive and backward-compatible; backup.tsx calls new NostrStorage(seckey) without a second argument and continues to work unchanged.

---

⚠️ Minor issues

src/hooks/useRemoteSolverCard.ts:17 — card object identity in useEffect deps

useEffect(() => {
  ...
}, [card, network])

If the caller re-creates the card object on every render (e.g. an inline object literal or a derived value from a selector), this effect re-fires on every render — potentially spamming the relay. The call site in Solvers.tsx passes input.card as Card from props, which is stable, so this isn't a regression from the inlined useRemoteCard. Still worth a comment near the hook signature warning that card must be a stable reference (memoised or from a stable store slice), since the hook is now a public-ish API sitting in the hooks folder rather than a local implementation detail.

src/hooks/useRemoteSolverCard.ts:11 — card: unknown in has-remote state

| { status: 'has-remote'; card: unknown }

Pre-existing from the Solvers.tsx inline version; not a regression. But now that this is an exported hook, the opaque unknown forces every consumer to cast. Worth typing as Card or at minimum Record<string, unknown> in a follow-up — callers downstream of the detail view (e.g. market-line diffing) will need to cast anyway.

src/lib/solverCardRemote.ts:50 — key construction

const key = network + ':' + rendezvous.pubkey

network values today are 'bitcoin' | 'regtest' | 'signet' and pubkeys are 64 hex chars, so no collision risk in practice. A structural key (e.g. ${network}/${rendezvous.pubkey}) or a comment noting the expected input shape would make future readers more confident.

---

🧪 Test coverage

• Singleton-reader identity, shared in-flight read, re-read after settle: ✓ all covered.
• NostrStorage injected-pool path: ✓ covered.
• The 'error' branch of useRemoteSolverCard (.catch path) has no dedicated test. Low risk since the path is trivially setState({ status: 'error' }), but a single rejection test would complete the matrix.

---

Cross-repo impact

No public API in @arkade-os/solver-discovery, SDK packages (ts-sdk, go-sdk, rust-sdk, dotnet-sdk), or ark changes. The removed isSameCardContent export has no consumers across any repo clone. No action needed.

---

Overall: approve once the card reference-stability caveat is documented (or addressed) on useRemoteSolverCard. Everything else is either correct or a follow-up quality item.

[bordalix]

@arkana-ai-bot review

[arkana-ai-bot]

Rate limited — I'll pick this up when capacity returns (attempt 1 of 8).

[arkana-ai-bot]

Arkana review — PR #1092 update (commit de59c9f)

Following up on @bordalix's push. The previous review covered the first two commits; this covers only what changed in de59c9f.

---

What changed

FlexRow.tsx — wrap prop removed cleanly. It was only consumed by the two <FlexRow end wrap> sites in Solvers.tsx, which this same commit updates. No other callers; prop is gone without residue.

Solvers.tsx — UI polish, no logic change:

• MarketLines switched to DetailRow — consistent with the rest of the card detail view. Fine.
• Relay list: <DetailRow key={relay} label={idx === 0 ? 'Relays' : ''} value={relay} /> — empty-string label on non-first rows is an intentional alignment pattern since DetailRow takes label: string. Renders correctly.
• <Text bold> for markets label (was <Text small bold>) — sizing fix, fine.
• Raw toggle: <div onClick> → <Button> — accessibility improvement, correct.

CardLine.handleShowInfo — Behaviour change worth noting:

// was:
<Button onClick={() => setShowInfo(true)} text={t('solvers.viewInfo')} />

// now:
const handleShowInfo = () => {
  setShowInfo((e) => !e)  // toggle
  setShowEditor(false)    // close editor when info opens
}

Two things here:

1. Toggle is correct — prevents a stranded open info panel when you want to dismiss it without scrolling.
2. setShowEditor(false) closes an open editor when info opens — good, prevents two panels competing for the same space.
3. Minor: the button label is always t('solvers.viewInfo') regardless of whether the info panel is currently open. When the panel is visible, clicking hides it, but the label doesn't communicate that. Not blocking — the toggle pattern is still usable — but a conditional showInfo ? t('solvers.hideInfo') : t('solvers.viewInfo') would make intent explicit. Follow-up quality item.

BundledCardLine still uses setShowInfo(true) directly (not a toggle) — that's correct; it has no editor state to close, and the behaviour parity difference is intentional.

---

Still open from prior review

These were flagged as follow-up items before; they remain unchanged:

• src/hooks/useRemoteSolverCard.ts:11 — card: unknown in has-remote state; worth typing as Card for consumers.
• src/hooks/useRemoteSolverCard.ts:17 — card dependency needs a stability note on the hook signature.
• useRemoteSolverCard error-path rejection test still missing.

None are blocking for this PR.

---

This commit is clean. Nothing protocol-touching. The prior review's overall assessment stands: fine to approve once the card reference-stability caveat is documented on the hook.

[arkana-ai-bot]

Arkana follow-up — PR #1092 (re-trigger at de59c9f)

Status: 0 new commits since my last pass (already reviewed de59c9f in the comment above). 3 prior minor items remain open; all non-blocking follow-up quality work. No new findings.

---

Prior-issue closure

#	Finding	Status
1	src/hooks/useRemoteSolverCard.ts:17 — card reference-stability caveat undocumented on hook signature	Still open — hook unchanged. Non-blocking; current call site in Solvers.tsx passes a stable reference.
2	src/hooks/useRemoteSolverCard.ts:11 — has-remote state types card: unknown; should be Card	Still open — unchanged. Non-blocking follow-up.
3	useRemoteSolverCard error-path (.catch) has no dedicated rejection test	Still open — unchanged. Low risk (path is trivially setState({ status: 'error' })).

de59c9f (committed by @bordalix) introduced FlexRow wrap prop removal and Solvers.tsx UI polish — covered in full in the review directly above. Nothing protocol-touching on this branch.

No action needed on this re-trigger; the prior review stands.

[arkana-ai-bot]

PROTOCOL-CRITICAL: human review required.

Status since de59c9f: 11 new commits; 0 of the 4 prior open items are closed; 1 new finding is protocol-relevant.

---

Prior-issue closure

#	Finding	Status
1	src/hooks/useRemoteSolverCard.ts:17 — card reference-stability caveat undocumented	Still open — hook unchanged. Non-blocking; current call site passes a stable reference.
2	src/hooks/useRemoteSolverCard.ts:11 — has-remote state types card: unknown	Still open — unchanged. Non-blocking follow-up.
3	useRemoteSolverCard .catch path has no dedicated rejection test	Still open — new tests in solvers.test.tsx cover no-relays, not-found, and has-remote but none sets mockFetchTransportCards.mockRejectedValue(...) to exercise the 'error' branch.
4	solvers.viewInfo button label unchanged when info panel is toggled open	Still open — handleShowInfo toggles state but label stays t('solvers.viewInfo') regardless. Non-blocking.

---

New findings

Protocol-critical — src/wallet-service-worker.ts:20,33

Commit 46c16dbf wires IndexedDBIntentRepository into MessageBus:

const intentRepository = new IndexedDBIntentRepository()
const worker = new MessageBus(walletRepository, contractRepository, {
  ...
  intentRepository,
})

The SDK MessageBus option docstring is explicit: "without it the worker persists no settlement intent, reconciles none on restart, and counts intent-locked VTXOs as available."

Counting intent-locked VTXOs as available after a worker restart is a correctness hazard — the wallet could attempt to spend a VTXO already committed to an in-flight settlement. This change is the fix. Why it needs explicit sign-off:

• Intent reconciliation on restart alters which VTXOs the wallet presents as spendable. Even a correct persistence layer can surface edge cases (e.g., stale intents from a failed round that was never cleaned up server-side).
• No wallet-repo test exercises restart → intent-reconcile → VTXO availability. Coverage exists only in ts-sdk.
• This is a first-time feature addition to the VTXO availability path in the service worker.

Mechanically correct (single instance, passed as the optional field). Flagging for sign-off, not rejection.

---

Minor — src/lib/solverCardRemote.ts:58 — as never type escape

The partial card literal passed to fetch() uses as never rather than as Card[]. as never is a stronger escape — it suppresses the down-cast diagnostic TypeScript emits for narrower assertions, so a future fetch() API shape change will produce no compile-time signal here. Prefer as Card[] or align the partial type to the actual parameter.

---

Nit — hardcoded strings bypassing i18n (src/screens/Settings/Solvers.tsx:153,207)

fieldRelays is added to i18n.ts in this PR and listed in the parity-test exception list, but line 207 uses the literal 'Relays' instead of t('solvers.fieldRelays'). 'Pair' at line 153 has no i18n key at all. Both are language-consistent in practice; follow-up quality item to route them through t().

---

Incremental additions assessed clean

• CardDetails original/current toggle — guard on remoteCard !== null prevents the segmented control rendering without data; shownCard correctly falls back to the local card when view === 'original' or remote is absent.
• watchSwapMarkets useCache: false — correct; prevents a double-stale discovery window on long-lived tabs.
• transportCards: sharedTransportCards() in discoveryOptions — singleton is safe; always returns the same object.
• Nostr pool refactor / dead-relay removal — matches what was assessed in the initial review.
• New tests in solverCardRemote.test.ts and swapMarkets.test.ts — in-flight dedup test uses Promise.all and verifies a single transport call; watchSwapMarkets cache-bypass assertion is precise.

---

Overall: Nostr pool refactor and solver-card detail UI are in good shape. The IndexedDBIntentRepository wiring is mechanically correct but touches the VTXO availability path — needs explicit sign-off from whoever owns the settlement lifecycle. Four prior minor items remain open non-blocking follow-up work.