Skip to content

feat: add BatchSignableIdentity for one-shot batch PSBT signing #395

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Kukks wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: add BatchSignableIdentity for one-shot batch PSBT signing #395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/identity/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,31 @@ export interface ReadonlyIdentity {
compressedPublicKey(): Promise<Uint8Array>;
}

/** A single PSBT signing request within a batch. */
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[]>;
Comment on lines +20 to +32
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

}

/** Type guard for identities that support batch signing. */
export function isBatchSignable(
identity: Identity
): identity is BatchSignableIdentity {
return (
"signMultiple" in identity &&
typeof (identity as BatchSignableIdentity).signMultiple === "function"
);
}

export * from "./singleKey";
export * from "./seedIdentity";
11 changes: 10 additions & 1 deletion src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ import type {
NetworkOptions,
DescriptorOptions,
} from "./identity/seedIdentity";
import { Identity, ReadonlyIdentity } from "./identity";
import {
Identity,
ReadonlyIdentity,
BatchSignableIdentity,
SignRequest,
isBatchSignable,
} from "./identity";
import { ArkAddress } from "./script/address";
import { VHTLC } from "./script/vhtlc";
import { DefaultVtxo } from "./script/default";
Expand Down Expand Up @@ -263,6 +269,7 @@ export {
SeedIdentity,
MnemonicIdentity,
ReadonlyDescriptorIdentity,
isBatchSignable,
OnchainWallet,
Ramps,
VtxoManager,
Expand Down Expand Up @@ -400,6 +407,8 @@ export type {
// Types and Interfaces
Identity,
ReadonlyIdentity,
BatchSignableIdentity,
SignRequest,
IWallet,
IReadonlyWallet,
BaseWalletConfig,
Expand Down
48 changes: 39 additions & 9 deletions src/wallet/wallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
validateVtxoTxGraph,
} from "../tree/validation";
import { validateBatchRecipients } from "./validation";
import { Identity, ReadonlyIdentity } from "../identity";
import { Identity, ReadonlyIdentity, isBatchSignable } from "../identity";
import {
ArkTransaction,
Asset,
Expand Down Expand Up @@ -62,6 +62,7 @@ import { getSequence, VtxoScript } from "../script/base";
import { CSVMultisigTapscript, RelativeTimelock } from "../script/tapscript";
import {
buildOffchainTx,
combineTapscriptSigs,
hasBoardingTxExpired,
isValidArkAddress,
} from "../utils/arkTransaction";
Expand Down Expand Up @@ -2485,7 +2486,22 @@ export class Wallet extends ReadonlyWallet implements IWallet {
outputs,
this.serverUnrollScript
);
const signedVirtualTx = await this.identity.sign(offchainTx.arkTx);

let signedVirtualTx: Transaction;
let userSignedCheckpoints: Transaction[] | undefined;

if (isBatchSignable(this.identity)) {
// Batch-sign arkTx + all checkpoints in one wallet popup
const requests = [
{ tx: offchainTx.arkTx },
...offchainTx.checkpoints.map((c) => ({ tx: c })),
];
Comment on lines +2495 to +2498
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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);
Comment on lines +2499 to +2501
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

} else {
signedVirtualTx = await this.identity.sign(offchainTx.arkTx);
}

// Mark pending before submitting — if we crash between submit and
// finalize, the next init will recover via finalizePendingTxs.
Expand All @@ -2496,13 +2512,27 @@ export class Wallet extends ReadonlyWallet implements IWallet {
base64.encode(signedVirtualTx.toPSBT()),
offchainTx.checkpoints.map((c) => base64.encode(c.toPSBT()))
);
const finalCheckpoints = await Promise.all(
signedCheckpointTxs.map(async (c) => {
const tx = Transaction.fromPSBT(base64.decode(c));
const signedCheckpoint = await this.identity.sign(tx);
return base64.encode(signedCheckpoint.toPSBT());
})
);

let finalCheckpoints: string[];

if (userSignedCheckpoints) {
// Merge pre-signed user signatures onto server-signed checkpoints
finalCheckpoints = signedCheckpointTxs.map((c, i) => {
const serverSigned = Transaction.fromPSBT(base64.decode(c));
combineTapscriptSigs(userSignedCheckpoints![i], serverSigned);
return base64.encode(serverSigned.toPSBT());
});
} else {
// Legacy: sign each checkpoint individually (N popups)
finalCheckpoints = await Promise.all(
signedCheckpointTxs.map(async (c) => {
const tx = Transaction.fromPSBT(base64.decode(c));
const signedCheckpoint = await this.identity.sign(tx);
return base64.encode(signedCheckpoint.toPSBT());
})
);
}

await this.arkProvider.finalizeTx(arkTxid, finalCheckpoints);

try {
Expand Down
Loading