feat: add BatchSignableIdentity for one-shot batch PSBT signing#395
feat: add BatchSignableIdentity for one-shot batch PSBT signing#395
Conversation
Adds a `BatchSignableIdentity` sub-interface that browser wallet providers can implement to sign all checkpoint + main tx PSBTs in a single wallet popup instead of N+1 individual confirmations. When the identity supports `signMultiple`, `buildAndSubmitOffchainTx` pre-signs everything upfront and merges the stashed user signatures onto the server-signed checkpoints after `submitTx` returns. Identities without batch support fall back to the existing sequential signing path unchanged.
WalkthroughThis PR introduces batch signing capabilities to the identity system by adding types ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Wallet as Wallet
participant Identity as Identity (Batch)
participant Server as Server
rect rgba(100, 200, 100, 0.5)
Note over Client,Server: Batch Signing Flow
Client->>Wallet: buildAndSubmitOffchainTx()
Wallet->>Wallet: Prepare offchainTx.arkTx + checkpoint PSBTs
Wallet->>Identity: signMultiple([arkTx, checkpoint1, ...])
Identity->>Identity: Sign all in batch
Identity-->>Wallet: [signedVirtualTx, checkpoint1_signed, ...]
Wallet->>Server: Submit signed virtual transaction
Wallet->>Server: Fetch server-signed checkpoints
Wallet->>Wallet: Merge user + server sigs via combineTapscriptSigs()
Wallet-->>Client: finalCheckpoints[] (merged signatures)
end
rect rgba(100, 100, 200, 0.5)
Note over Client,Server: Legacy Signing Flow (non-batch)
Client->>Wallet: buildAndSubmitOffchainTx()
Wallet->>Wallet: Prepare offchainTx.arkTx + checkpoint PSBTs
Wallet->>Identity: sign(offchainTx.arkTx)
Identity-->>Wallet: signedVirtualTx
Wallet->>Server: Submit signed virtual transaction
Wallet->>Server: Fetch server-signed checkpoints
loop Per checkpoint
Wallet->>Identity: sign(checkpoint PSBT)
Identity-->>Wallet: signed PSBT
Wallet->>Wallet: Encode signed PSBT
end
Wallet-->>Client: finalCheckpoints[] (individually signed)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/identity/index.ts`:
- Around line 20-32: The BatchSignableIdentity.signMultiple contract is
ambiguous and can lead to misapplied signatures; update the API to return
explicit mapping between requests and results (e.g., change SignRequest to
include a unique id and change signMultiple to return results keyed by that id,
or else document and enforce a strict same-order/same-length guarantee) so
Wallet.buildAndSubmitOffchainTx() can reliably match signatures to requests;
modify the SignRequest type to include an identifier (e.g., requestId) and
change BatchSignableIdentity.signMultiple to return either a Record<requestId,
Transaction> or an array of {requestId, Transaction} and adjust callers to match
by id rather than by array position.
In `@src/wallet/wallet.ts`:
- Around line 2499-2501: Check that identity.signMultiple returned the exact
number of signatures before destructuring: after calling
this.identity.signMultiple(requests) verify signed.length === requests.length
and throw or return an explicit error if it does not, so signedVirtualTx =
signed[0] and userSignedCheckpoints = signed.slice(1) are only executed when
counts match; update the code around the call to identity.signMultiple, using
the variables signed, requests, signedVirtualTx, and userSignedCheckpoints to
perform the validation and fail fast with a clear error message when the lengths
differ.
- Around line 2495-2498: The requests array currently passes references to
offchainTx.checkpoints into signMultiple(), allowing providers to mutate the
original checkpoint objects; change the code that builds requests (the const
requests = [...] construction) to deep-clone each checkpoint (e.g., shallow
clone objects and nested signature arrays) before including them so
signMultiple() receives clones, leaving offchainTx.checkpoints unchanged for
submitTx(); ensure combineTapscriptSigs() later merges signatures from the
signed clones back into the original checkpoint objects (or deduplicates) to
avoid duplicating user signatures when combining tapscript signatures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15354baf-c73f-427b-93ad-7ab19a7ba2d4
📒 Files selected for processing (3)
src/identity/index.tssrc/index.tssrc/wallet/wallet.ts
| export interface SignRequest { | ||
| tx: Transaction; | ||
| inputIndexes?: number[]; | ||
| } | ||
|
|
||
| /** | ||
| * Identity that supports signing multiple PSBTs in a single wallet interaction. | ||
| * Browser wallet providers that support batch signing (e.g. Xverse, UniSat, OKX) | ||
| * should implement this interface to reduce the number of confirmation popups | ||
| * from N+1 to 1 during Arkade send transactions. | ||
| */ | ||
| export interface BatchSignableIdentity extends Identity { | ||
| signMultiple(requests: SignRequest[]): Promise<Transaction[]>; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Encode the batch request/result mapping explicitly.
Wallet.buildAndSubmitOffchainTx() consumes signMultiple() positionally, but this API only promises a bare Transaction[]. A provider can legally reorder or drop results and still satisfy the type, which would misapply signatures during checkpoint finalization. Either make the response keyed/id-based, or at least document a strict same-order/same-length contract here so implementers don’t guess.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/identity/index.ts` around lines 20 - 32, The
BatchSignableIdentity.signMultiple contract is ambiguous and can lead to
misapplied signatures; update the API to return explicit mapping between
requests and results (e.g., change SignRequest to include a unique id and change
signMultiple to return results keyed by that id, or else document and enforce a
strict same-order/same-length guarantee) so Wallet.buildAndSubmitOffchainTx()
can reliably match signatures to requests; modify the SignRequest type to
include an identifier (e.g., requestId) and change
BatchSignableIdentity.signMultiple to return either a Record<requestId,
Transaction> or an array of {requestId, Transaction} and adjust callers to match
by id rather than by array position.
| const requests = [ | ||
| { tx: offchainTx.arkTx }, | ||
| ...offchainTx.checkpoints.map((c) => ({ tx: c })), | ||
| ]; |
There was a problem hiding this comment.
Send clones to signMultiple(), not the original checkpoints.
submitTx() still sends offchainTx.checkpoints as the unsigned checkpoint set. If a provider signs in place, those originals are mutated before submission, and the later combineTapscriptSigs() pass can duplicate user signatures. Batch the clones here instead of the shared offchainTx objects.
Possible fix
const requests = [
- { tx: offchainTx.arkTx },
- ...offchainTx.checkpoints.map((c) => ({ tx: c })),
+ { tx: offchainTx.arkTx.clone() },
+ ...offchainTx.checkpoints.map((c) => ({ tx: c.clone() })),
];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wallet/wallet.ts` around lines 2495 - 2498, The requests array currently
passes references to offchainTx.checkpoints into signMultiple(), allowing
providers to mutate the original checkpoint objects; change the code that builds
requests (the const requests = [...] construction) to deep-clone each checkpoint
(e.g., shallow clone objects and nested signature arrays) before including them
so signMultiple() receives clones, leaving offchainTx.checkpoints unchanged for
submitTx(); ensure combineTapscriptSigs() later merges signatures from the
signed clones back into the original checkpoint objects (or deduplicates) to
avoid duplicating user signatures when combining tapscript signatures.
| const signed = await this.identity.signMultiple(requests); | ||
| signedVirtualTx = signed[0]; | ||
| userSignedCheckpoints = signed.slice(1); |
There was a problem hiding this comment.
Validate the batch signer returned one result per request.
A short or extra response array will only fail later when signedVirtualTx or userSignedCheckpoints[i] is used. Check signed.length === requests.length immediately and fail fast before destructuring.
Possible fix
const signed = await this.identity.signMultiple(requests);
- signedVirtualTx = signed[0];
- userSignedCheckpoints = signed.slice(1);
+ if (signed.length !== requests.length) {
+ throw new Error(
+ `Batch signer returned ${signed.length} result(s) for ${requests.length} request(s)`
+ );
+ }
+ const [firstSignedTx, ...signedCheckpoints] = signed;
+ signedVirtualTx = firstSignedTx;
+ userSignedCheckpoints = signedCheckpoints;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/wallet/wallet.ts` around lines 2499 - 2501, Check that
identity.signMultiple returned the exact number of signatures before
destructuring: after calling this.identity.signMultiple(requests) verify
signed.length === requests.length and throw or return an explicit error if it
does not, so signedVirtualTx = signed[0] and userSignedCheckpoints =
signed.slice(1) are only executed when counts match; update the code around the
call to identity.signMultiple, using the variables signed, requests,
signedVirtualTx, and userSignedCheckpoints to perform the validation and fail
fast with a clear error message when the lengths differ.
🔍 Review —
|
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.
1 participant
Summary
BatchSignableIdentitysub-interface withsignMultiple()method that browser wallet providers (Xverse, UniSat, OKX) can implement to batch-sign all checkpoint + main tx PSBTs in a single wallet popupbuildAndSubmitOffchainTx()to detect batch-capable identities and pre-sign all PSBTs upfront, then merge stashed user signatures onto server-signed checkpoints using existingcombineTapscriptSigs()BatchSignableIdentity,SignRequest, andisBatchSignabletype guard from the SDKMotivation
Arkade send transactions require N+1 PSBT signatures (N checkpoints + 1 main tx). With
SingleKeythis is invisible (local signing), but browser wallet extensions show a confirmation popup per signature. This change enables wallets with batch signing APIs to reduce N+1 popups to 1.How it works
Test plan
tsc --noEmit)nigirilocal infra, pre-existing failures)Identityimplementations (SingleKey,SeedIdentity,MnemonicIdentity) are unchanged and take the legacy code pathBatchSignableIdentityprovider (to be done inarkade-os/packagesrepo)Summary by CodeRabbit