Repository navigation
feat: upgrade wallet to SDK 0.5 - #936
Merged
Merged
Conversation
Contributor
|
Important Review skippedThe saved review history does not include the base for the last reviewed commit. This saved history cannot establish the base for an incremental review. Comment You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThis PR migrates swap handling to a unified v2 swap client. It adds cross-tab coordination, routed Lightning/on-chain/asset payments, corridor activity history, on-chain claims, expanded transaction details, and related tests. ChangesUnified swap migration
Priority: ➖ Normal Estimated code review effort: 5 (Critical) | ~120 minutes Severity of issue fixed: Medium Suggested reviewers: Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 63 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check The changes address issue #934by updating SDK dependencies, replacing removed SDK APIs, migrating swap and wallet integrations, adapting transaction and send flows, and adding migration-focused tests…Out of Scope Changes check No clearly unrelated code changes are present. The provider migration, swap routing, on-chain claims, UI updates, workflow changes, and tests support the SDK 0.5 migration or the documented related in… Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely identifies the main change: upgrading the wallet to SDK 0.5.
✨ Finishing Touches 💡 1
-
📝 Generate docstrings 💡
-
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
-
- Create PR with unit tests
- Commit unit tests in branch
ts-sdk-0.5
-
Comment @coderabbitai help to get the list of available commands.
Deploying wallet-bitcoin with
|
| Latest commit: |
86df35b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fe294b67.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://ts-sdk-0-5.wallet-bitcoin.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
86df35b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0d23ced8.arkade-wallet.pages.dev |
| Branch Preview URL: | https://ts-sdk-0-5.arkade-wallet.pages.dev |
Contributor
|
This PR has been open 2+ days without review. @bordalix are there reviewers lined up for the ts-sdk 0.5 upgrade? |
Contributor
|
This PR has been open for 3+ days without review. @bordalix is anyone looking at this? (ts-sdk 0.5 upgrade) |
Contributor
|
This PR has been open for 5 days without review. @bordalix is anyone looking at ts-sdk 0.5? |
…ng send and receive Claude-Session: https://claude.ai/code/session_01W5xDq73G5jGA23XG8B9SLr
…s/swap/client Claude-Session: https://claude.ai/code/session_01W5xDq73G5jGA23XG8B9SLr
…t, v1 is /protocol Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR
Kukks
added a commit
that referenced
this pull request
Sep 7, 2026
#936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice.
Kukks
added a commit
that referenced
this pull request
Sep 7, 2026
#936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice.
Kukks
added a commit
that referenced
this pull request
Sep 7, 2026
#936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice.
Kukks
added a commit
that referenced
this pull request
Sep 7, 2026
#936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice.
Kukks
added a commit
that referenced
this pull request
Sep 7, 2026
#936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice.
Kukks
added a commit
that referenced
this pull request
Sep 7, 2026
#936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice.
Kukks
added a commit
that referenced
this pull request
Sep 7, 2026
#936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice.
…ord reading moves to /advanced Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR
…irst Supersedes #941. That PR shipped a solver-routed onchain send with ~2,600 lines of bespoke route selection, bounds matching and fallback — all of which `PaymentRouter` (ts-sdk #837) already does. This is the same corridor expressed as a rail registration. What the router replaces, and the proof it covers it: every arm of #941's `OnchainRouteRefusal` enum becomes `available()` returning false, and a rail that says no is dropped from `options()` without taking the router with it, so the collaborative exit wins by ranking. `sendRouter.test.ts` asserts one case per deleted arm — no_solver, amount_out_of_bounds, discovery_failed, unsupported_address, no_l1_endpoint — alongside a control proving the solver rail really can rank first, without which those five would pass vacuously. `chainSourceFrom` likewise replaces #941's 222-line hand-written esplora client. The wrong-address guard survives, differently. #941 carried a negotiated quote across the Form -> Details navigation and compared its `payoutAddress` to the screen; `RouteQuote` has no address field, so that comparison has no home. Instead nothing is carried: `PaymentOption.quote()` is lazy by contract, so the sign screen quotes against the address it is displaying at the moment of the spend, and a stale quote is not a thing that can exist. `quoteIsForThisSend` is the belt to that braces, and is stricter than the check it replaces — the screen must be showing the routed address, the recipient must get what was displayed, and no more may leave the wallet than was displayed. Two rails, and only one of them is the SDK's. `solverOnchainRail` comes from `@arkade-os/swap`; the collaborative exit stays this wallet's own `collaborativeExitWithFees` rather than the SDK's `onchainRail`, because that one settles through `Ramps.offboard` with its own coin selection while this wallet deliberately spends soonest-expiring VTXOs first. Adopting it is a coin-selection change, separate from adopting the router. Amounts are receiver-exact, as `RouteQuote` specifies, but the send form still lets the user type what LEAVES the wallet. The translation happens at the screen boundary rather than in the rails: a typed amount interpreted as receiver-exact would gross up past the balance it came from, so "send max" could never succeed. `isBTCAddress` now delegates to the SDK's `isBtcAddress`, so the form classifies a target exactly as `btcTarget` does. That changes which addresses the form accepts, in both directions, and the four cases are pinned in `address.test.ts`. `@arkade-os/swap` is vendored from ts-sdk `fa3a2d1f` — the #837 squash — because no published version carries the rails. See vendor/README.md for why the SDK did not need vendoring alongside it, and for the version-collision trap that makes the `pnpm.overrides` entry load-bearing.
The router and the guard had unit tests, but no test rendered `SendDetails` with a plain BTC address and pressed sign — `send-details.test.tsx` only covers the amount hierarchy. So the whole `payOnchain` path was green by absence, which is exactly what a suite cannot tell you. Five cases at the screen that moves the money: the exit is taken when no solver rail survived, a rail quoting a different recipient amount is refused, a rail that would spend more than the screen displayed is refused, a matching solver quote IS funded, and — the wrong-address regression from 1b3481b — the request is built from the address the screen is showing. The mocked `options()` takes the request and builds the exit rail from `req.raw` rather than a constant. Without that the last case asserts against a hardcoded address and proves nothing; mutating `raw` to a fixed address kills it, as does deleting the `quoteIsForThisSend` call.
Contributor
|
@coderabbitai review |
Contributor
|
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Status: 7 commits since c07b717 — GitHub Actions version bumps, a dismissible mobile "Open in App" banner, SegmentedControl keyboard accessibility, and animation-constant extraction. 0 of 16 prior findings are fixed; all 16 remain open. No new blocking findings from this increment.
Prior findings close-out
None of the 16 files carrying prior findings appear in this diff. All entries carry over unchanged.
| # | Sev | Finding | Status |
|---|---|---|---|
| 1 | High | src/lib/swapClient.ts — package-side audit of claimOnchainFill: whether a malformed solver fill can drive the function to sign an arbitrary sighash via crafted htlc/utxo parameters |
Still open — swapClient.ts not in this diff |
| 2 | High | RC packages: @arkade-os/sdk@0.5.0-rc.10, @arkade-os/swap@0.1.0-rc.13 still pre-release; onchain-claim behaviour can silently change between RC and final without a wallet-side diff |
Still open — package.json not touched |
| 3 | Medium | src/lib/swapRecords.ts / swapClient.ts — @arkade-os/swap/advanced subpath is not a stable export contract; needs re-check on every swap-package bump |
Still open — unchanged |
| 4 | Medium | src/lib/swapDriverChannel.ts — BroadcastChannel delegation: a tab that receives a drive message does not re-acquire the Web Lock before executing the swap step; two tabs can race through the same step if the lock-holder dies mid-message |
Still open — unchanged |
| 5 | Medium | Known mid-session spend miss (no regression guard test; escalated to package side) | Still open — unchanged |
| 6 | Medium | src/lib/swapRecords.ts — stale-quote acceptance in quoteIsForThisSend: validator checks amount and address but not quote timestamp or expiry |
Still open — unchanged |
| 7 | Medium | src/lib/activityHistory.ts — pending-offer history gap: ungroupedOfferTx fallback row removed; pending offer visibility now depends entirely on the SDK emitting a tag: 'gated' synthetic activity; three covering tests deleted |
Still open — unchanged |
| 8 | Low | src/lib/claimFee.ts:14 — v > MIN_CLAIM_FEE_RATE treats 1 sat/vB as unusable; comment frames the risk differently |
Still open — claimFee.ts unchanged |
| 9 | Low | src/providers/swaps.tsx — sendRouter calls driving() directly; non-driving tab gets the wrong error class, no grace period |
Still open — swaps.tsx unchanged |
| 10 | Low | src/providers/swaps.tsx — cancelSwapHere casts id as AssetSwapId before passing to client.cancel(); a runtime mismatch on an LN-receive ID is swallowed silently by .catch(consoleError) |
Still open — swaps.tsx unchanged |
| 11 | Low | src/lib/address.ts:35-45 — isBTCAddress accepts addresses across networks; cross-network acceptance |
Still open — unchanged |
| 12 | Low | src/lib/sendRouter.ts:122-130 — lnSendRefusal shows the union of solver ranges, not the intersection |
Still open — unchanged |
| 13 | Low | src/lib/swapClient.ts — session-scoped sealing key: covclaimd.pubkey is set once at client construction; per-receive key rotation is not performed |
Still open — swapClient.ts unchanged |
| 14 | Low | src/lib/onchainPayout.ts:8 — l1NetworkOf default arm is not union-exhaustive; new network values silently fall through |
Still open — unchanged |
| 15 | Low | src/screens/Wallet/Transaction.tsx:315-316 — showCompleteBoarding missing amountAboveDust guard present in showSettleButtons |
Still open — Transaction.tsx unchanged |
| 16 | Low | src/screens/Wallet/Send/Details.tsx — withinPricingBudget(priced).then(settle) tail: .catch was added in an earlier commit but file has not been confirmed stable through subsequent master merges |
Still open — Details.tsx unchanged |
Incremental diff assessment
CI: GitHub Actions version bumps (.github/workflows/*.yml)
actions/checkout v4→v7, actions/setup-node v4→v7, pnpm/action-setup v4→v6, actions/cache v3→v6. Standard maintenance. No concern.
src/components/SegmentedControl.tsx + src/test/components/SegmentedControl.test.tsx
<div> tabs replaced with <button role='tab'> inside <div role='tablist'>. aria-selected added. The change is a correct accessibility improvement and the new tests cover the semantics. One nit: the ARIA authoring practices for tablist specify that arrow keys (←/→) should move focus between tabs, and that individual tabs should not be part of the sequential Tab-key focus order (they should collectively be a single Tab stop). The current implementation makes each <button> independently Tab-focusable, which diverges from the pattern. Not a regression — it's better than before — but worth tracking as a follow-up a11y task.
src/screens/AppIntent/OpenInApp.tsx
Replaced bare <Button> + conditional <Text> with <DismissibleBanner>. Adds dismissed state (local, resets on unmount — correct for an ephemeral hint). Adds isMobile guard via useIsMobile(), so desktop users never see the banner. All logic paths are tested in app-intent.test.tsx including the new dismiss and desktop-hide cases.
Nit — afterEach viewport reset in app-intent.test.tsx:94-96: The test resets window.innerWidth to 1024 in afterEach. 1024 is above the 768 px breakpoint (MOBILE_BREAKPOINT) so the reset is effective, but using the explicit constant (MOBILE_BREAKPOINT + 1 or a named sentinel) would make the intent self-documenting. Minor.
src/hooks/use-mobile.ts
Initial state changed from undefined (coerced to false via !!isMobile) to mobileNow() (computed eagerly at mount). The mobileNow guard correctly short-circuits when window is undefined. The return type is now boolean directly rather than !!isMobile. This eliminates the first-render false-negative that previously hid the OpenInApp banner until the matchMedia effect fired. Correct.
src/screens/Wallet/Receive/QrCode.tsx
Only change is adding focusable to the button's class list and removing the now-CSS-managed inline styles (border: 'none', inline cursor). The .receive-invoice-qr rule in index.css overrides the default focusable border to transparent, preserving the previous visual. The focus-visible rule gives the button a visible keyboard-focus ring. No logic changed; the prior LN-receive race fix (cancelSwap on unmount) is untouched.
src/lib/animations.ts / useBounceMorph.ts / OnboardingLogo.tsx
Extraction of SQUASH and SPRING_BACK animation constants. Pure refactor; both consumers were previously duplicating the same values. No behavioural change.
src/lib/sleep.ts
Promise<void> explicit type annotation added. No behavioural change.
The two high findings (items 1 and 2 above) remain the merge blockers from prior passes. Everything in this increment is UI/CI work and is clean.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Status: 7 commits since c07b717 — GitHub Actions version bumps, a dismissible mobile "Open in App" banner, SegmentedControl keyboard accessibility, and animation-constant extraction. 0 of 16 prior findings fixed; all 16 remain open. No new blocking findings from this increment.
Prior findings close-out
None of the 16 files carrying prior findings appear in this diff. All entries carry over unchanged.
| # | Sev | Finding | Status |
|---|---|---|---|
| 1 | High | src/lib/swapClient.ts — package-side audit of claimOnchainFill: whether malformed solver fill can drive signing of an arbitrary sighash via crafted htlc/utxo |
Still open — swapClient.ts not in this diff |
| 2 | High | RC packages: @arkade-os/sdk@0.5.0-rc.10, @arkade-os/swap@0.1.0-rc.13 still pre-release; onchain-claim behaviour can change between RC and final without a wallet-side diff |
Still open — package.json not touched |
| 3 | Medium | src/lib/swapRecords.ts / swapClient.ts — @arkade-os/swap/advanced subpath is not a stable export contract |
Still open — unchanged |
| 4 | Medium | src/lib/swapDriverChannel.ts — BroadcastChannel delegation: receiving tab does not re-acquire the Web Lock before executing the swap step; two tabs can race through the same step |
Still open — unchanged |
| 5 | Medium | Known mid-session spend miss — no regression guard test | Still open — unchanged |
| 6 | Medium | src/lib/swapRecords.ts — stale-quote acceptance in quoteIsForThisSend: validates amount and address but not quote timestamp or expiry |
Still open — unchanged |
| 7 | Medium | src/lib/activityHistory.ts — pending-offer history gap: ungroupedOfferTx fallback row removed; three covering tests deleted |
Still open — unchanged |
| 8 | Low | src/lib/claimFee.ts:14 — v > MIN_CLAIM_FEE_RATE treats 1 sat/vB as unusable; comment frames the risk differently |
Still open — claimFee.ts unchanged |
| 9 | Low | src/providers/swaps.tsx — sendRouter calls driving() directly; non-driving tab gets the wrong error class, no grace period |
Still open — swaps.tsx unchanged |
| 10 | Low | src/providers/swaps.tsx — cancelSwapHere casts id as AssetSwapId before passing to client.cancel(); LN-receive ID mismatch swallowed silently |
Still open — unchanged |
| 11 | Low | src/lib/address.ts:35-45 — isBTCAddress accepts addresses across networks |
Still open — unchanged |
| 12 | Low | src/lib/sendRouter.ts:122-130 — lnSendRefusal shows union of solver ranges, not intersection |
Still open — unchanged |
| 13 | Low | src/lib/swapClient.ts — session-scoped sealing key; per-receive key rotation not performed |
Still open — swapClient.ts unchanged |
| 14 | Low | src/lib/onchainPayout.ts:8 — l1NetworkOf default arm not union-exhaustive |
Still open — unchanged |
| 15 | Low | src/screens/Wallet/Transaction.tsx:315-316 — showCompleteBoarding missing amountAboveDust guard present in showSettleButtons |
Still open — Transaction.tsx unchanged |
| 16 | Low | src/screens/Wallet/Send/Details.tsx — withinPricingBudget(priced).then(settle) tail catch status unconfirmed through subsequent master merges |
Still open — Details.tsx unchanged |
Incremental diff assessment
CI: GitHub Actions version bumps
actions/checkout v4→v7, actions/setup-node v4→v7, pnpm/action-setup v4→v6, actions/cache v3→v6. Standard maintenance. No concern.
src/components/SegmentedControl.tsx + new test
<div> tabs replaced with <button role='tab'> inside <div role='tablist'>. aria-selected added. The change is a correct accessibility improvement and the new unit tests cover semantics adequately. Nit: the ARIA tablist authoring pattern specifies arrow-key navigation between tabs and a single Tab-key stop for the group. The current implementation makes each <button> individually Tab-focusable, which diverges from the pattern. Not a regression; worth tracking as a follow-up a11y task.
src/screens/AppIntent/OpenInApp.tsx
Replaced bare button + conditional text with <DismissibleBanner>. Adds local dismissed state (resets on unmount — correct for an ephemeral hint). Adds isMobile guard so desktop users never see the banner. All paths tested in app-intent.test.tsx.
Nit — app-intent.test.tsx:94-96: afterEach resets window.innerWidth to 1024. The value is above MOBILE_BREAKPOINT (768) and is effective, but using a named sentinel or MOBILE_BREAKPOINT + 1 would make the intent self-documenting. Minor.
src/hooks/use-mobile.ts
Initial state changed from undefined (coerced via !!isMobile) to mobileNow() (computed eagerly at mount). The guard typeof window !== 'undefined' makes the SSR case safe. Eliminates the first-render false-negative that previously hid the OpenInApp banner until the matchMedia effect fired. Correct improvement.
src/screens/Wallet/Receive/QrCode.tsx
Adds focusable to the QR button's class list; removes inline border: 'none' (now delegated to CSS as border: 1px solid transparent via the .receive-invoice-qr override in index.css). The focus-visible rule gives the button a visible keyboard-focus ring. The prior LN-receive race fix (cancelSwap on unmount) is untouched.
src/lib/animations.ts / useBounceMorph.ts / OnboardingLogo.tsx
Extraction of SQUASH and SPRING_BACK animation constants from duplicated inline objects. Pure refactor; no behavioural change.
src/lib/sleep.ts
Explicit Promise<void> annotation added. No behavioural change.
Items 1 and 2 (High — RC packages and package-side claimOnchainFill audit) remain the merge blockers. Everything in this increment is UI/CI work and is clean.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
Test review submission check
Contributor
|
@coderabbitai review |
Contributor
|
arkana-ai-bot
requested changes
Oct 15, 2026
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
2 commits since a79ef199 — atomicToUnits/unitsToAtomic rename + missing-decimals default flip (master sync). 2 of 16 prior findings are now confirmed fixed; 14 remain open. No new findings from this increment.
Prior findings — close-out
Findings 6 and 7 were fixed between the last Arkana review (2026-09-24, 206fb882) and the stated baseline a79ef199. They were not in the incremental diff this pass, but I verified the current state.
Finding 6 — sendRouter calls driving() directly (wrong error class, no grace period): Fixed. src/providers/swaps.tsx now correctly guards sendRouter:
if (!held.current && granted.current) {
await Promise.race([granted.current, new Promise(resolve => setTimeout(resolve, LOCK_GRACE_MS))])
}
if (!held.current) throw new SwapsHeldElsewhere()
const client = await driving()Grace-period wait is present; throws SwapsHeldElsewhere (not a plain Error) after the wait. Both defects from prior review are resolved.
Finding 7 — withinPricingBudget(priced).then(settle) — no catch on tail: Fixed (was first confirmed fixed in e2c03b33, incorrectly carried as open in later reviews). Current Details.tsx has .catch((err) => consoleError(err, ...)).
All remaining 14 open findings are in files not touched by the incremental diff. Status unchanged:
| # | Severity | Finding | Status |
|---|---|---|---|
| 1 | High | swapClient.ts — claimOnchainFill package-side signing audit: whether a malformed solver fill can drive the function to sign an arbitrary sighash via crafted htlc/utxo |
Still open — src/lib/swapClient.ts not in this diff |
| 2 | High | RC packages: @arkade-os/sdk@0.5.0-rc.22, @arkade-os/swap@0.1.0-rc.25 still pre-release; onchain-claim behaviour can change between RC and final without a wallet-side diff |
Still open — versions bumped further (rc.10/rc.13 → rc.22/rc.25); the API-stability risk is unchanged |
| 3 | Medium | @arkade-os/swap/advanced subpath not a stable export contract (src/lib/swapRecords.ts, src/lib/swapClient.ts) |
Still open — unchanged |
| 4 | Medium | Known mid-session spend miss — no regression guard test | Still open — unchanged |
| 5 | Low | claimFeeRate comment ambiguity (src/lib/claimFee.ts:14) |
Still open — unchanged |
| 8 | Low | cancelSwapHere type-brand cast on LN-receive ID (src/providers/swaps.tsx) |
Still open — cast still present |
| 9 | Low | CORRIDOR_LABEL CAIP-19 forward-compat gap (src/hooks/useCorridorSendReceipt.ts) |
Still open — unchanged |
| 10 | Nit | l1NetworkOf union-exhaustive default (src/lib/onchainPayout.ts:8) |
Still open — unchanged |
| 11 | Low | isBTCAddress cross-network acceptance (src/lib/address.ts:35–45) |
Still open — unchanged |
| 12 | Low | saveQuoteSnapshot localStorage TOCTOU (src/lib/swapRepository.ts) |
Still open — unchanged |
| 13 | Low | lnSendRefusal shows union of solver ranges, not intersection (src/lib/sendRouter.ts:122–130) |
Still open — unchanged |
| 14 | Low | profile.refund_without_receiver_delay required field — hard-fails every LN send if any deployed solver omits it |
Still open — unchanged |
| 15 | Low | readSwapRecordAssets — fromAsset/toAsset optionality not reflected in return type (src/providers/wallet.tsx) |
Still open — unchanged |
| 16 | Low | showCompleteBoarding missing amountAboveDust guard (src/screens/Wallet/Transaction.tsx:315–316) |
Still open — unchanged |
Incremental diff assessment (a79ef199 → cea3b7f)
What changed
This is a clean master-to-branch sync of one logical change: treat assets with no decimal metadata as having 0 decimal places (whole units), not 8.
Rename: unitsToCents → unitsToAtomic, centsToUnits → atomicToUnits across 23 files. Purely mechanical; no logic changed.
Default parameter change: decimals = 8 → decimals = 0 in src/lib/assets.ts:20 and src/lib/assets.ts:28. All call sites that previously relied on the implicit 8-decimal fallback for unknown assets now get 0.
Fallback sites: Every ?? 8 in AssetCard.tsx, Burn.tsx, Detail.tsx, Index.tsx, Mint.tsx, MintSuccess.tsx, Reissue.tsx, Vtxos.tsx, Send/Details.tsx, Send/Form.tsx, Send/Success.tsx, transactionAmountDisplay.ts, usePortfolioFiat.ts changed to ?? 0.
Test updates: asset.test.ts rewrites the default-value cases (correctly: unitsToAtomic('1') now asserts BigInt(1), not BigInt(100_000_000)). transaction.test.tsx updates two assertions to match: 0.0001 → 10,000 and 0.0000005 unknown-… → 50 unknown-…. Both reflect that an unknown-decimal asset's atomic amount is now displayed as a whole unit rather than divided by 10^8. transactionAmountDisplay.test.ts adds a new test: "reads an asset with no decimals in its metadata as whole units" — amount: BigInt(1) displays as 1 CTRL, which pins the corrected behaviour.
Assessment
The code comment at src/lib/assets.ts:20 — // missing decimals read as whole units: understating an amount beats overstating it — captures the intent correctly. For a display-side change this is the right tradeoff: showing 10,000 TOKEN when the asset has 8 decimals (silent understatement in display) is safer than showing 0.0001 TOKEN when the asset has 0 decimals (a 10^8× overstatement in display). The change has no effect on how amounts are stored or transacted — decimals is always supplied explicitly at protocol boundaries.
One UX consequence worth flagging (not a blocker): in src/screens/Wallet/Receive/QrCode.tsx at the setAssetAmount call path, if a user enters 1.5 for an asset whose metadata has not yet resolved, unitsToAtomic('1.5', undefined) → unitsToAtomic('1.5', 0) → BigInt(1) — the fractional part is silently truncated. The conversionDecimals guard is the intentional mechanism; the user gets no feedback that the amount was adjusted. This is display-only risk (no funds are moved by this truncation), but a validation message on unknown-decimal assets with fractional input would improve clarity. Not introduced by this diff — the truncation behaviour was always there with out-of-range decimals.
No new protocol, security, or cross-repo concerns introduced by this increment.
Findings 1 and 2 remain the merge blockers: the claimOnchainFill package-side audit and pre-release dependency pins.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
2 commits since a79ef199 — atomicToUnits/unitsToAtomic rename + missing-decimals default flip (master sync). 2 of 16 prior findings confirmed fixed; 14 remain open. No new findings from this increment.
Prior findings — close-out
Findings 6 and 7 were fixed between the last Arkana review (2026-09-24) and the a79ef199 baseline — not in this diff window but verified in the current file state.
Finding 6 — sendRouter wrong error class / no grace period: Fixed. src/providers/swaps.tsx now guards sendRouter with the grace-period wait and throws SwapsHeldElsewhere (not a plain Error) after it.
Finding 7 — withinPricingBudget(priced).then(settle) no catch: Fixed (confirmed by e2c03b33, incorrectly carried as open in later passes). Details.tsx has .catch((err) => consoleError(err, ...)).
All 14 remaining findings are in files not touched by this diff. Status:
| # | Sev | Finding | Status |
|---|---|---|---|
| 1 | High | swapClient.ts — claimOnchainFill package-side signing audit (malformed solver htlc/utxo driving arbitrary sighash) |
Still open |
| 2 | High | RC packages: @arkade-os/sdk@0.5.0-rc.22, @arkade-os/swap@0.1.0-rc.25 — still pre-release; versions advanced (rc.10/rc.13 → rc.22/rc.25) but API-stability risk is unchanged |
Still open |
| 3 | Medium | @arkade-os/swap/advanced subpath not a stability promise (swapRecords.ts, swapClient.ts) |
Still open |
| 4 | Medium | Mid-session spend miss — no regression guard test | Still open |
| 5 | Low | claimFeeRate comment ambiguity (claimFee.ts:14) |
Still open |
| 8 | Low | cancelSwapHere type-brand cast on LN-receive ID (providers/swaps.tsx) |
Still open |
| 9 | Low | CORRIDOR_LABEL CAIP-19 forward-compat gap (useCorridorSendReceipt.ts) |
Still open |
| 10 | Nit | l1NetworkOf union-exhaustive default (onchainPayout.ts:8) |
Still open |
| 11 | Low | isBTCAddress cross-network acceptance (address.ts:35-45) |
Still open |
| 12 | Low | saveQuoteSnapshot localStorage TOCTOU (swapRepository.ts) |
Still open |
| 13 | Low | lnSendRefusal shows union of solver ranges, not intersection (sendRouter.ts:122-130) |
Still open |
| 14 | Low | profile.refund_without_receiver_delay required field — hard-fails LN sends if any solver omits it |
Still open |
| 15 | Low | readSwapRecordAssets — fromAsset/toAsset optionality not in return type (providers/wallet.tsx) |
Still open |
| 16 | Low | showCompleteBoarding missing amountAboveDust guard (Transaction.tsx:315-316) |
Still open |
Incremental diff assessment (a79ef199 → cea3b7f)
What changed: master-sync of one logical fix — treat assets with no decimal metadata as 0-decimal (whole units) rather than 8-decimal (BTC-scale). Two commits, 23 files, ~82 net lines.
- Rename:
unitsToCents→unitsToAtomic,centsToUnits→atomicToUnitseverywhere. Mechanical; no logic changed at call sites that supply explicitdecimals. - Default parameter:
assets.ts—decimals = 8→decimals = 0on bothunitsToAtomicandatomicToUnits. - Fallback sites: Every
?? 8acrossAssetCard.tsx,Burn.tsx,Detail.tsx,Index.tsx,Mint.tsx,MintSuccess.tsx,Reissue.tsx,Vtxos.tsx,Send/Details.tsx,Send/Form.tsx,Send/Success.tsx,transactionAmountDisplay.ts,usePortfolioFiat.tschanged to?? 0. - Tests:
asset.test.tsrewrites the default-value cases correctly (unitsToAtomic('1')assertsBigInt(1)notBigInt(100_000_000)).transaction.test.tsxupdates two assertions (0.0001→10,000;0.0000005 unknown-…→50 unknown-…).transactionAmountDisplay.test.tsadds a pinning test for the corrected behaviour.
Assessment: The inline comment // missing decimals read as whole units: understating an amount beats overstating it (assets.ts:20) captures the rationale correctly. Showing 10,000 TOKEN for an unknown-decimal asset that actually has 8 decimals (a display understatement) is safer than showing 0.0001 TOKEN for an asset that actually has 0 decimals (a 10^8 display overstatement). The change has no effect on stored or transacted amounts — decimals is always supplied explicitly at protocol call boundaries.
One UX note (not a blocker, not introduced by this diff): at the asset-receive path in QrCode.tsx, if metadata has not resolved, unitsToAtomic('1.5', undefined) → BigInt(1) — the fractional part is silently truncated via conversionDecimals. A validation hint when the asset's decimal count is unknown would improve UX, but the truncation is intentional safety behaviour.
No new protocol, security, or cross-repo concerns from this increment.
Findings 1 and 2 remain the merge blockers: the claimOnchainFill package-side audit and pre-release dependency pins.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
Test review submission check - Thu Oct 15 09:08:36 PM UTC 2026
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
Arkana review test 1792098535
Contributor
|
test issue comment 1792098544 |
arkana-ai-bot
requested changes
Oct 15, 2026
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Status: 1 commit since `a79ef199` (merge of master decimal-fix through `37a64cd6`) — asset decimal defaulting and helper rename only. 0 of 16 prior findings are fixed; all 16 remain open. No new critical findings in this increment. The decimal fix itself is correct and appropriate.
Incremental diff assessment
The sole substantive change is the asset-decimal-default and rename work merged from master:
unitsToCents→unitsToAtomic,centsToUnits→atomicToUnits— mechanical rename, no logic change.- Default decimal fallback
8 → 0across every call site (Send/Details.tsx:52,Send/Form.tsx:315/371,Send/Success.tsx:55,Swap/Index.tsx,AssetCard.tsx,InputAmount.tsx,Keyboard.tsx,usePortfolioFiat.ts:74,Apps/Assets/{Detail,Index,MintSuccess}.tsx,Settings/Vtxos.tsx:294). liquidBtcBalanceexport removed fromsrc/lib/assets.ts(confirmed CI still green — no dangling callers).
The direction is correct: when asset metadata is absent, showing 1 for 1 atomic unit is safer than showing 0.00000001. The conversionDecimals clamp means unitsToAtomic('1.5', 0) returns BigInt(1) — truncates rather than throws, keeping the safe side. Test updates in asset.test.ts and transactionAmountDisplay.test.ts pin the new expected values. ✅
src/screens/Wallet/Send/Details.tsx appears in this diff but only at the decimal-fallback line (line 52). Finding #7 (withinPricingBudget(priced).then(settle) — no catch on the tail) is at a different location in the file and was not touched.
Prior findings — close-out
| # | Severity | Finding | Status |
|---|---|---|---|
| 1 | High | src/lib/swapClient.ts — package-side audit of claimOnchainFill: whether a malformed solver fill can drive the function to sign an arbitrary sighash via crafted htlc/utxo parameters |
Still open — file not in this diff |
| 2 | High | package.json — RC packages @arkade-os/sdk@0.5.0-rc.22 / @arkade-os/swap@0.1.0-rc.25 still pre-release |
Still open — file not in this diff |
| 3 | Medium | src/lib/swapRecords.ts, src/lib/swapClient.ts — @arkade-os/swap/advanced subpath: not a stability contract, every bump must be re-checked |
Still open — files not in this diff |
| 4 | Medium | Known mid-session spend miss — no regression guard test | Still open — unchanged |
| 5 | Low | src/lib/claimFee.ts:14 — claimFeeRate comment ambiguity vs. strict > comparison |
Still open — file not in this diff |
| 6 | Low | src/providers/swaps.tsx — sendRouter calls driving() directly; wrong error class in non-driving tab |
Still open — file not in this diff |
| 7 | Low | src/screens/Wallet/Send/Details.tsx — withinPricingBudget(priced).then(settle) — no .catch() on the tail |
Still open — file was in diff but the relevant call site was not touched |
| 8 | Low | src/providers/swaps.tsx — cancelSwapHere type-brand cast on LN-receive ID |
Still open — file not in this diff |
| 9 | Low | src/hooks/useCorridorSendReceipt.ts — CORRIDOR_LABEL CAIP-19 forward-compat gap |
Still open — file not in this diff |
| 10 | Nit | src/lib/onchainPayout.ts:8 — l1NetworkOf non-exhaustive default silently passes through unrecognised network strings |
Still open — file not in this diff |
| 11 | Low | src/lib/address.ts:35–45 — isBTCAddress accepts testnet/signet addresses on a mainnet wallet; error is deferred to signing |
Still open — file not in this diff |
| 12 | Low | src/lib/swapRepository.ts — saveQuoteSnapshot localStorage TOCTOU (display-only risk) |
Still open — file not in this diff |
| 13 | Low | src/lib/sendRouter.ts:122–130 — lnSendRefusal computes union of solver min/max ranges instead of intersection |
Still open — file not in this diff |
| 14 | Low | Solver profile.refund_without_receiver_delay required field — hard-fails every LN send if any deployed solver omits it |
Still open — unchanged |
| 15 | Low | src/providers/wallet.tsx — readSwapRecordAssets fromAsset/toAsset optionality not reflected in return type |
Still open — file not in this diff |
| 16 | Low | src/screens/Wallet/Transaction.tsx:315–316 — showCompleteBoarding missing amountAboveDust guard |
Still open — file not in this diff |
Findings 1 and 2 remain the merge blockers. All else is in scope for ship but should land before a stable release tag is cut.
arkana-ai-bot
requested changes
Oct 15, 2026
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Status: 1 commit since a79ef199 (master decimal-fix merge through 37a64cd6) — asset decimal defaulting and helper rename only. 0 of 16 prior findings are fixed; all 16 remain open. No new critical findings in this increment. The decimal fix itself is correct and appropriate.
Incremental diff assessment
The sole substantive change is the asset-decimal-default and rename work merged from master:
unitsToCents→unitsToAtomic,centsToUnits→atomicToUnits— mechanical rename, no logic change.- Default decimal fallback
8 → 0across every call site (Send/Details.tsx:52,Send/Form.tsx:315/371,Send/Success.tsx:55,Swap/Index.tsx,AssetCard.tsx,InputAmount.tsx,Keyboard.tsx,usePortfolioFiat.ts:74,Apps/Assets/{Detail,Index,MintSuccess}.tsx,Settings/Vtxos.tsx:294). liquidBtcBalanceexport removed fromsrc/lib/assets.ts(CI still green — no dangling callers).
Direction is correct: when asset metadata is absent, showing 1 for 1 atomic unit is safer than 0.00000001. The conversionDecimals clamp means unitsToAtomic("1.5", 0) returns BigInt(1) — truncates rather than throws, staying on the safe side. Test updates in asset.test.ts and transactionAmountDisplay.test.ts pin the new expected values. ✅
src/screens/Wallet/Send/Details.tsx appears in this diff but only at the decimal-fallback line (line 52). Finding #7 (withinPricingBudget(priced).then(settle) — no catch on the tail) is at a different location and was not touched.
Prior findings — close-out
| # | Severity | Finding | Status |
|---|---|---|---|
| 1 | High | src/lib/swapClient.ts — package-side audit of claimOnchainFill: whether a malformed solver fill can drive signing of an arbitrary sighash via crafted htlc/utxo |
Still open — file not in this diff |
| 2 | High | package.json — RC packages @arkade-os/sdk@0.5.0-rc.22 / @arkade-os/swap@0.1.0-rc.25 still pre-release |
Still open — file not in this diff |
| 3 | Medium | src/lib/swapRecords.ts, src/lib/swapClient.ts — @arkade-os/swap/advanced subpath not a stability contract; must re-check on every bump |
Still open — files not in this diff |
| 4 | Medium | Known mid-session spend miss — no regression guard test | Still open — unchanged |
| 5 | Low | src/lib/claimFee.ts:14 — claimFeeRate comment ambiguity vs. strict > comparison |
Still open — file not in this diff |
| 6 | Low | src/providers/swaps.tsx — sendRouter calls driving() directly; wrong error class in non-driving tab |
Still open — file not in this diff |
| 7 | Low | src/screens/Wallet/Send/Details.tsx — withinPricingBudget(priced).then(settle) — no .catch() on the tail |
Still open — file was in this diff but the relevant call site was not touched |
| 8 | Low | src/providers/swaps.tsx — cancelSwapHere type-brand cast on LN-receive ID |
Still open — file not in this diff |
| 9 | Low | src/hooks/useCorridorSendReceipt.ts — CORRIDOR_LABEL CAIP-19 forward-compat gap |
Still open — file not in this diff |
| 10 | Nit | src/lib/onchainPayout.ts:8 — l1NetworkOf non-exhaustive default silently passes through unrecognised network strings |
Still open — file not in this diff |
| 11 | Low | src/lib/address.ts:35–45 — isBTCAddress accepts testnet/signet addresses on a mainnet wallet; error deferred to signing |
Still open — file not in this diff |
| 12 | Low | src/lib/swapRepository.ts — saveQuoteSnapshot localStorage TOCTOU (display-only risk) |
Still open — file not in this diff |
| 13 | Low | src/lib/sendRouter.ts:122–130 — lnSendRefusal computes union of solver min/max ranges instead of intersection |
Still open — file not in this diff |
| 14 | Low | Solver profile.refund_without_receiver_delay required field — hard-fails every LN send if any deployed solver omits it |
Still open — unchanged |
| 15 | Low | src/providers/wallet.tsx — readSwapRecordAssets fromAsset/toAsset optionality not reflected in return type |
Still open — file not in this diff |
| 16 | Low | src/screens/Wallet/Transaction.tsx:315–316 — showCompleteBoarding missing amountAboveDust guard |
Still open — file not in this diff |
Findings 1 and 2 remain the merge blockers. All else is in scope for ship but should land before a stable release tag is cut.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
Follow-up review pass at cea3b7f — see main review comment above.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
Arkana follow-up review at cea3b7f: decimal-default fix assessed, 0/16 prior findings resolved. See full review below.
The v2 swaps provider dropped the solver-card subscription the old AssetSwapsProvider had, and only watched markets per network. The markets cache is keyed on network and registry, not on pinned cards, so a newly pinned card (e.g. BTC/DePix on mainnet) stayed hidden until the cache's TTL lapsed, with no error. Subscribe to the card store again and, on a change, run discovery past the cache, which also rewrites it for the watch's next tick.
The bundled beta-solver card is canonical: its rail lives in the asset id (bolt11:bitcoin/slip44:0) and discovery leaves the deprecated quote_corridor field unset on a pinned card. The field checks restored by the revert of "price canonical solver cards" therefore missed it: - the offer composer kept the Lightning market, which turned swaps on with nothing to swap and, next to a legacy-id card (e.g. BTC/DePix), emptied the receive picker: findMarket resolved the wallet's btc against the canonical BTC leg and no legacy market matched; - the Lightning bounds pre-flight and lnSendRefusal never found a market; - Settings listed the card with a blank pair label. Filter with isRfqMarket, detect Lightning with marketCorridor via a shared isLightningMarket, and derive a missing pair label from the asset ids.
arkana-ai-bot
requested changes
Oct 16, 2026
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Status: 1 commit since `cea3b7f` — SDK rc.22→rc.23, swap rc.25→rc.26 (package.json + lockfile only). 0 of 14 prior open findings are fixed; all 14 carry forward. No new findings from this increment.
Incremental change — `e0e33d05` ("Bump SDK")
Two files touched: `package.json` (2 lines), `pnpm-lock.yaml` (lockfile churn).
- `@arkade-os/sdk` `0.5.0-rc.22` → `0.5.0-rc.23`
- `@arkade-os/swap` `0.1.0-rc.25` → `0.1.0-rc.26`; lockfile confirms the swap package's own SDK dep is now also `rc.23` (no split-version risk).
What changed in the packages (rc.20→rc.23 reconstructed from ts-sdk clone)
| Area | Change | Wallet impact |
|---|---|---|
ArkadeTransactionBuilder.withPacket() |
New additive API | None — wallet doesn't call it |
ContractSyncState.syncing |
New optional field on both modes | None — additive |
IContractManager.dispose() return type |
void → Promise<void> (breaking) |
Benign — wallet doesn't implement or directly call dispose() |
IContractManager.getStoredContractsWithVtxos?(), whenBooted?() |
New optional methods | None — additive |
@arkade-os/swap/nostr drops RFQ_AD_KIND export |
Breaking; SOLVER_CARD_KIND moves to @arkade-os/solver-discovery |
Already handled. src/test/lib/nostrRfq.test.ts at PR head (e0e33d05) correctly imports only RFQ_DIRECTED_KIND and nostrRfqTransport; the RFQ_AD_KIND test was removed in a prior commit. CI confirms 2 tests pass. ✅ |
accept.ts — assetPayment field tracking in swap records |
Internal record-keeping for asset-payment swaps | None — wallet consumes the client API, not the internals |
No signing-path changes in the onchain corridor (packages/swap/src/client/corridors/onchain.ts diff: none), so Finding H1 is not addressed by this bump.
Prior findings close-out
No wallet source file was modified in this commit. All 14 open findings carry forward without change.
| # | Sev | Finding | File:line | Status |
|---|---|---|---|---|
| H1 | High (merge blocker) | claimOnchainFill package-side signing audit — whether a malformed solver fill can drive sign with a crafted sighash via htlc/utxo parameters |
src/lib/swapClient.ts:86–93 |
Still open — onchain.ts corridor unchanged in rc.20→rc.23 diff |
| H2 | High (merge blocker) | RC dependency pins — sdk@0.5.0-rc.23, swap@0.1.0-rc.26 — onchain-claim behaviour can change between RC and final without a wallet-side diff |
package.json |
Still open — versions are incrementally closer to final but remain pre-release |
| M1 | Medium | @arkade-os/swap/advanced subpath is not a stable export contract; a minor swap bump can drop it without SemVer signal |
src/lib/swapRecords.ts, src/lib/swapClient.ts:33 |
Still open |
| M2 | Medium | Known mid-session spend miss — no regression guard test | (provider layer) | Still open |
| L1 | Low | claimFeeRate comment ambiguity |
src/lib/claimFee.ts:14 |
Still open |
| L2 | Low | cancelSwapHere type-brand cast on LN-receive ID |
src/providers/swaps.tsx |
Still open |
| L3 | Low | CORRIDOR_LABEL CAIP-19 forward-compat gap |
src/hooks/useCorridorSendReceipt.ts |
Still open |
| L4 | Nit | l1NetworkOf union-exhaustive default |
src/lib/onchainPayout.ts:8 |
Still open |
| L5 | Low | isBTCAddress cross-network acceptance |
src/lib/address.ts:35–45 |
Still open |
| L6 | Low | saveQuoteSnapshot localStorage TOCTOU |
src/lib/swapRepository.ts |
Still open |
| L7 | Low | lnSendRefusal shows union of solver ranges, not intersection |
src/lib/sendRouter.ts:122–130 |
Still open |
| L8 | Low | readSwapRecordAssets — fromAsset/toAsset optionality not reflected in return type |
src/providers/wallet.tsx |
Still open |
| L9 | Low | showCompleteBoarding missing amountAboveDust guard |
src/screens/Wallet/Transaction.tsx:315–316 |
Still open |
| L10 | Low | Fractional-input truncation on unknown-decimal assets silently drops the fractional part with no user feedback | src/screens/Wallet/Receive/QrCode.tsx (setAssetAmount path) |
Still open |
Merge blockers remain H1 and H2. This bump is otherwise clean.
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
Status: 1 commit since last pass — pure dep bump (sdk rc.22→rc.23, swap rc.25→rc.26). 0 prior findings fixed, 5 still open. 4 new observations from the package delta.
Prior findings — close-out
[Still open] High: onchain claim sign path (src/lib/swapClient.ts)
claimOnchainFill and its sighash construction are unchanged in rc.26. The concern from the prior pass — that the wallet trusts the package to construct a correct sighash from network-supplied htlc/utxo parameters in claimOnchainFill, and that only a package-side audit can close this — remains open.
[Still open, surface expanded] High: RC versions (package.json)
Now at @arkade-os/sdk@0.5.0-rc.23 / @arkade-os/swap@0.1.0-rc.26. rc.26 adds substantial new surface: the onchain->arkade corridor, carrier/taxi payment quoting, EVM RFQ, offer fill plan, and joint graph — none of which are activated by the wallet. The gap between "the RC that passes CI today" and "whatever ships as 0.1.0" is wider now than at the last review commit. Pinning to a final release or documenting the commit hash remains the clean answer.
[Still open, barrel grew] Medium: @arkade-os/swap/advanced subpath (src/lib/swapRecords.ts, src/lib/swapClient.ts)
rc.26 adds 13 more exported names to the /advanced barrel: offer fill plan, joint graph, EVM RFQ types. The wallet's own comment — "re-check on a swap-package bump" — applies to this bump immediately. No wallet import broke in this bump, but the re-check commitment grows with each RC.
[Still open] Medium: mid-session spend miss
rc.26 adds fundingNotSubmitted / retireUnsubmittedFunding for the deadline-exceeded-before-submission case. That is a distinct scenario. The general mid-session spend event miss (a spend lands with no subscription event, leaving a record pending until reload) is unchanged and still has no guarding test.
[Still open] Low: claimFeeRate comment (src/lib/claimFee.ts:14)
No wallet code changed. Still open.
New observations from the rc.23/rc.26 package delta
1. refundOnchain callback not wired — trader L1 funding undriven if record is restored
packages/swap/src/swapManager.ts — RfqSwapManagerCallbacks
rc.26 adds refundOnchain: (swap: OnchainReceiveSwap, utxo: ChainUtxo) => Promise<{ txid: string }> as a required callback for the onchain->arkade corridor. The wallet's provider (src/providers/lnSwaps.tsx) does not register it. Since the onchain corridor is not in the wallet's active SwapClientConfig, the manager will refuse with noRefundOnchainCallback on any onchain_receive record rather than silently skip it — that is the correct safe-fail path.
The gap to track: if a user creates an onchain->arkade swap outside this wallet and later restores the record here, the trader's L1 HTLC refund leaf will be undriven indefinitely. The HTLC refund leaf does not expire (nextOnchainReceiveAction returns "refund" at refundable phase; there is no refund_window_closed equivalent), so the funds are not destroyed — but they do remain locked until either the wallet wires refundOnchain or the user sweeps manually. This should be closed before the Taxi child PR (#1058), which likely activates this corridor.
2. claimLockup parameter widened to ReceiveLockupSwap — confirmed safe
packages/swap/src/swapManager.ts, packages/swap/src/client/corridors/onchain.ts
The callback signature is now claimLockup(swap: ReceiveLockupSwap, ...) where ReceiveLockupSwap = LightningReceiveSwap | OnchainReceiveSwap. The wallet's implementation accesses only swap.rfqId (on RfqSwapCommon) and then reads from the repository by id, so no subtype-specific fields are touched in the discriminated part. Typecheck passed at CI. No action required.
3. Timer overflow guards added — positive defensive change
rc.23 adds assertTimerMs (capped at 2^31 − 1 ms ≈ 24 days). rc.26 applies it to pollUntil in onchainHtlc.ts and refundIfUnresolved in refund.ts. Without this guard, a setTimeout(fn, N) with N > 2^31−1 wraps negative on 32-bit signed implementations and fires immediately, turning a "never retry before locktime" poll into a tight busy-loop. The wallet uses the default pollMs = 5000, so no operational impact, but the guard closes the footgun for callers that might pass large values.
4. SendDeadlineExceededError / fundingNotSubmitted — inert at this wallet version
rc.26 raises SendDeadlineExceededError before provider submission when validUntil fires, writes fundingNotSubmitted.reason: "send_deadline" to the record, and calls retireUnsubmittedFunding to release the offer contract. The wallet at e0e33d0 never passes validUntil to accept(), so this path is completely inert. fundingWasNotSubmitted returns false for all existing records (field missing ⇒ undefined ⇒ false). No regression.
Summary
The wallet-level code is unchanged in this commit. All five prior issues carry forward unchanged. The rc.26 package additions are substantial but latent — the refundOnchain callback gap is the item to close before the Taxi child PR activates the onchain->arkade corridor.
The client takes its discovery options once, pinned cards included. A
client built before a card was pinned therefore quoted against a snapshot
without it ("no market serves arkade:btc -> arkade:<asset> on the active
discovery snapshot"), and its cache writes overwrote the shared markets
entry with the older set, so after a reload the composer had no markets
and showed "Swaps are coming soon".
Key the client effect on the solver-card version as well, so the client
is stopped and rebuilt with the current cards, as on unlock.
pietro909
approved these changes
Oct 16, 2026
arkana-ai-bot
left a comment
arkana-ai-bot
left a comment
Contributor
There was a problem hiding this comment.
PROTOCOL-CRITICAL: human review required.
Status: 3 commits since e0e33d05 — all signed, all from @pietro909. 7 files changed. This increment re-lands canonical solver card support (previously reverted from 43ede002) using the now-native isRfqMarket / marketCorridor APIs from @arkade-os/solver-discovery@0.3.0 instead of a wallet-side pnpm patch against compiled JS. 0 of the 16 prior open findings are fixed by this diff; 16 remain open. Two observations on the new code below.
Note: the PR is already merged. This review covers the final 3 commits only; all prior items are carried for the record.
Prior findings close-out
| # | Severity | Finding | Status |
|---|---|---|---|
| 1 | High | swapClient.ts — claimOnchainFill package-side signing audit: whether a malformed solver fill can drive the callback to sign an arbitrary sighash via crafted htlc/utxo |
Still open — src/lib/swapClient.ts not in this diff |
| 2 | High | RC packages — @arkade-os/sdk@0.5.0-rc.23, @arkade-os/swap@0.1.0-rc.26 still pre-release |
Still open — package.json not in this diff; onchain-claim behaviour can change between RC and final with no wallet-side diff |
| 3 | Medium | @arkade-os/swap/advanced subpath not a stability-guaranteed export |
Still open — swapRecords.ts / swapClient.ts not in this diff |
| 4 | Medium | Mid-session spend miss — no regression guard test | Still open — unchanged |
| 5 | Low | claimFeeRate comment ambiguity (v > MIN_CLAIM_FEE_RATE already excludes 1 sat/vB, comment implies otherwise) — src/lib/claimFee.ts:14 |
Still open — not in this diff |
| 6 | Low | driving() called directly in cross-tab dispatch; wrong error class when not the lock holder — src/providers/swaps.tsx |
Still open — the diff adds cardsVersion to the client's dependency array but does not touch the dispatch path |
| 7 | Low | cancelSwapHere type-brand cast on LN-receive ID — src/providers/swaps.tsx:605 |
Still open — unchanged |
| 8 | Low | CORRIDOR_LABEL CAIP-19 forward-compat gap — src/hooks/useCorridorSendReceipt.ts |
Still open — not touched; the isLightningMarket fix addresses the market-filter side but the receipt hook's { lightning, onchain } map still uses legacy corridor names |
| 9 | Nit | l1NetworkOf union-exhaustive default — src/lib/onchainPayout.ts:8 |
Still open — not in this diff |
| 10 | Low | isBTCAddress cross-network acceptance — src/lib/address.ts:35–45 |
Still open — not in this diff |
| 11 | Low | saveQuoteSnapshot localStorage TOCTOU — src/lib/swapRepository.ts |
Still open — display-only risk, unchanged |
| 12 | Low | lnSendRefusal shows union of solver ranges, not intersection — src/lib/sendRouter.ts:152–153 |
Still open — the isLightningMarket predicate is new and correct; the union vs. intersection logic (min of mins, max of maxes) is unchanged |
| 13 | Low | profile.refund_without_receiver_delay required field hard-fails every LN send if a deployed solver omits it |
Still open — unchanged |
| 14 | Low | readSwapRecordAssets — fromAsset/toAsset optionality not reflected in return type — src/providers/wallet.tsx |
Still open — not in this diff |
| 15 | Low | showCompleteBoarding missing amountAboveDust guard — src/screens/Wallet/Transaction.tsx |
Still open — not in this diff |
| 16 | Low | DFX iframe auth-signature in URL — design-level issue, non-blocking, on the record | Still open — not in this diff |
Incremental findings
✅ isLightningMarket / isRfqMarket — correct
src/lib/sendRouter.ts:142 — isLightningMarket = (m) => marketCorridor(m, 'quote') === 'bolt11' reads the asset-id namespace rather than the legacy quote_corridor field. marketCorridor in solver-discovery@0.3.0 normalises both the legacy field and the CAIP-19 namespace to the same return value, so this predicate handles both card shapes correctly.
src/providers/swaps.tsx:301 — all.filter((m) => !isRfqMarket(m)) replaces !m.quote_corridor. The commit comment explains the second-order problem correctly: a canonical BTC leg with id arkade:bitcoin/slip44:0 was winning findMarket's id resolution and shadowing every legacy btc market. Filtering by isRfqMarket removes both legacy-corridor and canonical-corridor markets from the offer composer in one pass.
Tests at src/test/lib/bundledSolverCard.test.ts (canonical corridor detection, legacy-card pairing after filter) and src/test/providers/swaps.test.tsx (canonical corridor hiding) directly pin the regression. Coverage is adequate.
✅ Card-change reactivity — correct shape
src/providers/swaps.tsx:327–334 — useSyncExternalStore(subscribeSolverCards, getSolverCardsVersion, getSolverCardsVersion) is the idiomatic React 18 hook for subscribing to an external store. The seenCardsVersion ref guards against firing on mount (where cardsVersion === seenCardsVersion.current), so runDiscovery(false) is only called on genuine writes — correct.
src/providers/swaps.tsx:539 — cardsVersion added to the client useEffect dependency array. This triggers the client teardown-and-rebuild path when a card is pinned or removed, which is necessary because the client snapshots its discovery options at construction time. Tests in swaps.test.tsx verify both re-discovery and client rebuild are triggered. ✅
Nit: runDiscovery not in the useEffect dependency array
src/providers/swaps.tsx — the card-change useEffect at line 329–334 calls runDiscovery but lists only [cardsVersion] (with a lint-disable comment). runDiscovery is a function defined in the component body that closes over aspInfo.network, svcWallet, and the discovery state. In practice this is safe: runDiscovery only fires when cardsVersion changes, which only happens after mount, at which point those dependencies are always populated. But the omission means a future reader adding a conditional inside runDiscovery that depends on a new state variable will introduce a stale-closure bug without TypeScript or eslint catching it. The pattern is already present elsewhere in this file (the network-change effect uses the same lint-disable), so this is a pre-existing style choice rather than a new concern introduced here. Minor.
Nit: marketLabel — no null-guard on base_asset / quote_asset
src/screens/Settings/Solvers.tsx:44–46:
const marketLabel = (market: Market): string =>
market.pair ??
`${pairSideLabel(legacyMarketCorridor(market, 'base'), market.base_asset.ticker)}/ ...`Market is the type from @arkade-os/solver-discovery; if a card is syntactically valid but carries a market with missing base_asset or quote_asset (e.g. a future card version), this will throw and crash the Solvers screen. Since this is a display-only path with no user funds at risk, it is a nit rather than a blocker. A fallback (market.base_asset?.ticker ?? '?') would be safer.
This was referenced Oct 16, 2026
bordalix
added a commit
that referenced
this pull request
Oct 20, 2026
…ry (#1066) * rename ArkInfo (deprecated) to ArkadeInfo * remove arkServerUrl from requestLightningSend * remove arkServerUrl from requestLightningReceive * changes for sdk v0.5 * fix: use release candidate from npm * Upgrade to 0.5.0 RC 1 * move swaps to the v2 createSwapClient: one client for offers, lightning send and receive Claude-Session: https://claude.ai/code/session_01W5xDq73G5jGA23XG8B9SLr * move swaps to the v2 client: pay, receive and exchange from @arkade-os/swap/client Claude-Session: https://claude.ai/code/session_01W5xDq73G5jGA23XG8B9SLr * move to the M8 swap surface: the v2 client is the @arkade-os/swap root, v1 is /protocol Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * drop V1 swap support: one keyspace, one resolver, no chain rebuild for offers Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * fix: resolve one network for discovery, so an unnamed one keeps no solver cards Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * upgrade to 0.5.0-rc.3 / 0.1.0-rc.3: the promoted names come home, record reading moves to /advanced Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * feat(send): route on-chain sends through PaymentRouter, solver rail first Supersedes #941. That PR shipped a solver-routed onchain send with ~2,600 lines of bespoke route selection, bounds matching and fallback — all of which `PaymentRouter` (ts-sdk #837) already does. This is the same corridor expressed as a rail registration. What the router replaces, and the proof it covers it: every arm of #941's `OnchainRouteRefusal` enum becomes `available()` returning false, and a rail that says no is dropped from `options()` without taking the router with it, so the collaborative exit wins by ranking. `sendRouter.test.ts` asserts one case per deleted arm — no_solver, amount_out_of_bounds, discovery_failed, unsupported_address, no_l1_endpoint — alongside a control proving the solver rail really can rank first, without which those five would pass vacuously. `chainSourceFrom` likewise replaces #941's 222-line hand-written esplora client. The wrong-address guard survives, differently. #941 carried a negotiated quote across the Form -> Details navigation and compared its `payoutAddress` to the screen; `RouteQuote` has no address field, so that comparison has no home. Instead nothing is carried: `PaymentOption.quote()` is lazy by contract, so the sign screen quotes against the address it is displaying at the moment of the spend, and a stale quote is not a thing that can exist. `quoteIsForThisSend` is the belt to that braces, and is stricter than the check it replaces — the screen must be showing the routed address, the recipient must get what was displayed, and no more may leave the wallet than was displayed. Two rails, and only one of them is the SDK's. `solverOnchainRail` comes from `@arkade-os/swap`; the collaborative exit stays this wallet's own `collaborativeExitWithFees` rather than the SDK's `onchainRail`, because that one settles through `Ramps.offboard` with its own coin selection while this wallet deliberately spends soonest-expiring VTXOs first. Adopting it is a coin-selection change, separate from adopting the router. Amounts are receiver-exact, as `RouteQuote` specifies, but the send form still lets the user type what LEAVES the wallet. The translation happens at the screen boundary rather than in the rails: a typed amount interpreted as receiver-exact would gross up past the balance it came from, so "send max" could never succeed. `isBTCAddress` now delegates to the SDK's `isBtcAddress`, so the form classifies a target exactly as `btcTarget` does. That changes which addresses the form accepts, in both directions, and the four cases are pinned in `address.test.ts`. `@arkade-os/swap` is vendored from ts-sdk `fa3a2d1f` — the #837 squash — because no published version carries the rails. See vendor/README.md for why the SDK did not need vendoring alongside it, and for the version-collision trap that makes the `pnpm.overrides` entry load-bearing. * test(send): cover the sign screen's routing, which nothing exercised The router and the guard had unit tests, but no test rendered `SendDetails` with a plain BTC address and pressed sign — `send-details.test.tsx` only covers the amount hierarchy. So the whole `payOnchain` path was green by absence, which is exactly what a suite cannot tell you. Five cases at the screen that moves the money: the exit is taken when no solver rail survived, a rail quoting a different recipient amount is refused, a rail that would spend more than the screen displayed is refused, a matching solver quote IS funded, and — the wrong-address regression from 1b3481b — the request is built from the address the screen is showing. The mocked `options()` takes the request and builds the exit rail from `req.raw` rather than a constant. Without that the last case asserts against a hardcoded address and proves nothing; mutating `raw` to a fixed address kills it, as does deleting the `quoteIsForThisSend` call. * fix(send): three CodeRabbit findings, one of them a real money race **A funded reservation could be deleted on restart.** `isUnfundedReservation` treated a successful-but-empty lockup lookup as proof the swap was never funded. The record is written BEFORE the funding is broadcast, and the indexer learns of it later still, so a restart in that window read a funded swap as an abandoned reservation and dropped it — losing an L1 claim only this wallet can make. It now keeps anything younger than `RESERVATION_GRACE_SECONDS`. The asymmetry is the whole argument: the cost of keeping a stale row is a "Pending" line that prunes itself, and the cost of dropping a funded one is the fill. **The history row could show a fee nobody paid.** `handleSent` persisted `details.fees` — the number the screen displayed — while `quoteIsForThisSend` deliberately admits a quote whose total is LOWER than displayed. It now persists `quote.fee`. **The form accepted addresses nothing could spend to.** `isBtcAddress` is format-only by design, because a rail's `match()` runs before anything is known, so `bc1qqqqqqqq` and bad checksums reached the sign screen and failed there. `isBTCAddress` now also requires the address to decode on one of the three L1s. That is stricter than the SDK predicate but only on inputs that were never spendable, so the form and the rails still agree on every valid address. Fixing that exposed a bad fixture of my own: `2N2JD6wb56AFK4d9ppKAftvBoT9Cw3xL1Tm`, which I had cited as a testnet p2sh the old regex rejected, has an invalid checksum and fails on all three networks. Replaced with a verified one. Declined the fourth: `reserveOnchainSend`'s `managerRef.current?.addSwap` is optional on purpose, and matches `trackLnSend`. The record is saved first and is the invariant that matters — a reservation the manager never hears about still restores and is driven on the next start, so throwing would refuse a send the wallet can make in exchange for nothing. * fix(send): the exit rail must not claim ark addresses or invoices `match: (req) => Boolean(req.raw)` accepted any non-empty target, so the collaborative-exit rail would have claimed an ark address or a BOLT11 invoice if the router were ever asked to route one. It never is today — `payOnchain` is reached only from the plain-BTC branch — but a rail's `match` is its contract, not a note about who currently calls it. Now the same `btcTarget` classification the SDK's own `onchainRail` uses. Asserted rather than described: routing an ark address or an invoice returns no options at all, and reverting the predicate kills it. * docs(send): say that the reservation grace period is a guess `RESERVATION_GRACE_SECONDS = 600` is a judgement, not a measurement — nobody has profiled worst-case indexer lag after a funding broadcast. The comment now says so, and names the symptom of it being too short: a funded record dropped on restart, losing an L1 claim only this wallet can make. That failure is silent — the row simply disappears — so anyone with real lag figures should set the number from them rather than trusting a bare constant. * fix(send): take the first USABLE fee bucket, not the first present one CodeRabbit finding on the rebased head, and correct. `estimates['2'] ?? estimates['1'] ?? estimates['3']` falls through only on null/undefined, so an esplora reporting `{"1": 25, "2": 0}` had its zero taken, rejected by the validity check, and the claim dropped to the 1 sat/vB relay floor — with a perfectly good 25 sitting in the next bucket. That matters here more than it looks: `claimOnchainFill` refuses inside the refund leaf's margin, so a claim that sits unconfirmed is a claim that loses the fill. Underpaying by 24 sat/vB on a transaction racing a consensus deadline is the failure this function exists to avoid. Now picks the first bucket that is actually usable. Carried over from #941 unquestioned, like the reservation race — worth noting that both bugs came from code I ported rather than code I wrote. * fix(send): the solver rail was reading the wrong endpoint map `onchainClaimEndpoint` resolved through `src/lib/explorers.ts`, which exists to build explorer LINKS: `api` is optional there and `bitcoin` carries only `web`. So the endpoint came back undefined on mainnet, `createSendRouter` skipped the registration, and every mainnet on-chain send fell through to the collaborative exit — the PR's headline feature was a no-op in production. The SDK already ships the map this wanted: `ESPLORA_URL`, a `Record<NetworkName, string>` that is total by type, with mainnet on Arkade's own instance rather than mempool.space. Reading it there makes the endpoint unconditional, so the `if (onchainClaimEndpoint(...))` gate can go: what drops the solver is `available()` — no card serving the corridor, or an amount outside the ones that do — which is the real gate and was already tested. The test that pinned the old behaviour asserted mainnet gets only the exit rail, using bitcoin as the example of a network with no L1 endpoint. There is no such network now, so it asserts the opposite, and a second case pins that every `NetworkName` resolves an endpoint. The same change makes two branches in `lnSwaps.tsx` unreachable — `esploraUrl` can no longer be undefined — so the chain source is built directly and the "no L1 endpoint is configured" throw, which could no longer fire, is gone. Also from review: - A blank `details.destination` (what the screen stores when `utxoTxsAllowed()` is false) reached the router and failed `quoteIsForThisSend` on every rail, surfacing as "No route for this payment" when the real constraint was the account limit. Guarded at the callsite beside its siblings. - `l1NetworkOf` folding signet and mutinynet into 'testnet' is deliberate — they share HTLC script parameters — and now says so. * test(send): drive claimOnchain from the record it will really read The callback that claims the L1 HTLC had no test at all: `sendRouter.test.ts` stops at the quote, `send-onchain-route.test.tsx` stops at `send()`, and `lnReceive.test.tsx` only asserts that the receive leg installs no `claimOnchain`. Everything between a record written before funding and the arguments `claimOnchainFill` is handed was uncovered — which is the half where losing the money is possible. Real `onchainSendSwapRecord`, real `rfqClaimSecretOf`, real `preimageForSwapRecord`, real `claimPayoutScript`, real `claimFeeRate` against a stubbed fetch; only the manager and the fill itself are mocked. `mustPersistPreimage` puts the raw P in the record, so the round trip needs no signing wallet. Pins, each verified by mutation: - the payout script comes from the record, never derived (a derived one pays the sender while the screen says the recipient was paid) - P round-trips and still hashes to the payment hash - the fee rate is fetched from `ESPLORA_URL[network]` — including mainnet, where the pre-c8d284ec map had no entry and no estimate was ever fetched - an unreachable estimator drops to the floor rate rather than failing the claim - a missing record, a missing payout script and a missing hashlock each refuse by name and never reach the fill * test(send): cover the arm where P is re-derived, not stored Follow-up arkana raised on 529c0e18: only `mustPersistPreimage: true` was covered, where P is read straight out of the record. The other arm derives it from the seed, and a wallet that cannot must refuse rather than claim with a preimage the HTLC will reject — an L1 claim built on the wrong P burns the window and takes the refund path instead. Mutation: `preimageForSwapRecord(...).catch(() => new Uint8Array(32))` — `promise resolved "{ txid: 'claim-txid', … }" instead of rejecting`. Also asserts the estimator-down fallback against `MIN_CLAIM_FEE_RATE` rather than the literal 1. Raising the floor is a policy call (arkana's second review argues 1 sat/vB will not confirm under mempool pressure); the test should pin "falls back to the floor", not the number, so that call does not read as a regression. Same mutation strength: a hardcoded rate of 2 still fails it. * docs(send): name why send() is not caught, and why the fee floor is not a bump Both from arkana's full review. Comments only; no behaviour change. payOnchain — traced rather than assumed. The rail's send() does three things (@arkade-os/swap 0.0.12, solverOnchain.ts): assertFundable(...) pure validation, throws clean await deps.persist(swap) may write a record; no funds move await ctx.wallet.send() THE FUNDING BROADCAST ctx.wallet is the ServiceWorkerWallet (providers/wallet.tsx:112, passed at Details.tsx:200). Its send() is a message-bus round trip capped at DEFAULT_MESSAGE_TIMEOUTS.SEND = 50_000, and the worker awaits this.wallet.send() — submitting — before replying SEND_SUCCESS. A lost or late reply rejects with "Send failed: …" after the covenant is funded. That is #949's mechanism on this exact call, so the answer is the partial-work case: not catching is CORRECT, because falling through to the exit rail would pay the recipient twice. The saving grace is this PR's own ordering: persist runs BEFORE funding, so a throw at the funding step leaves a record, and RfqSwapManager restores and drives the swap. #949's funded-but-unrecorded window does not apply here. MIN_CLAIM_FEE_RATE — the "raise it to 2-3" suggestion is not one line, because the constant has two jobs: the fallback rate AND, via usableRate's `v > MIN_CLAIM_FEE_RATE`, the bar a real estimate must clear. Raising it to 3 would make the wallet disbelieve a genuine 1-2 sat/vB network and overpay whenever the mempool is calm — including on every testnet. A better fallback needs the two constants split first, which changes what users pay and belongs in its own diff. Recorded at the definition so the next reader does not bump it. This also answers the "cosmetic" note on usableRate: the strictly-greater comparison is not cosmetic, it is what makes the constant dual-purpose. * test(send): e2e for the on-chain send's unhappy paths (#948) * test(send): e2e for the on-chain send's unhappy paths #944 routes on-chain sends through the PaymentRouter with the solver rail first and the collaborative exit behind it, and adds no e2e at all — every test it ships is unit or component. The fallback is the whole point of the design and nothing exercised it in a browser. Four paths, each provoked at a different place: - the relay socket is refused -> "no relay accepted the RFQ message" - the request lands and is not answered -> the 30s RFQ timeout - both rails fail (offline) -> a named error and the retry button - the user leaves mid-quote (#946) -> the send completes, the screen is not hijacked Three facts made this harder than it looks, and each is why the tests are shaped the way they are: The regtest stack's own solver serves BTC/RGT asset markets only, with no `arkade:BTC -> onchain:BTC` corridor, so the solver rail's `available()` is false there and an on-chain send falls back whatever the code does. A "falls back to the exit" assertion against the stock stack passes with the rail deleted. Each test therefore pins its own card, and asserts the refusal it provoked rather than that a refusal happened — an earlier draft used an off-curve `discovery_pubkey` and both "unreachable" and "timeout" tests were silently failing at key decompression, never reaching a relay at all. `nak serve` refuses an ephemeral event when nothing is subscribed for it ("mute: no one was listening for this"), which collapses the timeout case back into the socket case. The timeout test therefore subscribes as the solver would and answers nothing, and asserts the request was delivered. An unset arkd intent fee parses as NaN, which makes the amount NaN and Tap to Sign a silent no-op; the fee is set for the same reason send.test.ts sets it. `playwright.yml` enumerates e2e files rather than globbing, so the new file is added to the matrix or it runs nowhere while the job stays green. * ci(e2e): generate the relay TLS cert before starting the proxy `relay-tls` bind-mounts ./.relay-tls for its cert and the workflow never created it, so wss://localhost:10548 has never worked in CI — nginx comes up with nothing to serve. `regtest:start` runs gen-relay-tls-cert.mjs for exactly this reason; the workflow open-codes the same steps and dropped it. Caught by the new "solver never answers" test, which needs a real relay: the RFQ publish was refused rather than delivered, so `delivered()` was 0 on all six attempts in both projects. A card's relays must be wss:// (RELAY in solver-discovery's validator), so there is no plaintext way round it. lnsend.test.ts depends on the same proxy and is in no CI group, which is presumably why nobody hit this before. * ci(e2e): reconcile the enumerated test list with the files on disk The matrix names files rather than globbing, and had drifted both ways: restore.test.ts listed, deleted in #908 with the Boltz removal swap.test.ts listed, renamed to swaps.test.ts in #784 — #815 wrote the pre-rename name when it split the suite into groups swaps.test.ts on disk, in no group solvers.test.ts on disk, in no group lnsend.test.ts on disk, in no group Nothing complained because Playwright treats the arguments as regexes and only errors when the TOTAL is zero: `playwright test src/test/e2e/restore.test.ts` alone exits 1 with "No tests found", but beside twelve filters that do match it is ignored and the job stays green. Verified with --list on both shapes. What this actually buys, measured locally rather than assumed: solvers.test.ts 2 tests, both pass — real coverage, and the repo's only solver e2e, which matters while #944 adds a solver rail swaps.test.ts 2 tests, both `test.skip` since 73e4084e. The typo fix is safe and buys nothing; the suite is dead at the source too lnsend.test.ts skips itself without .regtest-lightning.card.json, which CI does not produce, so it will report as skipped. Listed anyway: a named skip is visible, an unlisted file is not Separate from the test commits so it can be reverted on its own. * test(send): say why the card's base side is zeroed Raised by arkana on #948: `max_base_amount: '0'` reads as a mistake and will attract a "fix". Verified against solver-discovery 0.2.3 — `sideLimits(market, side)` reads only the queried side's keys and treats `max === "0"` as disabled, and `rendezvousOf` queries the QUOTE side, so the base side is unused on this path. Same shape as beta-solver.card.json. Comment only. * feat(send): route lightning and asset sends through PaymentRouter too (#947) * feat(send): route lightning and asset sends through PaymentRouter too paths still picked their provider by hand, so the send screen had three shapes and the UI had nowhere uniform to read options from. Both new rails are wallet-local, for the same reason `walletExitRail` is: the SDK/@arkade-os/swap rail for the corridor does not do what this wallet needs. - `lightning` wraps the existing `requestLnSend` + `sendOffChain` + `trackLnSend`. `solverLightningRail` was the obvious candidate and was rejected: it calls `persist` BEFORE funding and its `RouteResult` carries no txid, but `lnSendViews`/`viewOf` drop a record with no `fundingArkTxid` and `rfqSwapActivityInputs` groups on it — so the Lightning row would vanish from history. It also selects markets through `selectMarkets` rather than `lnSendRendezvous`, which is a different provider pick on a live money path. - `asset` wraps `sendAssets`. A `PaymentRequest` carries sats only, so the asset list rides the rail's deps. Which solver answers a Lightning send is unchanged: the rail calls the same `lnSendRendezvous` with the same per-network emulator pin, and the form's two distinct refusal messages are preserved by `lnSendRefusal`, since `options()` reports absence without a cause. The negotiation stays on the form, because the fee has to be on screen before the user signs; what moves is that it now goes through `options()` + `quote()`, and `SendInfo.pendingLnSend` carries the `RouteQuote` whose `send()` the sign screen calls. `quoteIsForThisInvoice` is the Lightning analogue of #944's wrong-address guard. `createSendRouter` now registers a rail only when the deps it needs are present, so the Lightning path does not have to derive an L1 payout pubkey it will never use. * fix(send): the wrong-invoice guard compared against an unstripped screen value `quoteIsForThisInvoice` compared `sendInfo.invoice` verbatim against the rail's `meta.invoice`, which is `invoiceTarget(req.raw)` — already stripped. `decodeBip21` assigns `result.invoice` from the `lightning=` param on nothing more than a lowercase `ln` prefix, so `lightning:lnbc…` passes that test and lands in flow state unstripped. Before routing that input simply failed: `requestLnSend` handed it to `decodeInvoice`, which rejects the prefix. Through the rail the negotiation now succeeds on the stripped invoice and only the sign screen would refuse — a burned quote and an invoice handed to a solver for a payment that then cannot be made. Normalising both sides through `invoiceTarget` keeps the guard refusing only a genuinely different invoice, and an empty target is refused rather than matching a quote that carries no meta. * fix(send): discover the solver cards once, not again on the refusal path `negotiate()` called `discoverMarkets` a second time to build the refusal message, after `router.options()` had already driven one through the rail's `available()`. Two costs, both on the path whose only job is to be informative: - a cold cache with an unreachable registry makes the SECOND call throw, and `negotiate().catch(handleError)` then shows the registry's error in place of "No Lightning solver available" or the card's bounds; - the rail ranked off one list and the message explained off another, so a market that moved between the two calls could produce a refusal that contradicts the ranking. Discovering once and handing the snapshot to the router through its existing `discover` seam fixes both, and restores the single lookup master made before routing — so a throwing registry surfaces exactly the error it always did. Also names, at the asset rail, why it emits only `settled` where the lightning rail emits `sent` first: no counterparty has to act, so the txid IS settlement. Both raised by arkana-ai-bot on #947. * refactor(send): port the send router onto the v2 swap rails #936 replaces the two `RfqSwapManager`s with one `SwapClient` under one Web Lock, and deletes `providers/lnSwaps`, `lib/lnSendRecords` and `lib/nostrRfq`. The router this branch built stood on all three, so it is rebuilt rather than relocated. The two solver rails now come from the package — `onchainSwapRail` and `lightningRail` over the driving tab's client — and the wallet keeps only the rails that have no counterparty: the collaborative exit (still not the SDK's `onchainRail`, which does its own coin selection) and the asset send. Ranking is unchanged: the solver rail is offered ahead of the exit, both stay registered, and an amount the solver will not take drops it with no error. What the package now owns, and this branch no longer does: - `reserveOnchainSend`. `accept()` persists before it funds, inside the corridor, so there is no wallet-side ordering left to get wrong. - the negotiation, the market pick and the rendezvous — the rails are adapters. - `claimLockup`. One client claims for every corridor under one lock, so the second unsynchronised claimer that argument was about cannot exist. What the wallet must still supply is the onchain corridor's `claim`: it is absent by default and deliberately not defaulted, and without it the drive reports an `arkade -> onchain` swap's L1 half blocked — the send funds an HTLC this wallet then never claims. That callback moves from the deleted provider into `makeSwapClient`, reading the payout script off the swap rather than rebuilding it, so `l1PayoutPubkey` and `claimPayoutScript` go with the provider. `quoteIsForThisInvoice` now compares payment hashes. The v2 rail's `meta` carries `quote.lock.hash` and never the BOLT11, so the old `meta.invoice` comparison would have refused every Lightning send rather than a mismatched one. Tests: the router's are rebuilt on a fake `SwapRailClient`, the claim keeps the coverage the deleted provider test had, and one new case pins the rule that a send which may already have funded is not retried on the next rail — that had no test, and the fallback it would allow pays the recipient twice. * test: give the suite the margin its slowest React flows already need Vitest's 5 s default is measured against full-suite contention, not isolation. `swap.test.tsx`'s two slowest tests run ~2.2 s and ~1.0 s alone and cross 5 s under load; `receive-lightning-error.test.tsx` has crossed it the same way on this branch's base. Both files pass alone and fail in the suite, which is the signature of a margin problem rather than a broken test. This branch is what makes it fire: five new test files raise parallel load enough to push those two over. On `bd507474` the suite is 3/3 green and with this branch's tests it was 4/5 red, on timeouts alone and never on an assertion. With this, 3/3 green. Config rather than a per-test timeout: the two files that have hit it are different files, so the margin is the thing that is short, not either test. A hang still fails — 15 s is headroom, not a mask. * fix(send): price the on-chain send by quoting, not by the exit rail's fee The sign screen committed to a total before any rail had quoted, and computed it from `calcOnchainOutputFee()` — the collaborative exit's output fee, which it SUBTRACTS from the amount rather than adding to the total. A solver rail charges its spread and the L1 claim ON TOP of the payout, so its total always exceeded the screen's, and `quoteIsForThisSend` refused it. On a deployment where that fee is 0 the screen's total equalled the amount, leaving no room for any fee at all, so `onchain-swap` could never win and every ark -> L1 payment fell back to the exit. From mutinynet: onchain send: onchain-swap refused: quote pays 40000 for 40754, screen shows 40000 for 40000 onchain send: paying via onchain The guard is not the bug and is unchanged: a rail must not quote worse than the user was shown, and it is the only thing standing between a user and silently overpaying. What changes is the figure it measures against. The screen now asks the router what the send really costs before it offers anything to sign, and displays that rail's fee and total; the spend re-quotes, and the guard refuses any rail that has since got dearer than the figure on screen. Signing is held back until that quote lands, so the total the user taps is never provisional. `payOnchain` is untouched, including the deliberate absence of a catch around `send()` — a rejection there can mean a covenant that IS funded, and trying the exit next would pay the recipient twice. The Lightning path already worked this way: `Form.tsx` quotes before navigating and the screen shows `pendingLnSend.total`. The asset path prices itself and the rail from the same `defaultFee`, so neither shares the defect. * fix(claim): read the fee tier these deployments actually serve `claimFeeRate` asked for Esplora's `/fee-estimates`, which every Arkade mempool deployment answers with a 404 — mainnet included. The catch returned the 1 sat/vB floor, so every L1 claim was priced at the floor on every network, silently. Confirmed live: mempool.arkade.sh reported fastestFee 3 while the wallet claimed at 1. These are mempool.space deployments, which serve named tiers at `/v1/fees/recommended`. `fastestFee` is the tightest tier they name and the nearest above the two blocks this claim is priced for; erring fast is the safe direction when an unconfirmed claim loses the fill, with `halfHourFee` as the looser fallback. Esplora's shape is kept as a second source rather than trading one hardcoded path for another — the base URL is otherwise Esplora-compatible and the SDK's default provider speaks it. A 200 carrying HTML is now refused on content-type instead of parsing as an empty fee set: the SPA answers both paths at a base URL missing `/api`. When no source can be read at all the wallet says so through `consoleError` rather than absorbing it, which is how this survived to mainnet. `MIN_CLAIM_FEE_RATE` keeps both of its jobs untouched — the fallback rate and the believability bar it sets through `usableRate`. * fix(claim): keep the fee read from throwing on an unnamed endpoint `748330df` moved `baseUrl.replace()` out of the try block, so a base URL the endpoint map could not name threw out of `claimFeeRate` rather than falling back to the floor, and leaked the abort timer with it. The original caught that; this restores the guarantee and reports the cause instead of swallowing it. Reachable because `providers/swaps.tsx` reads the endpoint through an unchecked `aspInfo.network as NetworkName` cast, so `ESPLORA_URL[network]` is `undefined` for any network outside the SDK's map. Throwing there blocks the send router outright, where the floor would merely have made the claim slow. Found by re-reading a review observation against the new code rather than the old. Removing the restored catch fails the new test with `TypeError: Cannot read properties of undefined (reading 'replace')`. * fix(send): stop pinning an amount the invoice already pins A Lightning send was refused with "Amount outside solver bounds (1,000-25,000 sats)" for a 1,000-sat invoice - bounds the amount satisfies. Two defects, one visible. The rail never answered. `lightningRail.available()` forwards `req.amount` into `client.resolve()`, and an amount-bearing invoice already pins the take leg, so the client throws `AmountMismatch` ("exactly one amount may be pinned") even when the two agree. `PaymentRouter.options()` warns that error to the console and drops the rail, so every amount-bearing invoice lost its only route. The unit suite missed it because the fake client's `resolve` stub answered `eligible: 1` to anything and never ran the real resolution. `lnSendRequest` leaves the amount to the invoice and carries one only when the invoice names none. The tests now drive the SDK's own `resolveRoute`, so this contract cannot drift unnoticed. `lnSendRefusal` asserted a cause it could not know: `options()` reports absence, not reason, so any drop - no route, an unreadable feed, a bounds miss on another card - rendered as "outside solver bounds", sending the operator to re-check the one config that was not at fault. It now names bounds only where it has checked them and none admit the amount, spans every lightning card rather than the first, and otherwise says the router does not report the reason and points at the log. That reason cannot be plumbed through `options()` today: it returns `{railId, quote}` only, and the rail's error is logged then discarded. Surfacing it needs an SDK change. * fix(send): keep an amount the destination named out of the fiat round-trip Two faults on the same screen, both reported against a 1,000-sat invoice. The sats readout drifted. Pasting an invoice writes the field text with `getTextValue`, which in fiat entry renders the amount to two decimals; `InputAmount` then re-derives sats from that text because no authoritative `valueSats` was given. At the reported rate one cent is worth ~13 sats, so the round-trip cannot be lossless: the screen showed 0.00000994 BTC for a 1,000-sat invoice. The pinned amount is now handed to the field directly, for an invoice and for an LNURL resolved to a fixed amount, and is cleared again when the recipient changes. `sendInfo.satoshis` itself was never corrupted by that path - the field is read-only for an invoice, so no keystroke reaches the write-back. The "N available" control was the one that did: it stayed a tap target while the amount was pinned, and `applySendAll` wrote the whole balance over the invoice's amount. Confirmed in test: a tap wrote satoshis 1,000,000 for a 2,100-sat invoice. It is no longer offered when the destination named the amount; `InputAmount` already withholds its own Max button there. A guard in `handleSendAll` was tried and dropped: no reachable caller survives once the control is withheld, and the programmatic LNURL caller runs before the flag it would read is set. * fix(send): log the rail the on-chain preview could not price Reported on #944: a 22,000-sat send showed a fee of zero, then logged "onchain-swap refused: quote pays 22000 for 22709, screen shows 22000 for 22000" and paid through the collaborative exit instead. The guard did its job. `previewOnchainCost` walks the rails in priority order, so the swap rail is priced first whenever it survives `available()`; a screen total equal to the amount means that pass never got a swap quote and the exit's own figures stood. The spend then found the swap rail quotable, at 22,709 against a screen committed to 22,000, and `quoteIsForThisSend` correctly refused a rail asking more than was shown. What is missing is the reason. `previewOnchainCost` swallowed every quote failure on the stated assumption that the spend "walks the same rails moments later" and would log it - and that assumption is exactly what breaks here, because the spend reaches a DIFFERENT, successful answer and nothing records why the preview did not. It now names the rail it could not price. The added screen test pins the sequence: solver absent from the pricing pass, present for the spend, guard refuses, exit pays. It characterises the behaviour rather than blessing it - whether a send may change rails under the user without re-confirming is a product decision, still open. * one tab drives, every tab acts: a follower delegates swap actions to the lock holder instead of being refused Claude-Session: https://claude.ai/code/session_016byhUnVjqXASJNJHqHum71 * fix(send): return at funding, and find the swap record by rfqId Four defects, one live-reproduced root cause. ACTIVITY METADATA NEVER GRAFTED. A send saved `{destination, networkFee}` keyed by the funding txid and the row rendered neither — fees read $0.00 and the destination row was absent, on BOTH rails. Confirmed live: the wallet stored networkFee 54 under 95d9477f and 862 under 80eca7e0, while the rows rendered were keyed 98fa9ba9 and 1d51bb5d. Not two txid identities: `wallet.send()` returns `arkTxid`, the worker forwards it verbatim, and history keys its SENT row `arkTxid: vtxo.arkTxId`. Writer and reader already agree. The funding tx pays a covenant this wallet registered, so its output counts as change, the row nets to zero, and the `txAmount !== 0` guard drops it — leaving the lockup's SPEND as the only SENT member of the group. `lightningSendTx` anchored both the graft and the record lookup on that member, which is a different transaction. `corridorSendTx` now finds the record by `rfqId` and grafts under `record.fundingTxid`. SPINNER HELD TO TERMINAL. `payLightning`/`payOnchain` awaited `.settled()`, so the screen waited out the counterparty's whole pipeline — 63s measured for an on-chain swap. `fundedResult` resolves on the first `sent`/`settled` and rejects on `failed`, which is what the docblock above `payLightning` already described and the code did not do. The #949 no-catch property is preserved. WRONG-DIRECTION TOAST. `paid`/`claimed` are how both directions succeed, so the outcome alone cannot tell a send from a receive and every send announced "sats received". Discriminated on `route.take.corridor`. ON-CHAIN OVERCLAIMED. It said "sent successfully" where Lightning says "on the way" — and on-chain is the weaker claim of the two: `ONCHAIN_DRIVE` declares `actions: ["claimOnchain", "refundArkade"]`, so the trader must claim the L1 HTLC. The drive runs in the foreground provider (the service worker has no swap code), so closing the tab forfeits the claim; the on-chain screen now says so. Lightning is left alone, where the solver claims and "on the way" is complete. Not changed, deliberately: the amount/total decomposition. `Transaction.tsx` passes gross for both by a wallet-wide convention a plain Arkade send shares, so narrowing it here would alter every sent receipt. tsc 0. Unit suite 94 files/804 -> 95/819 passed, 1 skipped, 0 failing. * Adapt to @arkade-os/sdk 0.5.0-rc.4 / swap 0.1.0-rc.4 * fix(send): read the swap's own state, and show what the corridor actually did The status came off the funding transaction. A corridor send carries no `assetSwap`, so `swapStatusForTx` fell through to `tx.settled` — the confirmation state of one transaction, not the swap. A funded lockup nobody has spent yet is a settled transaction and an in-flight swap, and the detail screen read `Settled` for it with no completion row to show. It now reads `lnSwap.outcome`, the record's own state already projected onto the resolver's tokens, and `refunded` renders as "Refunded" rather than "Failed": nothing broke, the covenant gave the money back. The screen also could not say what the swap did. `useLnSendReceipt` carried two txids and nothing else, so Lightning and on-chain were indistinguishable and a completed on-chain send showed only the lockup spend — which proves the solver took its side, never that the recipient was paid. Widened into `useCorridorSendReceipt`, which owns every corridor field: the corridor, what the recipient gets against what left, the solver, the swap fee distinct from the network fee, and the L1 claim that actually pays, linked to the block explorer rather than Arkade's. The on-chain lockup spend is now labelled "Lockup spent"; Lightning keeps "Completed", where the hash-verified spend IS the invoice being paid. Not a third branch in `Transaction.tsx`: a corridor send is an ordinary payment plus swap facts, so one would restate ten generic rows to add six. The hook owns the swap half instead. Covenant internals — the L1 HTLC address and the refund deadline — go behind a local collapsible. Settings' "Advanced" is a navigation target, not a preference, so there is no flag to gate on; and the existing covenant surface, Settings > Advanced > Contracts, structurally cannot hold an L1 HTLC because it is not an Arkade contract row. Corridor ids are `arkade | lightning | onchain`. `bitcoin` is the RAIL that `onchain` settles on, and comparing against it left every on-chain send unlabelled; a test pins the literal against discovery's own constant. Drops the `RouterContext.wallet` double cast, which rc.4 makes unnecessary now that the field is typed `IWallet`. Closes #950. * fix(swaps): stop replaying history as news, and group a claimed receive Four bugs found in a live six-corridor run. **Completed swaps re-announced on every load.** The client's `restore()` ends by calling `emitAll()` over every record it read, and this provider subscribes before `client.ready` — so each page load replayed the whole history as terminal updates and `announce` toasted every one of them. Six at once on the load that caught it, and it scales with history. The drive's own dedupe is per instance, so a reload builds a new one and it cannot help. `announce` now speaks only past the restore, which takes the balance reload with it: fifty old swaps used to reload the wallet fifty times on open. **A claimed Lightning receive formed no group.** The resolver registered `fundingTxid`, `refundTxid` and `lockupSpendTxids`, but the package's own `activityTxids` also collects `profile.claimTxid` — and a CLAIMED receive has none of the other three: the solver funds it, the trader holds no refund, and a lockup spend is the solver reclaiming. Its claim was the only transaction identifying it, so no group formed and the row fell back to a bare "Received", indistinguishable from any other receive. **Onboarding stayed in the DOM.** `screen` starts at `Init` and the Init screen is what bounces an existing wallet on to Wallet, so onboarding mounted on every boot. Invisible under the wallet, but focusable and read aloud — a screen reader offered "Create wallet" to a wallet that already existed. Answered in the page derivation rather than with another `navigate`, which would race Init's own. **The completion toast said USDT where the wallet says USD.** `tickerFor` read the market card's ticker; every other surface — the account list, the picker, the review screen, the balance — resolves the designated currency first. It now does too, and only for a verified asset, or a lookalike mint could claim a currency. * fix(activity): group a receive on its lockup's funding tx, not on its claim A claimed Lightning receive rendered as a bare "Received", indistinguishable from an on-chain deposit, while every send labelled correctly. **`vtxo.arkTxId` names the transaction that SPENT the output, not the one that created it.** Three of us read it the other way, and that inversion is what hid this. Reading it correctly against a live wallet: the vtxo the record points at is the SOLVER'S LOCKUP — created by the solver's funding tx, spent by our claim — and the claim's own output sits at a different script and is suppressed from history as change from our own spend. So the RECEIVED row history builds is keyed on the lockup's FUNDING txid, and a receive record names no such field: it has no funding tx of the trader's, no refund, and — once claimed rather than reclaimed — its `lockupSpendTxids` and `profile.claimTxid` both hold the claim. Every identifier we had was the wrong transaction, which is why adding another one did not help. The asymmetry with sends falls out of the same fact: a SENT row is keyed on `vtxo.arkTxId`, the spending tx, which is exactly what `wallet.send()` returns — so sends compare like with like and receives never could. The fix is the one the package already had and this resolver dropped: `rfqSwapActivityInputs` reads the vtxos at the lockup script and registers `vtxo.txid` when the record carries no funding txid of its own. The hand-maintained field list was NOT the bug — the per-corridor `activityTxids` returns the same claim txid we already had — so do not "simplify" the read out again; `if (wantFunding) out.push(vtxo.txid)` is the line that matters. The wallet's own `getVtxos()` cannot serve it: it drops anything `hasTerminalSpend` matches and a claimed lockup is spent by definition. The `ArkadeReader` from the service worker can, needs no client lock, and so still works in a tab that is not driving. An empty read is logged rather than swallowed, so "found nothing at a script the record names" stays distinguishable from "the read failed". * fix(swaps): mark a replay across tabs, and quieten a normal empty lockup Two review findings, both real, both introduced by earlier commits on this branch. **A replayed outcome is now marked on the update itself.** `restoredRef` lived at component scope, so once the first client finished restoring the gate was open for good — and a network switch re-runs that effect in the SAME instance, building a client whose `restore()` replays the whole history again. Every terminal swap would toast a second time. Resetting the ref per client fixes that, but not the other half: the flag was per TAB and only the lock holder ever set it, so a follower tab never announced anything at all — the opposite of what the provider documents ("Every tab announces, deliberately"). So the replay bit now travels ON the update. The driver marks its own replay and publishes the mark; a follower applies the state either way and announces only what is news. **An empty lockup read is normal until the counterparty funds.** `lockupTxids` logged an error whenever a lockup had no virtual output, but on a receive the covenant exists from the moment it is derived and holds nothing until the sender pays — so every history load during a live receive logged one. An error that fires routinely is how the real one gets ignored. Only an ENDED swap with nothing at its lockup is anomalous, and only that is loud now. **Plus a regression guard for a bug that already happened once.** The solver rail is gated on `onchainClaimEndpoint`, which once read `explorers.ts` — a map that builds explorer links and has no mainnet `api`. The endpoint was therefore `undefined` on mainnet and the rail went unregistered there while working on every network it was tested on. That was fixed in 8aacb5da; this asserts the endpoint resolves for EVERY network rather than the one that broke, because two maps that can disagree is the shape of it. Pointed back at `explorers.ts` the guard fails on `bitcoin` alone. * fix(receipt): stop a corridor send billing the solver spread twice The receipt showed the solver's fee under two labels. "Swap fees" renders `swapFeeSats`, the quoted spread; "Network fees" renders `Details`' own `fees`, and on this path that is `tx.networkFee` -- which the send screen saved from the SAME corridor quote. The totals proved only one fee was charged: $5.11 out, $5.06 delivered, two rows at $0.06. The mechanism is a gap in a spread. `Transaction` builds the row with its own `fees`, then spreads `...corridorReceipt` over it; `CorridorSendReceipt` set every `Details` field it owns EXCEPT `fees`, so the value underneath leaked through. The asset-swap branch of the same ternary already passes `fees: 0`; this makes the corridor branch do the same, by giving the receipt a required `fees` it always spreads. Fixed here rather than at the source (`Send/Details.tsx` writing the spread into `networkFee`): both would correct new sends, but only suppressing the row also corrects receipts ALREADY stored, which would otherwise render a phantom network fee forever. A corridor send has no separate network fee being hidden: the lockup is funded offchain, and the on-chain rail prices its L1 claim out of the recipient's payout rather than charging the sender a second time. * refactor(details): collapse the two rows both labelled "Swap fees" `Details` defined the label twice -- once for `swapFees`, the asset-denominated `SwapDisplayAmount`, and once for `swapFeeSats`, a number. `Table` keys rows by their title and renders it as the DOM id and the test id, so any receipt setting both would render the label twice and leave `Swap fees` ambiguous to `getElementById` and to `getByTestId`. Latent, not live: an asset swap sets the first and a corridor send the second, and no transaction is both. Behaviour is unchanged today -- the absent row rendered null, so the surviving row keeps its position, label and icon. * fix(send): stop an unanswered solver holding the sign button #944 put the router's rail loop in front of Tap to Sign: the screen prices the send through `previewOnchainCost` before it will offer a signature. `options()` drops a rail whose `available()` refuses or throws, but nothing bounds one that simply never answers, and the nostr RFQ's own deadline is 30_000ms. So a solver that is advertised but silent held the button for the whole 30s, and `send-onchain-fallback.test.ts` failed on `locator.click: Timeout 30000ms exceeded — waiting for getByText('Tap to Sign')`, not on anything it asserted about the fallback. The fallback design was never reached, because the refusal never arrived. `withinPricingBudget` gives that pass a deadline of its own. Expiring costs only the dearer route: the exit rail's figures are already on screen and are correct, and `quoteIsForThisSend` then holds any solver to them. The SPEND is deliberately left unbounded — the user has committed by then, and the RFQ's own 30s refusal is what names the reason the fallback tests assert. * fix(send): say "sent" when the collaborative exit really did send The exit rail's own success is reported as if it were a swap. #944 decided the success copy from the DESTINATION's shape: const isOnchainSend = Boolean(sendInfo.address) && !arkAddress && !invoice so every on-chain send is headlined "Payment is on the way" and told to "keep the wallet open until it completes". That is true of the solver rail, which funds a covenant and still owes an L1 claim. It is false of the collaborative exit, which calls `wallet.settle()` and returns a broadcast txid: nothing further is coming, and the wallet may be closed. Instrumenting the failing CI run showed the exit paying in 615ms with a txid — and the screen still not saying "Payment sent". So `send.test.ts:356`, green on master since #460/#485, and the three `send-onchain-fallback` tests were all failing on wording, not on routing. The rail that paid now rides in `sendInfo.railId` and decides the copy. An unknown rail keeps the cautious wording, so #944's two existing cases are unchanged: only a rail we know settled outright earns "sent". * test(send): assert the refusal the RFQ actually raises `expectRoute(page, /not responding/)` matched a string no code path has ever produced: "not responding" appears nowhere in @arkade-os/swap, the SDK or this wallet — only in this assertion. The transport raises `no solver reply within ${timeoutMs}ms`. The test never ran in CI (the `pull_request: branches` filter matched the base), so nothing caught it. Its purpose is unchanged and still discriminating: the socket case asserts `/no relay accepted/` and this one the timeout, so the two cannot pass as each other. * fix(send): handle a rejection on the pricing tail Review nit from arkana-ai-bot: `withinPricingBudget(priced).then(settle)` had no catch. `priced` cannot reject — it ends in a `.catch` that returns undefined — but that is a convention the helper's doc asserts rather than enforces, and a throw out of `settle` itself would surface as an unhandled rejection. Logging it costs nothing and removes the dependency on the convention holding. * add covclaimd * update deps to rc-8 * fix: accept canonical solver cards * test: use unsupported solver card version * chore: update swap to 0.1.0-rc.9 * chore: bump @arkade-os/swap to 0.1.0-rc.10 * chore: bump @arkade-os/swap to 0.1.0-rc.11 * fix race where lightning swap was created after receiving * update @arkade-os/solver-discovery dependency * chore: bump @arkade-os/sdk to 0.5.0-rc.9 and @arkade-os/swap to 0.1.0-rc.12 Both move together because swap@0.1.0-rc.12 pins sdk@0.5.0-rc.9 exactly; the lockfile resolves one copy of each. Gate: lint and 863 unit tests pass. * fix test * fix(activity): load history once after client.ready (#1007) * fix(activity): load history once after client.ready A second history load is not needed. Start the swap client first so client.ready can rebuild offer records, then fetch Activity once. * test: remove swaps provider unit tests Drop src/test/providers/swaps.test.tsx from this branch. * fix(activity): load history even if swap client.ready fails Keep a failed restore from pinning the loading screen, restore the SwapsProvider tests, and document the ASP-empty wallet reload fallback. * chore: drop extra wallet reload fallback comments * test: remove swaps provider unit tests * unit tests for providers/swaps * remove double refreshSwaps on startup --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: João Bordalo <bordalix@users.noreply.github.com> * feat(swap): price canonical solver cards A published card names assets as CAIP-19 and omits pair and corridor fields. Pinned cards are down-projected to the short ids the balances and the offer covenant speak, the same rewrite the registry reducer applies to a published index. Lightning bounds and the swap composer read the rail from the asset id, and Settings rejects a card whose chain reference is not the wallet's network. * Revert "feat(swap): price canonical solver cards" This reverts commit 43ede00269d8f415e661397c8c2289dab9c2f024. * update dependencies * update pnpm-lock.yaml * update bun.lock * remove bun.lock * update AGENTS.md * don't show assets with slip44 on it's id (#1022) * chore: update wallet v0.5 to SDK and swap RC releases * rename ArkInfo (deprecated) to ArkadeInfo * remove arkServerUrl from requestLightningSend * remove arkServerUrl from requestLightningReceive * changes for sdk v0.5 * fix: use release candidate from npm * Upgrade to 0.5.0 RC 1 * move swaps to the v2 createSwapClient: one client for offers, lightning send and receive Claude-Session: https://claude.ai/code/session_01W5xDq73G5jGA23XG8B9SLr * move swaps to the v2 client: pay, receive and exchange from @arkade-os/swap/client Claude-Session: https://claude.ai/code/session_01W5xDq73G5jGA23XG8B9SLr * move to the M8 swap surface: the v2 client is the @arkade-os/swap root, v1 is /protocol Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * drop V1 swap support: one keyspace, one resolver, no chain rebuild for offers Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * fix: resolve one network for discovery, so an unnamed one keeps no solver cards Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * upgrade to 0.5.0-rc.3 / 0.1.0-rc.3: the promoted names come home, record reading moves to /advanced Claude-Session: https://claude.ai/code/session_01AiLnFz7WU9q6J88EToQELR * feat(send): route on-chain sends through PaymentRouter, solver rail first Supersedes #941. That PR shipped a solver-routed onchain send with ~2,600 lines of bespoke route selection, bounds matching and fallback — all of which `PaymentRouter` (ts-sdk #837) already does. This is the same corridor expressed as a rail registration. What the router replaces, and the proof it covers it: every arm of #941's `OnchainRouteRefusal` enum becomes `available()` returning false, and a rail that says no is dropped from `options()` without taking the router with it, so the collaborative exit wins by ranking. `sendRouter.test.ts` asserts one case per deleted arm — no_solver, amount_out_of_bounds, discovery_failed, unsupported_address, no_l1_endpoint — alongside a control proving the solver rail really can rank first, without which those five would pass vacuously. `chainSourceFrom` likewise replaces #941's 222-line hand-written esplora client. The wrong-address guard survives, differently. #941 carried a negotiated quote across the Form -> Details navigation and compared its `payoutAddress` to the screen; `RouteQuote` has no address field, so that comparison has no home. Instead nothing is carried: `PaymentOption.quote()` is lazy by contract, so the sign screen quotes against the address it is displaying at the moment of the spend, and a stale quote is not a thing that can exist. `quoteIsForThisSend` is the belt to that braces, and is stricter than the check it replaces — the screen must be showing the routed address, the recipient must get what was displayed, and no more may leave the wallet than was displayed. Two rails, and only one of them is the SDK's. `solverOnchainRail` comes from `@arkade-os/swap`; the collaborative exit stays this wallet's own `collaborativeExitWithFees` rather than the SDK's `onchainRail`, because that one settles through `Ramps.offboard` with its own coin selection while this wallet deliberately spends soonest-expiring VTXOs first. Adopting it is a coin-selection change, separate from adopting the router. Amounts are receiver-exact, as `RouteQuote` specifies, but the send form still lets the user type what LEAVES the wallet. The translation happens at the screen boundary rather than in the rails: a typed amount interpreted as receiver-exact would gross up past the balance it came from, so "send max" could never succeed. `isBTCAddress` now delegates to the SDK's `isBtcAddress`, so the form classifies a target exactly as `btcTarget` does. That changes which addresses the form accepts, in both directions, and the four cases are pinned in `address.test.ts`. `@arkade-os/swap` is vendored from ts-sdk `fa3a2d1f` — the #837 squash — because no published version carries the rails. See vendor/README.md for why the SDK did not need vendoring alongside it, and for the version-collision trap that makes the `pnpm.overrides` entry load-bearing. * test(send): cover the sign screen's routing, which nothing exercised The router and the guard had unit tests, but no test rendered `SendDetails` with a plain BTC address and pressed sign — `send-details.test.tsx` only covers the amount hierarchy. So the whole `payOnchain` path was green by absence, which is exactly what a suite cannot tell you. Five cases at the screen that moves the money: the exit is taken when no solver rail survived, a rail quoting a different recipient amount is refused, a rail that would spend more than the screen displayed is refused, a matching solver quote IS funded, and — the wrong-address regression from 1b3481b — the request is built from the address the screen is showing. The mocked `options()` takes the request and builds the exit rail from `req.raw` rather than a constant. Without that the last case asserts against a hardcoded address and proves nothing; mutating `raw` to a fixed address kills it, as does deleting the `quoteIsForThisSend` call. * fix(send): three CodeRabbit findings, one of them a real money race **A funded reservation could be deleted on restart.** `isUnfundedReservation` treated a successful-but-empty lockup lookup as proof the swap was never funded. The record is written BEFORE the funding is broadcast, and the indexer learns of it later still, so a restart in that window read a funded swap as an abandoned reservation and dropped it — losing an L1 claim only this wallet can make. It now keeps anything younger than `RESERVATION_GRACE_SECONDS`. The asymmetry is the whole argument: the cost of keeping a stale row is a "Pending" line that prunes itself, and the cost of dropping a funded one is the fill. **The history row could show a fee nobody paid.** `handleSent` persisted `details.fees` — the number the screen displayed — while `quoteIsForThisSend` deliberately admits a quote whose total is LOWER than displayed. It now persists `quote.fee`. **The form accepted addresses nothing could spend to.** `isBtcAddress` is format-only by design, because a rail's `match()` runs before anything is known, so `bc1qqqqqqqq` and bad checksums reached the sign screen and failed there. `isBTCAddress` now also requires the address to decode on one of the three L1s. That is stricter than the SDK predicate but only on inputs that were never spendable, so the form and the rails still agree on every valid address. Fixing that exposed a bad fixture of my own: `2N2JD6wb56AFK4d9ppKAftvBoT9Cw3xL1Tm`, which I had cited as a testnet p2sh the old regex rejected, has an invalid checksum and fails on all three networks. Replaced with a verified one. Declined the fourth: `reserveOnchainSend`'s `managerRef.current?.addSwap` is optional on purpose, and matches `trackLnSend`. The record is saved first and is the invariant that matters — a reservation the manager never hears about still restores and is driven on the next start, so throwing would refuse a send the wallet can make in exchange for nothing. * fix(send): the exit rail must not claim ark addresses or invoices `match: (req) => Boolean(req.raw)` accepted any non-empty target, so the collaborative-exit rail would have claimed an ark address or a BOLT11 invoice if the router were ever asked to route one. It never is today — `payOnchain` is reached only from the plain-BTC branch — but a rail's `match` is its contract, not a note about who currently calls it. Now the same `btcTarget` classification the SDK's own `onchainRail` uses. Asserted rather than described: routing an ark address or an invoice returns no options at all, and reverting the predicate kills it. * docs(send): say that the reservation grace period is a guess `RESERVATION_GRACE_SECONDS = 600` is a judgement, not a measurement — nobody has profiled worst-case indexer lag after a funding broadcast. The comment now says so, and names the symptom of it being too short: a funded record dropped on restart, losing an L1 claim only this wallet can make. That failure is silent — the row simply disappears — so anyone with real lag figures should set the number from them rather than trusting a bare constant. * fix(send): take the first USABLE fee bucket, not the first present one CodeRabbit finding on the rebased head, and correct. `estimates['2'] ?? estimates['1'] ?? estimates['3']` falls through only on null/undefined, so an esplora reporting `{"1": 25, "2": 0}` had its zero taken, rejected by the validity check, and the claim dropped to the 1 sat/vB relay floor — with a perfectly good 25 sitting in the next bucket. That matters here more than it looks: `claimOnchainFill` refuses inside the refund leaf's margin, so a claim that sits unconfirmed is a claim that loses the fill. Underpaying by 24 sat/vB on a transaction racing a consensus deadline is the failure this function exists to avoid. Now picks the first bucket that is actually usable. Carried over from #941 unquestioned, like the reservation race — worth noting that both bugs came from code I ported rather than code I wrote. * fix(send): the solver rail was reading the wrong endpoint map `onchainClaimEndpoint` resolved through `src/lib/explorers.ts`, which exists to build explorer LINKS: `api` is optional there and `bitcoin` carries only `web`. So the endpoint came back undefined on mainnet, `createSendRouter` skipped the registration, and every mainnet on-chain send fell through to the collaborative exit — the PR's headline feature was a no-op in production. The SDK already ships the map this wanted: `ESPLORA_URL`, a `Record<NetworkName, string>` that is total by type, with mainnet on Arkade's own instance rather than mempool.space. Reading it there makes the endpoint unconditional, so the `if (onchainClaimEndpoint(...))` gate can go: what drops the solver is `available()` — no card serving the corridor, or an amount outside the ones that do — which is the real gate and was already tested. The test that pinned the old behaviour asserted mainnet gets only the exit rail, using bitcoin as the example of a network with no L1 endpoint. There is no such network now, so it asserts the opposite, and a second case pins that every `NetworkName` resolves an endpoint. The same change makes two branches in `lnSwaps.tsx` unreachable — `esploraUrl` can no longer be undefined — so the chain source is built directly and the "no L1 endpoint is configured" throw, which could no longer fire, is gone. Also from review: - A blank `details.destination` (what the screen stores when `utxoTxsAllowed()` is false) reached the router and failed `quoteIsForThisSend` on every rail, surfacing as "No route for this payment" when the real constraint was the account limit. Guarded at the callsite beside its siblings. - `l1NetworkOf` folding signet and mutinynet into 'testnet' is deliberate — they share HTLC script parameters — and now says so. * test(send): drive claimOnchain from the record it will really read The callback that claims the L1 HTLC had no test at all: `sendRouter.test.ts` stops at the quote, `send-onchain-route.test.tsx` stops at `send()`, and `lnReceive.test.tsx` only asserts that the receive leg installs no `claimOnchain`. Everything between a record written before funding and the arguments `claimOnchainFill` is handed was uncovered — which is the half where losing the money is possible. Real `onchainSendSwapRecord`, real `rfqClaimSecretOf`, real `preimageForSwapRecord`, real `claimPayoutScript`, real `claimFeeRate` against a stubbed fetch; only the manager and the fill itself are mocked. `mustPersistPreimage` puts the raw P in the record, so the round trip needs no signing wal…
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
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.
Closes #934
Upgrades the wallet to SDK 0.5 and the native swap-client/PaymentRouter APIs: send/receive routing, SDK record-based history and recovery, receipt amounts and fees, and restoration before the swap client starts.
This SDK 0.5 upgrade is based on master. Taxi integration #1058 is based on ts-sdk-0.5. This parent has no Taxi imports, /v1/fills calls, carrier archives or native Taxi browser jobs. All four standard browser lanes remain registered.
Preserves the shared branch's SDK 0.5.0-rc.22, swap 0.1.0-rc.25 and solver-discovery 0.3.0 upgrades, Nostr market refresh, dust-carrier balance correction and nak dependency-download retries. Wallet and SDK use btc-signer 2.4.1. The Taxi child owns its separate feature archives and consumer validation; published and feature packages are not claimed equivalent.
The October 15 refresh appends master through 37a64cd without rewriting shared ancestry. This includes missing asset-decimal metadata defaulting to whole units and the atomicToUnits/unitsToAtomic helper rename across the SDK 0.5 call paths. The animation conflict retains master's shared bounce keyframes and the SDK branch's per-effect cancellation/layout cleanup; lifecycle tests cover stale animation completions, StrictMode and unmount. Master's keyboard-accessible segmented control, mobile app banner and action-version updates are retained.
At cea3b7f, a clean Linux snapshot verified byte-for-byte against all 531 indexed blobs passes frozen install, typecheck, all three original pre-commit checks (format, lint and CSP), and all 100 unit files / 962 tests (baseline 961). The stalled Windows cold-collection run was stopped incomplete; the same complete suite passed in Linux without changing test selection or deadlines. Exact-head CI also passes all 100 files / 962 tests, app/worker build, both previews and all four standard browser lanes (core and assets/send on Google Chrome and Mobile Chrome: 75 passed, 37 existing skips, matching the baseline skip counts).
Formal, inline and issue feedback were read after CI settled. CodeRabbit's current incremental review is skipped because its saved history cannot establish the review base; its last reviewed checkpoint remains 190e04e. No fresh CodeRabbit coverage is claimed. Package-side on-chain signing review, Nostr discovery trust-boundary review and human approval of pre-release dependencies remain open. No merge or production deployment is included.