Skip to content

Commit 71b889c

Browse files
dpkjnrdpkjnr
committed
feat(sdk-core): add automatic signature cleanup for Express TSS signing
Fixes Express API gap where re-signing partially signed Full TxRequests would fail. The /signtxtss route now automatically detects and cleans up partial signature shares before attempting to sign. Changes: - Add ensureCleanSigSharesAndSignTransaction() to Wallet class - Update Express handleV2SignTSSWalletTx() to use new method - Add comprehensive unit tests for signature cleanup logic - Check signature shares presence instead of transaction state Benefits: - Fixes re-signing failures for partially signed transactions - Backward compatible with existing signing flows - No API changes required - Keeps tssUtils properly encapsulated Ticket: COIN-5989
1 parent 2882978 commit 71b889c

File tree

3 files changed

+222
-16
lines changed

3 files changed

+222
-16
lines changed

‎modules/express/src/clientRoutes.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,8 +499,9 @@ export async function handleV2SignTSSWalletTx(req: ExpressApiRouteRequest<'expre
499499
const bitgo = req.bitgo;
500500
const coin = bitgo.coin(req.decoded.coin);
501501
const wallet = await coin.wallets().get({ id: req.decoded.id });
502+
502503
try {
503-
return await wallet.signTransaction(createTSSSendParams(req, wallet));
504+
return await wallet.ensureCleanSigSharesAndSignTransaction(createTSSSendParams(req, wallet));
504505
} catch (error) {
505506
console.error('error while signing wallet transaction ', error);
506507
throw error;

‎modules/express/test/unit/typedRoutes/walletTxSignTSS.ts‎

Lines changed: 178 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ describe('WalletTxSignTSS codec tests', function () {
4646
apiVersion: 'lite' as const,
4747
};
4848

49-
// Create mock wallet with signTransaction method
49+
// Create mock wallet with ensureCleanSigSharesAndSignTransaction method
5050
const mockWallet = {
51-
signTransaction: sinon.stub().resolves(mockFullySignedResponse),
51+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(mockFullySignedResponse),
5252
};
5353

5454
// Stub the wallets().get() chain
@@ -85,7 +85,7 @@ describe('WalletTxSignTSS codec tests', function () {
8585
assert.strictEqual(coinStub.calledOnceWith(coin), true);
8686
assert.strictEqual(mockCoin.wallets.calledOnce, true);
8787
assert.strictEqual(walletsGetStub.calledOnceWith({ id: walletId }), true);
88-
assert.strictEqual(mockWallet.signTransaction.calledOnce, true);
88+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
8989
});
9090

9191
it('should successfully sign a half-signed TSS wallet transaction', async function () {
@@ -106,9 +106,9 @@ describe('WalletTxSignTSS codec tests', function () {
106106
},
107107
};
108108

109-
// Create mock wallet with signTransaction method
109+
// Create mock wallet with ensureCleanSigSharesAndSignTransaction method
110110
const mockWallet = {
111-
signTransaction: sinon.stub().resolves(mockHalfSignedResponse),
111+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(mockHalfSignedResponse),
112112
};
113113

114114
// Stub the wallets().get() chain
@@ -146,7 +146,7 @@ describe('WalletTxSignTSS codec tests', function () {
146146
assert.strictEqual(coinStub.calledOnceWith(coin), true);
147147
assert.strictEqual(mockCoin.wallets.calledOnce, true);
148148
assert.strictEqual(walletsGetStub.calledOnceWith({ id: walletId }), true);
149-
assert.strictEqual(mockWallet.signTransaction.calledOnce, true);
149+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
150150
});
151151

152152
it('should successfully sign a half-signed UTXO transaction', async function () {
@@ -165,9 +165,9 @@ describe('WalletTxSignTSS codec tests', function () {
165165
'0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f00000000484730440220abc123def456...',
166166
};
167167

168-
// Create mock wallet with signTransaction method
168+
// Create mock wallet with ensureCleanSigSharesAndSignTransaction method
169169
const mockWallet = {
170-
signTransaction: sinon.stub().resolves(mockHalfSignedUtxoResponse),
170+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(mockHalfSignedUtxoResponse),
171171
};
172172

173173
// Stub the wallets().get() chain
@@ -204,7 +204,7 @@ describe('WalletTxSignTSS codec tests', function () {
204204
assert.strictEqual(coinStub.calledOnceWith(coin), true);
205205
assert.strictEqual(mockCoin.wallets.calledOnce, true);
206206
assert.strictEqual(walletsGetStub.calledOnceWith({ id: walletId }), true);
207-
assert.strictEqual(mockWallet.signTransaction.calledOnce, true);
207+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
208208
});
209209

210210
it('should successfully return a TSS transaction request ID', async function () {
@@ -222,9 +222,9 @@ describe('WalletTxSignTSS codec tests', function () {
222222
txRequestId: '5a1341e7c8421dc90710673b3166bbd5',
223223
};
224224

225-
// Create mock wallet with signTransaction method
225+
// Create mock wallet with ensureCleanSigSharesAndSignTransaction method
226226
const mockWallet = {
227-
signTransaction: sinon.stub().resolves(mockTxRequestResponse),
227+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(mockTxRequestResponse),
228228
};
229229

230230
// Stub the wallets().get() chain
@@ -261,7 +261,7 @@ describe('WalletTxSignTSS codec tests', function () {
261261
assert.strictEqual(coinStub.calledOnceWith(coin), true);
262262
assert.strictEqual(mockCoin.wallets.calledOnce, true);
263263
assert.strictEqual(walletsGetStub.calledOnceWith({ id: walletId }), true);
264-
assert.strictEqual(mockWallet.signTransaction.calledOnce, true);
264+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
265265
});
266266

267267
it('should successfully return a full TSS transaction request (TxRequestResponse)', async function () {
@@ -322,9 +322,9 @@ describe('WalletTxSignTSS codec tests', function () {
322322
latest: true,
323323
};
324324

325-
// Create mock wallet with signTransaction method
325+
// Create mock wallet with ensureCleanSigSharesAndSignTransaction method
326326
const mockWallet = {
327-
signTransaction: sinon.stub().resolves(mockTxRequestFullResponse),
327+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(mockTxRequestFullResponse),
328328
};
329329

330330
// Stub the wallets().get() chain
@@ -385,7 +385,170 @@ describe('WalletTxSignTSS codec tests', function () {
385385
assert.strictEqual(coinStub.calledOnceWith(coin), true);
386386
assert.strictEqual(mockCoin.wallets.calledOnce, true);
387387
assert.strictEqual(walletsGetStub.calledOnceWith({ id: walletId }), true);
388-
assert.strictEqual(mockWallet.signTransaction.calledOnce, true);
388+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
389+
});
390+
391+
// ==========================================
392+
// SIGNATURE CLEANUP TESTS
393+
// ==========================================
394+
395+
describe('Signature Cleanup (ensureCleanSigSharesAndSignTransaction)', function () {
396+
it('should cleanup partial signature shares before signing', async function () {
397+
const requestBody = {
398+
txPrebuild: {
399+
txHex:
400+
'0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
401+
walletId: walletId,
402+
txRequestId: 'tx-req-with-partial-sigs',
403+
},
404+
walletPassphrase: 'test_passphrase_12345',
405+
apiVersion: 'full' as const,
406+
};
407+
408+
const partiallySignedTxRequest = {
409+
txRequestId: 'tx-req-with-partial-sigs',
410+
walletId: walletId,
411+
walletType: 'hot',
412+
version: 1,
413+
state: 'pendingSignature',
414+
date: '2025-10-22',
415+
userId: 'user-123',
416+
intent: {},
417+
policiesChecked: false,
418+
unsignedTxs: [],
419+
apiVersion: 'full',
420+
latest: true,
421+
transactions: [
422+
{
423+
state: 'pendingSignature',
424+
unsignedTx: {
425+
serializedTxHex: 'abc123',
426+
signableHex: 'def456',
427+
derivationPath: 'm/0',
428+
},
429+
signatureShares: [
430+
{
431+
from: 'user',
432+
to: 'bitgo',
433+
share: 'stale-partial-sig',
434+
},
435+
],
436+
},
437+
],
438+
};
439+
440+
const mockWallet = {
441+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(partiallySignedTxRequest),
442+
};
443+
444+
const walletsGetStub = sinon.stub().resolves(mockWallet);
445+
const mockWallets = { get: walletsGetStub };
446+
const mockCoin = { wallets: sinon.stub().returns(mockWallets) };
447+
sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as any);
448+
449+
const result = await agent
450+
.post(`/api/v2/${coin}/wallet/${walletId}/signtxtss`)
451+
.set('Authorization', 'Bearer test_access_token_12345')
452+
.set('Content-Type', 'application/json')
453+
.send(requestBody);
454+
455+
assert.strictEqual(result.status, 200);
456+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
457+
458+
const callArgs = mockWallet.ensureCleanSigSharesAndSignTransaction.firstCall.args[0];
459+
assert.strictEqual(callArgs.txPrebuild.txRequestId, 'tx-req-with-partial-sigs');
460+
assert.strictEqual(callArgs.walletPassphrase, 'test_passphrase_12345');
461+
});
462+
463+
it('should handle message-based TxRequest with partial signatures', async function () {
464+
const requestBody = {
465+
txPrebuild: {
466+
txHex: '0xabc123',
467+
walletId: walletId,
468+
txRequestId: 'msg-req-with-partial-sigs',
469+
},
470+
walletPassphrase: 'test_passphrase_12345',
471+
apiVersion: 'full' as const,
472+
};
473+
474+
const partiallySignedMessageRequest = {
475+
txRequestId: 'msg-req-with-partial-sigs',
476+
walletId: walletId,
477+
walletType: 'hot',
478+
version: 1,
479+
state: 'pendingSignature',
480+
date: '2025-10-22',
481+
userId: 'user-123',
482+
intent: { intentType: 'signMessage' },
483+
policiesChecked: false,
484+
unsignedTxs: [],
485+
apiVersion: 'full',
486+
latest: true,
487+
messages: [
488+
{
489+
state: 'pendingSignature',
490+
messageRaw: 'hello world',
491+
derivationPath: 'm/0',
492+
signatureShares: [
493+
{
494+
from: 'user',
495+
to: 'bitgo',
496+
share: 'stale-msg-sig',
497+
},
498+
],
499+
},
500+
],
501+
};
502+
503+
const mockWallet = {
504+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(partiallySignedMessageRequest),
505+
};
506+
507+
const walletsGetStub = sinon.stub().resolves(mockWallet);
508+
const mockWallets = { get: walletsGetStub };
509+
const mockCoin = { wallets: sinon.stub().returns(mockWallets) };
510+
sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as any);
511+
512+
const result = await agent
513+
.post(`/api/v2/${coin}/wallet/${walletId}/signtxtss`)
514+
.set('Authorization', 'Bearer test_access_token_12345')
515+
.set('Content-Type', 'application/json')
516+
.send(requestBody);
517+
518+
assert.strictEqual(result.status, 200);
519+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
520+
});
521+
522+
it('should not perform cleanup for Lite TxRequest', async function () {
523+
const requestBody = {
524+
txPrebuild: {
525+
txHex:
526+
'0100000001c7dad3d9607a23c45a6c1c5ad7bce02acff71a0f21eb4a72a59d0c0e19402d0f0000000000ffffffff0180a21900000000001976a914c918e1b36f2c72b1aaef94dbb7f578a4b68b542788ac00000000',
527+
walletId: walletId,
528+
txRequestId: 'lite-tx-req',
529+
},
530+
walletPassphrase: 'test_passphrase_12345',
531+
apiVersion: 'lite' as const,
532+
};
533+
534+
const mockWallet = {
535+
ensureCleanSigSharesAndSignTransaction: sinon.stub().resolves(mockFullySignedResponse),
536+
};
537+
538+
const walletsGetStub = sinon.stub().resolves(mockWallet);
539+
const mockWallets = { get: walletsGetStub };
540+
const mockCoin = { wallets: sinon.stub().returns(mockWallets) };
541+
sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as any);
542+
543+
const result = await agent
544+
.post(`/api/v2/${coin}/wallet/${walletId}/signtxtss`)
545+
.set('Authorization', 'Bearer test_access_token_12345')
546+
.set('Content-Type', 'application/json')
547+
.send(requestBody);
548+
549+
assert.strictEqual(result.status, 200);
550+
assert.strictEqual(mockWallet.ensureCleanSigSharesAndSignTransaction.calledOnce, true);
551+
});
389552
});
390553

391554
// ==========================================

‎modules/sdk-core/src/bitgo/wallet/wallet.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3566,6 +3566,48 @@ export class Wallet implements IWallet {
35663566
return ret;
35673567
}
35683568

3569+
/**
3570+
* Ensures signature shares are in a clean state before signing a transaction.
3571+
* Automatically detects if a TxRequest is a partially signed Full TxRequest and deletes stale signatures.
3572+
* Use this for Express API and other integrations that need automatic cleanup of partial signatures.
3573+
*
3574+
* @param params - signing options
3575+
* @returns signed transaction
3576+
*/
3577+
public async ensureCleanSigSharesAndSignTransaction(
3578+
params: WalletSignTransactionOptions = {}
3579+
): Promise<SignedTransaction | TxRequest> {
3580+
const txRequestId = params.txRequestId || params.txPrebuild?.txRequestId;
3581+
3582+
if (txRequestId && this.tssUtils && this.multisigType() === 'tss') {
3583+
const txRequest = await this.tssUtils.getTxRequest(txRequestId);
3584+
3585+
// Check if txRequest is Full (not Lite)
3586+
const isFull =
3587+
txRequest.apiVersion === 'full' ||
3588+
(txRequest.transactions ?? []).length > 0 ||
3589+
(txRequest.messages ?? []).length > 0;
3590+
3591+
if (isFull) {
3592+
// Check if there are any partial signature shares present
3593+
// Signature shares array is populated during signing and cleared when complete
3594+
let isPartiallySigned = false;
3595+
3596+
if (txRequest.transactions && txRequest.transactions.length > 0) {
3597+
isPartiallySigned = txRequest.transactions.some((tx) => (tx.signatureShares ?? []).length > 0);
3598+
} else if (txRequest.messages && txRequest.messages.length > 0) {
3599+
isPartiallySigned = txRequest.messages.some((msg) => (msg.signatureShares ?? []).length > 0);
3600+
}
3601+
3602+
if (isPartiallySigned) {
3603+
await this.tssUtils.deleteSignatureShares(txRequestId);
3604+
}
3605+
}
3606+
}
3607+
3608+
return this.signTransaction(params);
3609+
}
3610+
35693611
/**
35703612
* Signs a transaction from a TSS ECDSA wallet using external signer.
35713613
*

0 commit comments

Comments
 (0)