Skip to content

feat: add @arkade-os/wallet-providers package#2

Open
Kukks wants to merge 2 commits intomasterfrom
feat/wallet-providers
Open

feat: add @arkade-os/wallet-providers package#2
Kukks wants to merge 2 commits intomasterfrom
feat/wallet-providers

Conversation

@Kukks
Copy link
Copy Markdown

@Kukks Kukks commented Apr 1, 2026

Summary

  • New @arkade-os/wallet-providers package with browser wallet Identity implementations for UniSat, OKX, Leather, and Phantom
  • UniSat and OKX implement BatchSignableIdentity (batch PSBT signing in one popup)
  • Leather and Phantom implement standard Identity (sequential signing)
  • Shared BrowserWalletIdentity base class handles pubkey normalization, signerSession stubs, and PSBT merge+validation
  • Each provider is independently importable via subpath exports (e.g. @arkade-os/wallet-providers/unisat)
  • Adds signMultiple() to existing SatsConnectIdentity using sats-connect's signMultipleTransactions API for batch signing
  • Includes a Vite test app at apps/wallet-providers-test/ for manual browser testing

Depends on

Provider matrix

Provider Package PSBT format Batch signing
Xverse (sats-connect) @arkade-os/sats-connect (updated) base64 Yes (signMultipleTransactions)
UniSat @arkade-os/wallet-providers hex Yes (signPsbts)
OKX @arkade-os/wallet-providers hex Yes (signPsbts)
Leather @arkade-os/wallet-providers hex No
Phantom @arkade-os/wallet-providers Uint8Array No

Test plan

  • TypeScript type-checks clean (both packages)
  • tsup build succeeds for both packages (ESM + CJS + DTS)
  • Manual test with each browser extension using apps/wallet-providers-test/
  • Integration test with real Arkade send (requires signet VTXOs)

Browser wallet provider Identity implementations for the Arkade SDK:
- UnisatIdentity (batch signing via signPsbts)
- OkxIdentity (batch signing via signPsbts)
- LeatherIdentity (single sign only)
- PhantomIdentity (single sign only)

UniSat and OKX implement BatchSignableIdentity, enabling the SDK
to batch-sign all checkpoint + main tx PSBTs in a single wallet popup
(depends on arkade-os/ts-sdk#395).

Includes a Vite test app at apps/wallet-providers-test/ for manual
testing with installed browser extensions.
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
arkade-checkout Error Error Apr 2, 2026 5:33am

@arkanaai
Copy link
Copy Markdown

arkanaai bot commented Apr 1, 2026

🔍 Arkana Review — arkade-os/packages#2

New @arkade-os/wallet-providers package — browser wallet Identity implementations

Clean, well-structured package. The BrowserWalletIdentity base class is a solid abstraction. Key observations:

Architecture ✅

  • Good separation: base class handles pubkey normalization/PSBT merge, each provider only implements its signing API
  • BatchSignableIdentity correctly applied only to UniSat + OKX (wallets that support signPsbts)
  • Subpath exports (/unisat, /okx, etc.) — nice for tree-shaking, consumers only import what they need
  • Dual ESM+CJS output via tsup is correct for a library package

Security

  • PSBT tampering check in mergeSignedPsbt() — comparing unsignedTx before/after merge is the right approach. This prevents a malicious wallet extension from silently altering outputs.
  • Pubkey validation in constructor (32 or 33 bytes) is good

Cross-Repo Dependencies

  • Depends on arkade-os/ts-sdk#395 for BatchSignableIdentity and SignRequest types — currently duplicated locally in types.ts with TODO comments. This is fine for now but should be tracked: once ts-sdk 0.4.15+ publishes, remove the local copies
  • @arkade-os/sdk is correctly a peerDependency (not bundled)

Items to Address

  1. compressedPublicKey() throws on 32-byte x-only keys — If a wallet returns an x-only pubkey (Phantom does this via Uint8Array), calling compressedPublicKey() will throw. If the SDK ever calls this internally, Phantom users will hit an error. Consider prefixing with 0x02 as a fallback, or document that Phantom identity cannot be used in contexts requiring compressed keys.

  2. signMessage() throws unconditionally — Every provider stub throws Error. Some wallets (UniSat, OKX) actually support signMessage(). Since you already have the provider references, these could be wired up. Not blocking, but worth a follow-up.

  3. OKX signPsbts options signature — The OKX API passes a single options array where each element corresponds to a PSBT. Verify OKX actually accepts OkxSignOptions[] as the second arg to signPsbts — some versions expect a different shape. The UniSat implementation looks correct.

  4. Test app: empty PSBT magic bytes — The test app sends 70736274ff01000a0000000000000000000000 which is a valid PSBT header but with an empty global map. Some wallets may crash rather than gracefully reject. Consider adding a note in the UI that errors are expected.

  5. No signMultiple error handling for partial failures — If signPsbts returns fewer results than inputs (some wallets do this on partial reject), the map() in signMultiple() will access undefined elements. Consider validating signedHexs.length === requests.length.

Nits

  • psbtToBase64() is defined but never used — keep or remove?
  • The default re-exports at the bottom of each provider file (export { LeatherIdentity as default }) are unusual alongside the named export. Intentional for dynamic import() usage?

Overall: well-designed, ready for manual testing. The batch signing UX improvement (single popup instead of N popups) is a significant win for Arkade web wallet users.

Adds signMultiple() to SatsConnectIdentity using sats-connect's
signMultipleTransactions callback API. This enables batch signing of
all checkpoint + main tx PSBTs in a single Xverse wallet popup.

Also refactors the merge+validate logic into a shared private method
and threads the network config through to the identity constructor.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Apr 2, 2026

Deploying snap with  Cloudflare Pages  Cloudflare Pages

Latest commit: ef7828b
Status: ✅  Deploy successful!
Preview URL: https://10b1ccbc.snap-bu9.pages.dev
Branch Preview URL: https://feat-wallet-providers.snap-bu9.pages.dev

View logs

@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying arkade-xverse with  Cloudflare Pages  Cloudflare Pages

Latest commit: ef7828b
Status: ✅  Deploy successful!
Preview URL: https://520492e6.arkade-xverse.pages.dev
Branch Preview URL: https://feat-wallet-providers.arkade-xverse.pages.dev

View logs

@arkanaai
Copy link
Copy Markdown

arkanaai bot commented Apr 2, 2026

🔍 Review — Second commit: feat(sats-connect): add signMultiple for batch PSBT signing

Reviewing the delta from 68d0399ef7828b.

What changed

  1. SatsConnectIdentity.signMultiple() — new batch signing method using sats-connect's signMultipleTransactions callback API
  2. mergeAndValidate() extracted — the inline PSBT merge+validate logic from sign() is now a private method, reused by both sign() and signMultiple()
  3. Network threading — constructor now accepts optional SatsConnectNetwork, mapped to BitcoinNetworkType enum via toBitcoinNetworkType()
  4. buildInputIndexArray() extracted — small helper for input index resolution
  5. ArkadeWallet — passes this.config.satsConnectNetwork to the identity constructor
  6. SignRequest + re-exportsSignRequest type defined locally and exported from the package index

Security analysis

mergeAndValidate refactor is safe. The extracted method is byte-for-byte identical to the original inline logic. The unsigned-tx tamper check is preserved — this is critical for preventing wallet extensions from silently altering transaction structure.

signMultiple validates each PSBT individually. Each response goes through mergeAndValidate(), so a malicious wallet extension can't tamper with any individual PSBT in the batch without detection.

⚠️ Constructor signature is now (pubkey, address, request, network?). The 4th param is optional so this is backwards-compatible. ArkadeWallet already passes the network. Good.

toBitcoinNetworkType defaults to Mainnet. Correct — safe default for production, and the test app explicitly targets signet.

Observations

  1. SignRequest type duplication — This is the third place SignRequest is defined (also in wallet-providers/types.ts and will be in @arkade-os/sdk per ts-sdk#395). The TODO comment acknowledges this. Not a problem now but track the cleanup.

  2. signMultipleTransactions callback API — The sats-connect API uses onFinish/onCancel callbacks wrapped in a Promise. This is fine but note that if the wallet extension never calls either callback (buggy extension), this Promise will hang indefinitely. Consider adding a timeout in a future iteration for robustness.

  3. SignMultipleTransactionsResponse type — Assumes the response is an array indexed 1:1 with the input psbts array. This matches the sats-connect docs but there's no explicit length check. A defensive if (responses.length !== requests.length) guard would be cheap insurance.

  4. No BatchSignableIdentity interface — Unlike UnisatIdentity and OkxIdentity in the wallet-providers package, SatsConnectIdentity has signMultiple() but doesn't implement a BatchSignableIdentity interface. This is fine if the SDK does duck-typing ("signMultiple" in identity), but worth aligning once ts-sdk#395 lands.

Verdict

Clean refactor. The merge+validate extraction reduces code duplication without changing behavior. The batch signing implementation correctly validates each PSBT individually. The network threading is a good addition for multi-network support.

Ship it once ts-sdk#395 is merged and the SignRequest/BatchSignableIdentity imports are switched from local definitions to SDK imports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant