Boltz swap : offline receive#609
Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| testnet: null, | ||
| } | ||
|
|
||
| const BANCOD_URL = import.meta.env.VITE_BANCOD_URL |
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: Boltz swap — offline receive (wallet#609)
This PR wires the offline-receive feature from boltz-swap#141 into the wallet UI. The wallet-side changes are thin — pass-through of offlineReceiveOptions to two swap creation calls. The protocol-critical logic lives in boltz-swap, which I already reviewed with changes requested (boltz-swap#141).
🔴 Blocking: Dependency pinned to unmerged branch
package.json:7
"@arkade-os/boltz-swap": "github:arkade-os/boltz-swap#offline-receive"This pins to a moving branch target that has outstanding changes requested on boltz-swap#141 (orphaned swap on registerOfflineReceive failure, missing claimAddress validation, Buffer usage in browser context). This wallet PR inherits all those bugs.
This PR must not merge until boltz-swap#141 is merged and published as a proper semver release. The dependency should then be pinned to that version, not a git branch.
🟡 Medium: No validation of VITE_BOLTZ_INTROSPECTOR_PUBKEY format
src/providers/swaps.tsx:34-38
const BANCOD_URL = import.meta.env.VITE_BANCOD_URL
const BOLTZ_INTROSPECTOR_PUBKEY = import.meta.env.VITE_BOLTZ_INTROSPECTOR_PUBKEY
const offlineReceiveOptions =
BANCOD_URL && BOLTZ_INTROSPECTOR_PUBKEY
? { bancodUrl: BANCOD_URL, introspectorPubkey: BOLTZ_INTROSPECTOR_PUBKEY }
: undefinedintrospectorPubkey is a hex-encoded secp256k1 public key that directly controls who can claim VHTLCs. A malformed value (wrong length, non-hex, invalid point) will produce a confusing error deep in the swap flow rather than failing fast at init.
Add basic validation:
if (BOLTZ_INTROSPECTOR_PUBKEY && !/^[0-9a-fA-F]{64,66}$/.test(BOLTZ_INTROSPECTOR_PUBKEY)) {
console.error('Invalid VITE_BOLTZ_INTROSPECTOR_PUBKEY — expected 32 or 33 byte hex pubkey')
}🟡 Medium: Offline receive silently changes custody model with no user signal
src/providers/swaps.tsx:222, :271-274
When offlineReceiveOptions is defined (env vars are set), every BTC→ARK and reverse swap silently delegates VHTLC claiming to bancod. The user has no visibility or opt-in. This is a significant trust change — the user is now trusting a third-party solver with preimage custody.
Consider:
- Making this opt-in per swap, or at least surfacing it in the UI
- At minimum, logging when offline receive mode is active
🟢 Minor: console.error change is good
src/lib/logs.ts:50 — Adding the raw error object to console.error improves debuggability. No issues.
🟢 Minor: Type declarations are correct
src/vite-env.d.ts:10-11 — Both new env vars correctly typed as string | undefined. Fine.
Summary
The wallet-side integration is straightforward — it's a thin pass-through. The real risk is in the dependency:
- boltz-swap#141 must merge first with the critical fixes I flagged there (orphaned swap on registration failure, claimAddress validation)
- Pin to a published version, not a git branch
- Validate the introspector pubkey at init time
- Consider user consent for the changed custody model
🏷️ Protocol-critical — this delegates VHTLC claiming to a third-party solver. Human review required.
| const str = (msg ? `${msg}: ` : '') + extractError(err) | ||
| addLog(LogLevel.Error, [str]) | ||
| console.error(str) | ||
| console.error(str, err) |
There was a problem hiding this comment.
seems better to have dedciate PR for this?
Deploying wallet-bitcoin with
|
| Latest commit: |
81f2545
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4e77d19c.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://swap-offline-receive.wallet-bitcoin.pages.dev |
Deploying tmp-boltz-upstream-mainnet-arkade-wallet with
|
| Latest commit: |
81f2545
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3864390a.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
| Branch Preview URL: | https://swap-offline-receive.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
81f2545
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5547e075.arkade-wallet.pages.dev |
| Branch Preview URL: | https://swap-offline-receive.arkade-wallet.pages.dev |
There was a problem hiding this comment.
Follow-up review: merge commit 81f2545
Reviewed the new commit — it's a routine merge of origin/master into the feature branch. The merge brings in a TailwindCSS/Sonner UI refactor, SDK bump to 0.4.25, and VITE_DEV_NSEC env var. None of the swap-related feature files (src/providers/swaps.tsx, src/lib/logs.ts) were modified.
All my previous findings still stand:
- 🔴 Blocking:
boltz-swap#141is still open with changes requested — this PR must not merge until that dependency ships as a versioned release - 🟡 Validate
VITE_BOLTZ_INTROSPECTOR_PUBKEYat init - 🟡 User consent for changed custody model
No new issues introduced by this merge commit.
2 participants
No description provided.