Skip to content

Commit f8fcd9e

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 f8fcd9e

File tree

4 files changed

+176
-16
lines changed

4 files changed

+176
-16
lines changed

‎modules/bitgo/test/v2/unit/wallet.ts‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3765,6 +3765,123 @@ describe('V2 Wallet:', function () {
37653765
});
37663766
});
37673767

3768+
describe('ensureCleanSigSharesAndSignTransaction', function () {
3769+
const exampleSignedTx = {
3770+
txHex: '0x123',
3771+
txid: '0x456',
3772+
status: 'signed',
3773+
};
3774+
3775+
const partiallySignedTxRequest: TxRequest = {
3776+
txRequestId: 'test-tx-id',
3777+
walletId: 'test-wallet-id',
3778+
walletType: 'hot',
3779+
version: 1,
3780+
state: 'initialized',
3781+
date: '2025-10-15',
3782+
userId: 'user-123',
3783+
intent: {},
3784+
policiesChecked: false,
3785+
unsignedTxs: [],
3786+
apiVersion: 'full',
3787+
latest: true,
3788+
transactions: [
3789+
{
3790+
state: 'pendingSignature',
3791+
unsignedTx: {
3792+
serializedTxHex: 'abc123',
3793+
signableHex: 'def456',
3794+
derivationPath: 'm/0',
3795+
},
3796+
signatureShares: [
3797+
{
3798+
from: 'user',
3799+
to: 'bitgo',
3800+
share: 'partial-share-data',
3801+
},
3802+
],
3803+
},
3804+
],
3805+
};
3806+
3807+
const cleanTxRequest: TxRequest = {
3808+
...partiallySignedTxRequest,
3809+
transactions: [
3810+
{
3811+
state: 'initialized',
3812+
unsignedTx: {
3813+
serializedTxHex: 'abc123',
3814+
signableHex: 'def456',
3815+
derivationPath: 'm/0',
3816+
},
3817+
signatureShares: [],
3818+
},
3819+
],
3820+
};
3821+
3822+
afterEach(async function () {
3823+
sandbox.restore();
3824+
});
3825+
3826+
it('should delete signature shares for partially signed Full TxRequest before signing', async function () {
3827+
const getTxRequestSpy = sandbox.stub(TssUtils.prototype, 'getTxRequest');
3828+
getTxRequestSpy.resolves(partiallySignedTxRequest);
3829+
3830+
const deleteSignatureSharesSpy = sandbox.stub(TssUtils.prototype, 'deleteSignatureShares');
3831+
deleteSignatureSharesSpy.resolves([]);
3832+
3833+
const signTransactionSpy = sandbox.stub(tssSolWallet, 'signTransaction');
3834+
signTransactionSpy.resolves(exampleSignedTx);
3835+
3836+
const signedTx = await tssSolWallet.ensureCleanSigSharesAndSignTransaction({
3837+
txRequestId: 'test-tx-id',
3838+
walletPassphrase: 'passphrase',
3839+
});
3840+
3841+
sandbox.assert.calledOnce(getTxRequestSpy);
3842+
sandbox.assert.calledOnce(deleteSignatureSharesSpy);
3843+
sandbox.assert.calledOnce(signTransactionSpy);
3844+
signedTx.should.deepEqual(exampleSignedTx);
3845+
});
3846+
3847+
it('should not delete signature shares for clean Full TxRequest', async function () {
3848+
const getTxRequestSpy = sandbox.stub(TssUtils.prototype, 'getTxRequest');
3849+
getTxRequestSpy.resolves(cleanTxRequest);
3850+
3851+
const deleteSignatureSharesSpy = sandbox.stub(TssUtils.prototype, 'deleteSignatureShares');
3852+
3853+
const signTransactionSpy = sandbox.stub(tssSolWallet, 'signTransaction');
3854+
signTransactionSpy.resolves(exampleSignedTx);
3855+
3856+
const signedTx = await tssSolWallet.ensureCleanSigSharesAndSignTransaction({
3857+
txRequestId: 'test-tx-id',
3858+
walletPassphrase: 'passphrase',
3859+
});
3860+
3861+
sandbox.assert.calledOnce(getTxRequestSpy);
3862+
sandbox.assert.notCalled(deleteSignatureSharesSpy);
3863+
sandbox.assert.calledOnce(signTransactionSpy);
3864+
signedTx.should.deepEqual(exampleSignedTx);
3865+
});
3866+
3867+
it('should sign transaction directly when no txRequestId provided', async function () {
3868+
const getTxRequestSpy = sandbox.stub(TssUtils.prototype, 'getTxRequest');
3869+
const deleteSignatureSharesSpy = sandbox.stub(TssUtils.prototype, 'deleteSignatureShares');
3870+
3871+
const signTransactionSpy = sandbox.stub(tssSolWallet, 'signTransaction');
3872+
signTransactionSpy.resolves(exampleSignedTx);
3873+
3874+
const signedTx = await tssSolWallet.ensureCleanSigSharesAndSignTransaction({
3875+
walletPassphrase: 'passphrase',
3876+
});
3877+
3878+
sandbox.assert.notCalled(getTxRequestSpy);
3879+
sandbox.assert.notCalled(deleteSignatureSharesSpy);
3880+
sandbox.assert.calledOnce(signTransactionSpy);
3881+
signedTx.should.deepEqual(exampleSignedTx);
3882+
});
3883+
});
3884+
37683885
describe('Message Signing', function () {
37693886
const txHash = '0xrrrsss1b';
37703887
const messageRaw = 'hello world';

‎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: 15 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,7 @@ 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);
389389
});
390390

391391
// ==========================================

‎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)