From 2d0867726787b5a35697902da19895651afff5f4 Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Tue, 8 Jul 2025 13:02:34 -0400 Subject: [PATCH 01/10] feat(mbe): consolidation eddsa --- masterBitgoExpress.json | 120 ++++++++++++++++++- src/api/master/handlerUtils.ts | 10 +- src/api/master/handlers/eddsa.ts | 70 +++++++++++ src/api/master/handlers/handleConsolidate.ts | 62 ++++++++-- src/api/master/routers/masterApiSpec.ts | 3 +- 5 files changed, 248 insertions(+), 17 deletions(-) diff --git a/masterBitgoExpress.json b/masterBitgoExpress.json index 7cb4a8e2..ff66e051 100644 --- a/masterBitgoExpress.json +++ b/masterBitgoExpress.json @@ -6,6 +6,120 @@ "description": "BitGo Enclaved Express - Secure enclave for BitGo signing operations with mTLS" }, "paths": { + "/api/{coin}/wallet/{walletId}/accelerate": { + "post": { + "parameters": [ + { + "name": "walletId", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "coin", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "pubkey": { + "type": "string" + }, + "source": { + "type": "string", + "enum": [ + "user", + "backup" + ] + }, + "cpfpTxIds": { + "type": "array", + "items": { + "type": "string" + } + }, + "cpfpFeeRate": { + "type": "number" + }, + "maxFee": { + "type": "number" + }, + "rbfTxIds": { + "type": "array", + "items": { + "type": "string" + } + }, + "feeMultiplier": { + "type": "number" + } + }, + "required": [ + "pubkey", + "source" + ] + } + } + } + }, + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "txid": { + "type": "string" + }, + "tx": { + "type": "string" + } + }, + "required": [ + "txid", + "tx" + ] + } + } + } + }, + "500": { + "description": "Internal Server Error", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "error": { + "type": "string" + }, + "details": { + "type": "string" + } + }, + "required": [ + "error", + "details" + ] + } + } + } + } + } + } + }, "/api/{coin}/wallet/{walletId}/consolidate": { "post": { "parameters": [ @@ -54,6 +168,9 @@ "full", "lite" ] + }, + "commonKeychain": { + "type": "string" } }, "required": [ @@ -142,9 +259,6 @@ "backup" ] }, - "walletPassphrase": { - "type": "string" - }, "feeRate": { "type": "number" }, diff --git a/src/api/master/handlerUtils.ts b/src/api/master/handlerUtils.ts index 9c297f57..ed480dd5 100644 --- a/src/api/master/handlerUtils.ts +++ b/src/api/master/handlerUtils.ts @@ -17,7 +17,7 @@ export async function getWalletAndSigningKeychain({ bitgo: BitGo; coin: string; walletId: string; - params: { source: 'user' | 'backup'; pubkey?: string }; + params: { source: 'user' | 'backup'; pubkey?: string; commonKeychain?: string }; reqId: RequestTracer; KeyIndices: { USER: number; BACKUP: number; BITGO: number }; }) { @@ -34,7 +34,7 @@ export async function getWalletAndSigningKeychain({ id: wallet.keyIds()[keyIdIndex], }); - if (!signingKeychain || !signingKeychain.pub) { + if (!signingKeychain) { throw new Error(`Signing keychain for ${params.source} not found`); } @@ -42,6 +42,12 @@ export async function getWalletAndSigningKeychain({ throw new Error(`Pub provided does not match the keychain on wallet for ${params.source}`); } + if (params.commonKeychain && signingKeychain.commonKeychain !== params.commonKeychain) { + throw new Error( + `Common keychain provided does not match the keychain on wallet for ${params.source}`, + ); + } + return { baseCoin, wallet, signingKeychain }; } /** diff --git a/src/api/master/handlers/eddsa.ts b/src/api/master/handlers/eddsa.ts index 6a1d3e99..88fe7274 100644 --- a/src/api/master/handlers/eddsa.ts +++ b/src/api/master/handlers/eddsa.ts @@ -10,6 +10,9 @@ import { BaseCoin, ApiKeyShare, TxRequest, + CustomRShareGeneratingFunction, + CustomGShareGeneratingFunction, + CustomCommitmentGeneratingFunction, } from '@bitgo/sdk-core'; import { EnclavedExpressClient } from '../clients/enclavedExpressClient'; import { exchangeEddsaCommitments } from '@bitgo/sdk-core/dist/src/bitgo/tss/common'; @@ -203,3 +206,70 @@ export async function orchestrateEddsaKeyGen({ walletParams.keys = [userMpcKey.id, backupMpcKey.id, bitgoKeychain.id]; return { walletParams, keychains }; } + +// Commitment +export function createCustomCommitmentGenerator( + bitgo: BitGoBase, + wallet: Wallet, + enclavedExpressClient: EnclavedExpressClient, + source: 'user' | 'backup', + pub: string, +): CustomCommitmentGeneratingFunction { + return async function customCommitmentGeneratingFunction(params) { + const eddsaUtils = new EddsaUtils(bitgo, wallet.baseCoin); + const bitgoGpgKey = await eddsaUtils.getBitgoPublicGpgKey(); + const { txRequest } = params; + const response = await enclavedExpressClient.signMpcCommitment({ + txRequest, + bitgoGpgPubKey: bitgoGpgKey.armor(), + source, + pub, + }); + return { + ...response, + encryptedUserToBitgoRShare: { + ...response.encryptedUserToBitgoRShare, + encryptedDataKey: response.encryptedDataKey, + }, + }; + }; +} + +// RShare +export function createCustomRShareGenerator( + enclavedExpressClient: EnclavedExpressClient, + source: 'user' | 'backup', + pub: string, +): CustomRShareGeneratingFunction { + return async function customRShareGeneratingFunction(params) { + const { txRequest, encryptedUserToBitgoRShare } = params; + const encryptedDataKey = (encryptedUserToBitgoRShare as any).encryptedDataKey; + return await enclavedExpressClient.signMpcRShare({ + txRequest, + encryptedUserToBitgoRShare, + encryptedDataKey, + source, + pub, + }); + }; +} + +// GShare +export function createCustomGShareGenerator( + enclavedExpressClient: EnclavedExpressClient, + source: 'user' | 'backup', + pub: string, +): CustomGShareGeneratingFunction { + return async function customGShareGeneratingFunction(params) { + const { txRequest, bitgoToUserRShare, userToBitgoRShare, bitgoToUserCommitment } = params; + const response = await enclavedExpressClient.signMpcGShare({ + txRequest, + bitgoToUserRShare, + userToBitgoRShare, + bitgoToUserCommitment, + source, + pub, + }); + return response.gShare; + }; +} diff --git a/src/api/master/handlers/handleConsolidate.ts b/src/api/master/handlers/handleConsolidate.ts index 30940d4d..e6ac1e56 100644 --- a/src/api/master/handlers/handleConsolidate.ts +++ b/src/api/master/handlers/handleConsolidate.ts @@ -1,7 +1,17 @@ -import { RequestTracer, KeyIndices } from '@bitgo/sdk-core'; +import { + RequestTracer, + KeyIndices, + BuildConsolidationTransactionOptions, + MPCType, +} from '@bitgo/sdk-core'; import logger from '../../../logger'; import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec'; import { getWalletAndSigningKeychain, makeCustomSigningFunction } from '../handlerUtils'; +import { + createCustomCommitmentGenerator, + createCustomRShareGenerator, + createCustomGShareGenerator, +} from './eddsa'; export async function handleConsolidate( req: MasterApiSpecRouteRequest<'v1.wallet.consolidate', 'post'>, @@ -33,20 +43,50 @@ export async function handleConsolidate( } try { - // Create custom signing function that delegates to EBE - const customSigningFunction = makeCustomSigningFunction({ - enclavedExpressClient, - source: params.source, - pub: signingKeychain.pub!, - }); - - // Prepare consolidation parameters - const consolidationParams = { + const consolidationParams: BuildConsolidationTransactionOptions = { ...params, - customSigningFunction, reqId, }; + // --- TSS/MPC support --- + if (wallet._wallet.multisigType === 'tss') { + // Always force apiVersion to 'full' for TSS/MPC + consolidationParams.apiVersion = 'full'; + + // EDDSA (e.g., Solana) + if (baseCoin.getMPCAlgorithm() === MPCType.EDDSA) { + consolidationParams.customCommitmentGeneratingFunction = createCustomCommitmentGenerator( + bitgo, + wallet, + enclavedExpressClient, + params.source, + signingKeychain.commonKeychain!, + ); + consolidationParams.customRShareGeneratingFunction = createCustomRShareGenerator( + enclavedExpressClient, + params.source, + signingKeychain.commonKeychain!, + ); + consolidationParams.customGShareGeneratingFunction = createCustomGShareGenerator( + enclavedExpressClient, + params.source, + signingKeychain.commonKeychain!, + ); + } + // ECDSA (future-proof, not needed for Solana) + else if (baseCoin.getMPCAlgorithm() === MPCType.ECDSA) { + // Add ECDSA custom signing hooks here if needed, following SDK pattern + throw new Error('ECDSA MPC consolidations not yet implemented'); + } + } else { + // Non-TSS: legacy custom signing function + consolidationParams.customSigningFunction = makeCustomSigningFunction({ + enclavedExpressClient, + source: params.source, + pub: signingKeychain.pub!, + }); + } + // Send account consolidations const result = await wallet.sendAccountConsolidations(consolidationParams); diff --git a/src/api/master/routers/masterApiSpec.ts b/src/api/master/routers/masterApiSpec.ts index a9548e50..a1ba1dbc 100644 --- a/src/api/master/routers/masterApiSpec.ts +++ b/src/api/master/routers/masterApiSpec.ts @@ -106,10 +106,11 @@ export const SendManyResponse: HttpResponse = { // Request type for /consolidate endpoint export const ConsolidateRequest = { - pubkey: t.string, + pubkey: t.union([t.undefined, t.string]), source: t.union([t.literal('user'), t.literal('backup')]), consolidateAddresses: t.union([t.undefined, t.array(t.string)]), apiVersion: t.union([t.undefined, t.literal('full'), t.literal('lite')]), + commonKeychain: t.union([t.undefined, t.string]), }; // Response type for /consolidate endpoint From d19e96e49f74cc4affcc34d4bb775403552d8bac Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Wed, 9 Jul 2025 12:03:34 -0400 Subject: [PATCH 02/10] feat(mbe): integration test for eddsa consolidation --- .../api/master/consolidateMPC.test.ts | 220 ++++++++++++++++++ src/api/master/handlers/handleConsolidate.ts | 3 - 2 files changed, 220 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/api/master/consolidateMPC.test.ts diff --git a/src/__tests__/api/master/consolidateMPC.test.ts b/src/__tests__/api/master/consolidateMPC.test.ts new file mode 100644 index 00000000..627a01db --- /dev/null +++ b/src/__tests__/api/master/consolidateMPC.test.ts @@ -0,0 +1,220 @@ +import 'should'; +import sinon from 'sinon'; +import * as request from 'supertest'; +import nock from 'nock'; +import { app as expressApp } from '../../../masterExpressApp'; +import { AppMode, MasterExpressConfig, TlsMode } from '../../../shared/types'; +import { Environments, Wallet } from '@bitgo/sdk-core'; +import * as eddsa from '../../../api/master/handlers/eddsa'; + +describe('POST /api/:coin/wallet/:walletId/consolidate (EDDSA MPC)', () => { + let agent: request.SuperAgentTest; + const coin = 'tsol'; + const walletId = 'test-wallet-id'; + const accessToken = 'test-access-token'; + const bitgoApiUrl = Environments.test.uri; + const enclavedExpressUrl = 'https://test-enclaved-express.com'; + + before(() => { + nock.disableNetConnect(); + nock.enableNetConnect('127.0.0.1'); + + const config: MasterExpressConfig = { + appMode: AppMode.MASTER_EXPRESS, + port: 0, + bind: 'localhost', + timeout: 30000, + logFile: '', + env: 'test', + disableEnvCheck: true, + authVersion: 2, + enclavedExpressUrl: enclavedExpressUrl, + enclavedExpressCert: 'test-cert', + tlsMode: TlsMode.DISABLED, + mtlsRequestCert: false, + allowSelfSigned: true, + }; + const app = expressApp(config); + agent = request.agent(app); + }); + + afterEach(() => { + nock.cleanAll(); + sinon.restore(); + }); + + it('should consolidate using EDDSA MPC custom hooks', async () => { + // Mock wallet get request + const walletGetNock = nock(bitgoApiUrl) + .get(`/api/v2/${coin}/wallet/${walletId}`) + .reply(200, { + id: walletId, + type: 'cold', + subType: 'onPrem', + keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], + multisigType: 'tss', + }); + + // Mock keychain get request + const keychainGetNock = nock(bitgoApiUrl) + .get(`/api/v2/${coin}/key/user-key-id`) + .reply(200, { + id: 'user-key-id', + commonKeychain: 'pubkey', + }); + + // Mock sendAccountConsolidations on Wallet prototype + const sendConsolidationsStub = sinon + .stub(Wallet.prototype, 'sendAccountConsolidations') + .resolves({ + success: [ + { + txid: 'mpc-txid-1', + status: 'signed', + }, + ], + failure: [], + }); + + // Spy on custom EDDSA hooks - these should return actual functions, not strings + const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); + const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); + const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); + + const commitmentSpy = sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); + const rshareSpy = sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); + const gshareSpy = sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + commonKeychain: 'pubkey', + }); + + response.status.should.equal(200); + response.body.should.have.property('success'); + response.body.success.should.have.length(1); + response.body.success[0].should.have.property('txid', 'mpc-txid-1'); + + walletGetNock.done(); + keychainGetNock.done(); + sinon.assert.calledOnce(sendConsolidationsStub); + sinon.assert.calledOnce(commitmentSpy); + sinon.assert.calledOnce(rshareSpy); + sinon.assert.calledOnce(gshareSpy); + }); + + it('should handle partial failures (some success, some failure)', async () => { + // Mock wallet get request + const walletGetNock = nock(bitgoApiUrl) + .get(`/api/v2/${coin}/wallet/${walletId}`) + .reply(200, { + id: walletId, + type: 'cold', + subType: 'onPrem', + keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], + multisigType: 'tss', + }); + + // Mock keychain get request + const keychainGetNock = nock(bitgoApiUrl) + .get(`/api/v2/${coin}/key/user-key-id`) + .reply(200, { + id: 'user-key-id', + commonKeychain: 'pubkey', + }); + + // Mock partial failure response + sinon + .stub(Wallet.prototype, 'sendAccountConsolidations') + .resolves({ + success: [{ txid: 'success-txid', status: 'signed' }], + failure: [{ error: 'Insufficient funds', address: '0xfailed' }], + }); + + // Mock EDDSA hooks + const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); + const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); + const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); + + sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); + sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); + sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + commonKeychain: 'pubkey', + consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], + }); + + response.status.should.equal(500); + response.body.should.have.property('error', 'Internal Server Error'); + response.body.should.have + .property('details') + .which.match(/Consolidations failed: 1 and succeeded: 1/); + + walletGetNock.done(); + keychainGetNock.done(); + }); + + it('should handle total failures (all failed)', async () => { + // Mock wallet get request + const walletGetNock = nock(bitgoApiUrl) + .get(`/api/v2/${coin}/wallet/${walletId}`) + .reply(200, { + id: walletId, + type: 'cold', + subType: 'onPrem', + keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], + multisigType: 'tss', + }); + + // Mock keychain get request + const keychainGetNock = nock(bitgoApiUrl) + .get(`/api/v2/${coin}/key/user-key-id`) + .reply(200, { + id: 'user-key-id', + commonKeychain: 'pubkey', + }); + + // Mock total failure response + sinon + .stub(Wallet.prototype, 'sendAccountConsolidations') + .resolves({ + success: [], + failure: [ + { error: 'Insufficient funds', address: '0xfailed1' }, + { error: 'Invalid address', address: '0xfailed2' }, + ], + }); + + // Mock EDDSA hooks + const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); + const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); + const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); + + sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); + sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); + sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + commonKeychain: 'pubkey', + }); + + response.status.should.equal(500); + response.body.should.have.property('error'); + response.body.should.have.property('details').which.match(/All consolidations failed/); + + walletGetNock.done(); + keychainGetNock.done(); + }); +}); \ No newline at end of file diff --git a/src/api/master/handlers/handleConsolidate.ts b/src/api/master/handlers/handleConsolidate.ts index e6ac1e56..55fb0b12 100644 --- a/src/api/master/handlers/handleConsolidate.ts +++ b/src/api/master/handlers/handleConsolidate.ts @@ -53,7 +53,6 @@ export async function handleConsolidate( // Always force apiVersion to 'full' for TSS/MPC consolidationParams.apiVersion = 'full'; - // EDDSA (e.g., Solana) if (baseCoin.getMPCAlgorithm() === MPCType.EDDSA) { consolidationParams.customCommitmentGeneratingFunction = createCustomCommitmentGenerator( bitgo, @@ -73,9 +72,7 @@ export async function handleConsolidate( signingKeychain.commonKeychain!, ); } - // ECDSA (future-proof, not needed for Solana) else if (baseCoin.getMPCAlgorithm() === MPCType.ECDSA) { - // Add ECDSA custom signing hooks here if needed, following SDK pattern throw new Error('ECDSA MPC consolidations not yet implemented'); } } else { From 7b98d91517e0baa48aefa4b466146c30ab6d5434 Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Wed, 9 Jul 2025 12:19:42 -0400 Subject: [PATCH 03/10] feat(mbe): fix linting --- .../api/master/consolidateMPC.test.ts | 70 ++++++++----------- src/api/master/handlers/handleConsolidate.ts | 3 +- 2 files changed, 32 insertions(+), 41 deletions(-) diff --git a/src/__tests__/api/master/consolidateMPC.test.ts b/src/__tests__/api/master/consolidateMPC.test.ts index 627a01db..d1c2aa04 100644 --- a/src/__tests__/api/master/consolidateMPC.test.ts +++ b/src/__tests__/api/master/consolidateMPC.test.ts @@ -56,12 +56,10 @@ describe('POST /api/:coin/wallet/:walletId/consolidate (EDDSA MPC)', () => { }); // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/key/user-key-id`) - .reply(200, { - id: 'user-key-id', - commonKeychain: 'pubkey', - }); + const keychainGetNock = nock(bitgoApiUrl).get(`/api/v2/${coin}/key/user-key-id`).reply(200, { + id: 'user-key-id', + commonKeychain: 'pubkey', + }); // Mock sendAccountConsolidations on Wallet prototype const sendConsolidationsStub = sinon @@ -80,8 +78,10 @@ describe('POST /api/:coin/wallet/:walletId/consolidate (EDDSA MPC)', () => { const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); - - const commitmentSpy = sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); + + const commitmentSpy = sinon + .stub(eddsa, 'createCustomCommitmentGenerator') + .returns(mockCommitmentFn); const rshareSpy = sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); const gshareSpy = sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); @@ -119,26 +119,22 @@ describe('POST /api/:coin/wallet/:walletId/consolidate (EDDSA MPC)', () => { }); // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/key/user-key-id`) - .reply(200, { - id: 'user-key-id', - commonKeychain: 'pubkey', - }); + const keychainGetNock = nock(bitgoApiUrl).get(`/api/v2/${coin}/key/user-key-id`).reply(200, { + id: 'user-key-id', + commonKeychain: 'pubkey', + }); // Mock partial failure response - sinon - .stub(Wallet.prototype, 'sendAccountConsolidations') - .resolves({ - success: [{ txid: 'success-txid', status: 'signed' }], - failure: [{ error: 'Insufficient funds', address: '0xfailed' }], - }); + sinon.stub(Wallet.prototype, 'sendAccountConsolidations').resolves({ + success: [{ txid: 'success-txid', status: 'signed' }], + failure: [{ error: 'Insufficient funds', address: '0xfailed' }], + }); // Mock EDDSA hooks const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); - + sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); @@ -175,29 +171,25 @@ describe('POST /api/:coin/wallet/:walletId/consolidate (EDDSA MPC)', () => { }); // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/key/user-key-id`) - .reply(200, { - id: 'user-key-id', - commonKeychain: 'pubkey', - }); + const keychainGetNock = nock(bitgoApiUrl).get(`/api/v2/${coin}/key/user-key-id`).reply(200, { + id: 'user-key-id', + commonKeychain: 'pubkey', + }); // Mock total failure response - sinon - .stub(Wallet.prototype, 'sendAccountConsolidations') - .resolves({ - success: [], - failure: [ - { error: 'Insufficient funds', address: '0xfailed1' }, - { error: 'Invalid address', address: '0xfailed2' }, - ], - }); + sinon.stub(Wallet.prototype, 'sendAccountConsolidations').resolves({ + success: [], + failure: [ + { error: 'Insufficient funds', address: '0xfailed1' }, + { error: 'Invalid address', address: '0xfailed2' }, + ], + }); // Mock EDDSA hooks const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); - + sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); @@ -213,8 +205,8 @@ describe('POST /api/:coin/wallet/:walletId/consolidate (EDDSA MPC)', () => { response.status.should.equal(500); response.body.should.have.property('error'); response.body.should.have.property('details').which.match(/All consolidations failed/); - + walletGetNock.done(); keychainGetNock.done(); }); -}); \ No newline at end of file +}); diff --git a/src/api/master/handlers/handleConsolidate.ts b/src/api/master/handlers/handleConsolidate.ts index 55fb0b12..f43f4801 100644 --- a/src/api/master/handlers/handleConsolidate.ts +++ b/src/api/master/handlers/handleConsolidate.ts @@ -71,8 +71,7 @@ export async function handleConsolidate( params.source, signingKeychain.commonKeychain!, ); - } - else if (baseCoin.getMPCAlgorithm() === MPCType.ECDSA) { + } else if (baseCoin.getMPCAlgorithm() === MPCType.ECDSA) { throw new Error('ECDSA MPC consolidations not yet implemented'); } } else { From 87d9bd80428404cb565dd1516cfeac591ff1b84b Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Thu, 10 Jul 2025 10:10:03 -0400 Subject: [PATCH 04/10] feat(mbe): refactor consolidation --- .../api/master/consolidateMPC.test.ts | 212 ------------------ src/api/master/handlers/eddsa.ts | 71 ------ src/api/master/handlers/handleConsolidate.ts | 107 +++++---- src/api/master/handlers/handleSendMany.ts | 94 +++++++- 4 files changed, 147 insertions(+), 337 deletions(-) delete mode 100644 src/__tests__/api/master/consolidateMPC.test.ts diff --git a/src/__tests__/api/master/consolidateMPC.test.ts b/src/__tests__/api/master/consolidateMPC.test.ts deleted file mode 100644 index d1c2aa04..00000000 --- a/src/__tests__/api/master/consolidateMPC.test.ts +++ /dev/null @@ -1,212 +0,0 @@ -import 'should'; -import sinon from 'sinon'; -import * as request from 'supertest'; -import nock from 'nock'; -import { app as expressApp } from '../../../masterExpressApp'; -import { AppMode, MasterExpressConfig, TlsMode } from '../../../shared/types'; -import { Environments, Wallet } from '@bitgo/sdk-core'; -import * as eddsa from '../../../api/master/handlers/eddsa'; - -describe('POST /api/:coin/wallet/:walletId/consolidate (EDDSA MPC)', () => { - let agent: request.SuperAgentTest; - const coin = 'tsol'; - const walletId = 'test-wallet-id'; - const accessToken = 'test-access-token'; - const bitgoApiUrl = Environments.test.uri; - const enclavedExpressUrl = 'https://test-enclaved-express.com'; - - before(() => { - nock.disableNetConnect(); - nock.enableNetConnect('127.0.0.1'); - - const config: MasterExpressConfig = { - appMode: AppMode.MASTER_EXPRESS, - port: 0, - bind: 'localhost', - timeout: 30000, - logFile: '', - env: 'test', - disableEnvCheck: true, - authVersion: 2, - enclavedExpressUrl: enclavedExpressUrl, - enclavedExpressCert: 'test-cert', - tlsMode: TlsMode.DISABLED, - mtlsRequestCert: false, - allowSelfSigned: true, - }; - const app = expressApp(config); - agent = request.agent(app); - }); - - afterEach(() => { - nock.cleanAll(); - sinon.restore(); - }); - - it('should consolidate using EDDSA MPC custom hooks', async () => { - // Mock wallet get request - const walletGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/wallet/${walletId}`) - .reply(200, { - id: walletId, - type: 'cold', - subType: 'onPrem', - keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], - multisigType: 'tss', - }); - - // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl).get(`/api/v2/${coin}/key/user-key-id`).reply(200, { - id: 'user-key-id', - commonKeychain: 'pubkey', - }); - - // Mock sendAccountConsolidations on Wallet prototype - const sendConsolidationsStub = sinon - .stub(Wallet.prototype, 'sendAccountConsolidations') - .resolves({ - success: [ - { - txid: 'mpc-txid-1', - status: 'signed', - }, - ], - failure: [], - }); - - // Spy on custom EDDSA hooks - these should return actual functions, not strings - const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); - const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); - const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); - - const commitmentSpy = sinon - .stub(eddsa, 'createCustomCommitmentGenerator') - .returns(mockCommitmentFn); - const rshareSpy = sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); - const gshareSpy = sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); - - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - commonKeychain: 'pubkey', - }); - - response.status.should.equal(200); - response.body.should.have.property('success'); - response.body.success.should.have.length(1); - response.body.success[0].should.have.property('txid', 'mpc-txid-1'); - - walletGetNock.done(); - keychainGetNock.done(); - sinon.assert.calledOnce(sendConsolidationsStub); - sinon.assert.calledOnce(commitmentSpy); - sinon.assert.calledOnce(rshareSpy); - sinon.assert.calledOnce(gshareSpy); - }); - - it('should handle partial failures (some success, some failure)', async () => { - // Mock wallet get request - const walletGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/wallet/${walletId}`) - .reply(200, { - id: walletId, - type: 'cold', - subType: 'onPrem', - keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], - multisigType: 'tss', - }); - - // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl).get(`/api/v2/${coin}/key/user-key-id`).reply(200, { - id: 'user-key-id', - commonKeychain: 'pubkey', - }); - - // Mock partial failure response - sinon.stub(Wallet.prototype, 'sendAccountConsolidations').resolves({ - success: [{ txid: 'success-txid', status: 'signed' }], - failure: [{ error: 'Insufficient funds', address: '0xfailed' }], - }); - - // Mock EDDSA hooks - const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); - const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); - const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); - - sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); - sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); - sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); - - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - commonKeychain: 'pubkey', - consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], - }); - - response.status.should.equal(500); - response.body.should.have.property('error', 'Internal Server Error'); - response.body.should.have - .property('details') - .which.match(/Consolidations failed: 1 and succeeded: 1/); - - walletGetNock.done(); - keychainGetNock.done(); - }); - - it('should handle total failures (all failed)', async () => { - // Mock wallet get request - const walletGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/wallet/${walletId}`) - .reply(200, { - id: walletId, - type: 'cold', - subType: 'onPrem', - keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], - multisigType: 'tss', - }); - - // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl).get(`/api/v2/${coin}/key/user-key-id`).reply(200, { - id: 'user-key-id', - commonKeychain: 'pubkey', - }); - - // Mock total failure response - sinon.stub(Wallet.prototype, 'sendAccountConsolidations').resolves({ - success: [], - failure: [ - { error: 'Insufficient funds', address: '0xfailed1' }, - { error: 'Invalid address', address: '0xfailed2' }, - ], - }); - - // Mock EDDSA hooks - const mockCommitmentFn = sinon.stub().resolves({ userToBitgoCommitment: 'commitment' }); - const mockRShareFn = sinon.stub().resolves({ rShare: 'rshare' }); - const mockGShareFn = sinon.stub().resolves({ gShare: 'gshare' }); - - sinon.stub(eddsa, 'createCustomCommitmentGenerator').returns(mockCommitmentFn); - sinon.stub(eddsa, 'createCustomRShareGenerator').returns(mockRShareFn); - sinon.stub(eddsa, 'createCustomGShareGenerator').returns(mockGShareFn); - - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - commonKeychain: 'pubkey', - }); - - response.status.should.equal(500); - response.body.should.have.property('error'); - response.body.should.have.property('details').which.match(/All consolidations failed/); - - walletGetNock.done(); - keychainGetNock.done(); - }); -}); diff --git a/src/api/master/handlers/eddsa.ts b/src/api/master/handlers/eddsa.ts index 88fe7274..edc76f23 100644 --- a/src/api/master/handlers/eddsa.ts +++ b/src/api/master/handlers/eddsa.ts @@ -9,10 +9,6 @@ import { EddsaUtils, BaseCoin, ApiKeyShare, - TxRequest, - CustomRShareGeneratingFunction, - CustomGShareGeneratingFunction, - CustomCommitmentGeneratingFunction, } from '@bitgo/sdk-core'; import { EnclavedExpressClient } from '../clients/enclavedExpressClient'; import { exchangeEddsaCommitments } from '@bitgo/sdk-core/dist/src/bitgo/tss/common'; @@ -206,70 +202,3 @@ export async function orchestrateEddsaKeyGen({ walletParams.keys = [userMpcKey.id, backupMpcKey.id, bitgoKeychain.id]; return { walletParams, keychains }; } - -// Commitment -export function createCustomCommitmentGenerator( - bitgo: BitGoBase, - wallet: Wallet, - enclavedExpressClient: EnclavedExpressClient, - source: 'user' | 'backup', - pub: string, -): CustomCommitmentGeneratingFunction { - return async function customCommitmentGeneratingFunction(params) { - const eddsaUtils = new EddsaUtils(bitgo, wallet.baseCoin); - const bitgoGpgKey = await eddsaUtils.getBitgoPublicGpgKey(); - const { txRequest } = params; - const response = await enclavedExpressClient.signMpcCommitment({ - txRequest, - bitgoGpgPubKey: bitgoGpgKey.armor(), - source, - pub, - }); - return { - ...response, - encryptedUserToBitgoRShare: { - ...response.encryptedUserToBitgoRShare, - encryptedDataKey: response.encryptedDataKey, - }, - }; - }; -} - -// RShare -export function createCustomRShareGenerator( - enclavedExpressClient: EnclavedExpressClient, - source: 'user' | 'backup', - pub: string, -): CustomRShareGeneratingFunction { - return async function customRShareGeneratingFunction(params) { - const { txRequest, encryptedUserToBitgoRShare } = params; - const encryptedDataKey = (encryptedUserToBitgoRShare as any).encryptedDataKey; - return await enclavedExpressClient.signMpcRShare({ - txRequest, - encryptedUserToBitgoRShare, - encryptedDataKey, - source, - pub, - }); - }; -} - -// GShare -export function createCustomGShareGenerator( - enclavedExpressClient: EnclavedExpressClient, - source: 'user' | 'backup', - pub: string, -): CustomGShareGeneratingFunction { - return async function customGShareGeneratingFunction(params) { - const { txRequest, bitgoToUserRShare, userToBitgoRShare, bitgoToUserCommitment } = params; - const response = await enclavedExpressClient.signMpcGShare({ - txRequest, - bitgoToUserRShare, - userToBitgoRShare, - bitgoToUserCommitment, - source, - pub, - }); - return response.gShare; - }; -} diff --git a/src/api/master/handlers/handleConsolidate.ts b/src/api/master/handlers/handleConsolidate.ts index f43f4801..c8b66a51 100644 --- a/src/api/master/handlers/handleConsolidate.ts +++ b/src/api/master/handlers/handleConsolidate.ts @@ -2,16 +2,12 @@ import { RequestTracer, KeyIndices, BuildConsolidationTransactionOptions, - MPCType, + PrebuildAndSignTransactionOptions, } from '@bitgo/sdk-core'; import logger from '../../../logger'; import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec'; -import { getWalletAndSigningKeychain, makeCustomSigningFunction } from '../handlerUtils'; -import { - createCustomCommitmentGenerator, - createCustomRShareGenerator, - createCustomGShareGenerator, -} from './eddsa'; +import { getWalletAndSigningKeychain } from '../handlerUtils'; +import { signAndSendMultisig, signAndSendTxRequests } from './handleSendMany'; export async function handleConsolidate( req: MasterApiSpecRouteRequest<'v1.wallet.consolidate', 'post'>, @@ -42,59 +38,70 @@ export async function handleConsolidate( throw new Error('consolidateAddresses must be an array of addresses'); } + const isMPC = wallet._wallet.multisigType === 'tss'; + try { const consolidationParams: BuildConsolidationTransactionOptions = { ...params, reqId, }; - // --- TSS/MPC support --- - if (wallet._wallet.multisigType === 'tss') { - // Always force apiVersion to 'full' for TSS/MPC - consolidationParams.apiVersion = 'full'; - - if (baseCoin.getMPCAlgorithm() === MPCType.EDDSA) { - consolidationParams.customCommitmentGeneratingFunction = createCustomCommitmentGenerator( - bitgo, - wallet, - enclavedExpressClient, - params.source, - signingKeychain.commonKeychain!, - ); - consolidationParams.customRShareGeneratingFunction = createCustomRShareGenerator( - enclavedExpressClient, - params.source, - signingKeychain.commonKeychain!, - ); - consolidationParams.customGShareGeneratingFunction = createCustomGShareGenerator( - enclavedExpressClient, - params.source, - signingKeychain.commonKeychain!, + isMPC && (consolidationParams.apiVersion = 'full'); + + const successfulTxs: any[] = []; + const failedTxs = new Array(); + + const unsignedBuilds = await wallet.buildAccountConsolidations(consolidationParams); + + logger.info( + `Consolidation request for wallet ${walletId} with ${unsignedBuilds.length} unsigned builds`, + ); + + if (unsignedBuilds && unsignedBuilds.length > 0) { + for (const unsignedBuild of unsignedBuilds) { + const unsignedBuildWithOptions: PrebuildAndSignTransactionOptions = Object.assign( + {}, + consolidationParams, ); - } else if (baseCoin.getMPCAlgorithm() === MPCType.ECDSA) { - throw new Error('ECDSA MPC consolidations not yet implemented'); + unsignedBuildWithOptions.apiVersion = consolidationParams.apiVersion; + unsignedBuildWithOptions.prebuildTx = unsignedBuild; + + try { + const sendTx = isMPC + ? await signAndSendTxRequests( + bitgo, + wallet, + unsignedBuild.txRequestId!, + enclavedExpressClient, + signingKeychain, + reqId, + ) + : await signAndSendMultisig( + wallet, + params.source, + unsignedBuild, + unsignedBuildWithOptions, + enclavedExpressClient, + signingKeychain, + reqId, + ); + + successfulTxs.push(sendTx); + } catch (e) { + console.dir(e); + failedTxs.push(e as any); + } } - } else { - // Non-TSS: legacy custom signing function - consolidationParams.customSigningFunction = makeCustomSigningFunction({ - enclavedExpressClient, - source: params.source, - pub: signingKeychain.pub!, - }); } - // Send account consolidations - const result = await wallet.sendAccountConsolidations(consolidationParams); - // Handle failures - if (result.failure && result.failure.length > 0) { - logger.debug('Consolidation result: %s', JSON.stringify(result, null, 2)); + if (failedTxs.length > 0) { let msg = ''; let status = 202; - if (result.success && result.success.length > 0) { + if (successfulTxs.length > 0) { // Some succeeded, some failed - msg = `Consolidations failed: ${result.failure.length} and succeeded: ${result.success.length}`; + msg = `Consolidations failed: ${failedTxs.length} and succeeded: ${successfulTxs.length}`; } else { // All failed status = 400; @@ -103,11 +110,17 @@ export async function handleConsolidate( const error = new Error(msg); (error as any).status = status; - (error as any).result = result; + (error as any).result = { + success: successfulTxs, + failure: failedTxs, + }; throw error; } - return result; + return { + success: successfulTxs, + failure: failedTxs, + }; } catch (error) { const err = error as Error; logger.error('Failed to consolidate account: %s', err.message); diff --git a/src/api/master/handlers/handleSendMany.ts b/src/api/master/handlers/handleSendMany.ts index 46fdc5b8..26b65a9c 100644 --- a/src/api/master/handlers/handleSendMany.ts +++ b/src/api/master/handlers/handleSendMany.ts @@ -192,7 +192,7 @@ export async function handleSendMany(req: MasterApiSpecRouteRequest<'v1.wallet.s } } -async function signAndSendMultisig( +export async function signAndSendMultisig( wallet: Wallet, source: 'user' | 'backup', txPrebuilt: PrebuildTransactionResult, @@ -214,16 +214,96 @@ async function signAndSendMultisig( pub: signingKeychain.pub, }); + console.log('Signed transaction:', JSON.stringify(signedTx, null, 2)); + // Get extra prebuild parameters - const extraParams = await wallet.baseCoin.getExtraPrebuildParams({ - ...params, - wallet, - }); + const extraParams = + Array.isArray(params.recipients) && params.recipients.length > 0 + ? await wallet.baseCoin.getExtraPrebuildParams({ + ...params, + wallet, + }) + : {}; // Combine the signed transaction with extra parameters const finalTxParams = { ...signedTx, ...extraParams }; + console.log('Final transaction parameters:', JSON.stringify(finalTxParams, null, 2)); // Submit the half signed transaction - const result = (await wallet.submitTransaction(finalTxParams, reqId)) as any; - return result; + return await wallet.submitTransaction(finalTxParams, reqId); +} + +/** + * Signs and sends a transaction from a TSS wallet. + * + * @param bitgo - BitGo instance + * @param wallet - Wallet instance + * @param txRequestId - Transaction request ID + * @param enclavedExpressClient - Enclaved express client + * @param signingKeychain - Signing keychain + * @param reqId - Request tracer + */ +export async function signAndSendTxRequests( + bitgo: BitGoBase, + wallet: Wallet, + txRequestId: string, + enclavedExpressClient: EnclavedExpressClient, + signingKeychain: Keychain, + reqId: RequestTracer, +): Promise { + if (!signingKeychain.commonKeychain) { + throw new Error(`Common keychain not found for keychain ${signingKeychain.pub || 'unknown'}`); + } + if (signingKeychain.source === 'backup') { + throw new Error('Backup MPC signing not supported for sendMany'); + } + + let signedTxRequest: TxRequest; + const mpcAlgorithm = wallet.baseCoin.getMPCAlgorithm(); + + if (mpcAlgorithm === 'eddsa') { + signedTxRequest = await handleEddsaSigning( + bitgo, + wallet, + txRequestId, + enclavedExpressClient, + signingKeychain.commonKeychain, + reqId, + ); + } else if (mpcAlgorithm === 'ecdsa') { + signedTxRequest = await handleEcdsaSigning( + bitgo, + wallet, + txRequestId, + enclavedExpressClient, + signingKeychain.source as 'user' | 'backup', + signingKeychain.commonKeychain, + reqId, + ); + } else { + throw new Error(`Unsupported MPC algorithm: ${mpcAlgorithm}`); + } + + if (!signedTxRequest.txRequestId) { + throw new Error('txRequestId missing from signed transaction'); + } + + if (signedTxRequest.apiVersion !== 'full') { + throw new Error('Only TxRequest API version full is supported.'); + } + + bitgo.setRequestTracer(reqId); + if (signedTxRequest.state === 'pendingApproval') { + const pendingApprovals = new PendingApprovals(bitgo, wallet.baseCoin); + const pendingApproval = await pendingApprovals.get({ id: signedTxRequest.pendingApprovalId }); + return { + pendingApproval: pendingApproval.toJSON(), + txRequest: signedTxRequest, + }; + } + return { + txRequest: signedTxRequest, + txid: (signedTxRequest.transactions ?? [])[0]?.signedTx?.id, + tx: (signedTxRequest.transactions ?? [])[0]?.signedTx?.tx, + }; } From e4c82f848769eb3bff5938c98a68ffb8735690ed Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Thu, 10 Jul 2025 10:39:18 -0400 Subject: [PATCH 05/10] feat(mbe): fix rebase conflict --- src/api/master/handlers/handleConsolidate.ts | 8 ++- src/api/master/handlers/handleSendMany.ts | 75 -------------------- 2 files changed, 6 insertions(+), 77 deletions(-) diff --git a/src/api/master/handlers/handleConsolidate.ts b/src/api/master/handlers/handleConsolidate.ts index c8b66a51..809ab8d8 100644 --- a/src/api/master/handlers/handleConsolidate.ts +++ b/src/api/master/handlers/handleConsolidate.ts @@ -3,11 +3,13 @@ import { KeyIndices, BuildConsolidationTransactionOptions, PrebuildAndSignTransactionOptions, + getTxRequest, } from '@bitgo/sdk-core'; import logger from '../../../logger'; import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec'; import { getWalletAndSigningKeychain } from '../handlerUtils'; -import { signAndSendMultisig, signAndSendTxRequests } from './handleSendMany'; +import { signAndSendMultisig } from './handleSendMany'; +import { signAndSendTxRequests } from './transactionRequests'; export async function handleConsolidate( req: MasterApiSpecRouteRequest<'v1.wallet.consolidate', 'post'>, @@ -66,12 +68,14 @@ export async function handleConsolidate( unsignedBuildWithOptions.apiVersion = consolidationParams.apiVersion; unsignedBuildWithOptions.prebuildTx = unsignedBuild; + const txRequest = await getTxRequest(bitgo, wallet.id(), unsignedBuild.txRequestId!, reqId); + try { const sendTx = isMPC ? await signAndSendTxRequests( bitgo, wallet, - unsignedBuild.txRequestId!, + txRequest, enclavedExpressClient, signingKeychain, reqId, diff --git a/src/api/master/handlers/handleSendMany.ts b/src/api/master/handlers/handleSendMany.ts index 26b65a9c..e2581b18 100644 --- a/src/api/master/handlers/handleSendMany.ts +++ b/src/api/master/handlers/handleSendMany.ts @@ -232,78 +232,3 @@ export async function signAndSendMultisig( // Submit the half signed transaction return await wallet.submitTransaction(finalTxParams, reqId); } - -/** - * Signs and sends a transaction from a TSS wallet. - * - * @param bitgo - BitGo instance - * @param wallet - Wallet instance - * @param txRequestId - Transaction request ID - * @param enclavedExpressClient - Enclaved express client - * @param signingKeychain - Signing keychain - * @param reqId - Request tracer - */ -export async function signAndSendTxRequests( - bitgo: BitGoBase, - wallet: Wallet, - txRequestId: string, - enclavedExpressClient: EnclavedExpressClient, - signingKeychain: Keychain, - reqId: RequestTracer, -): Promise { - if (!signingKeychain.commonKeychain) { - throw new Error(`Common keychain not found for keychain ${signingKeychain.pub || 'unknown'}`); - } - if (signingKeychain.source === 'backup') { - throw new Error('Backup MPC signing not supported for sendMany'); - } - - let signedTxRequest: TxRequest; - const mpcAlgorithm = wallet.baseCoin.getMPCAlgorithm(); - - if (mpcAlgorithm === 'eddsa') { - signedTxRequest = await handleEddsaSigning( - bitgo, - wallet, - txRequestId, - enclavedExpressClient, - signingKeychain.commonKeychain, - reqId, - ); - } else if (mpcAlgorithm === 'ecdsa') { - signedTxRequest = await handleEcdsaSigning( - bitgo, - wallet, - txRequestId, - enclavedExpressClient, - signingKeychain.source as 'user' | 'backup', - signingKeychain.commonKeychain, - reqId, - ); - } else { - throw new Error(`Unsupported MPC algorithm: ${mpcAlgorithm}`); - } - - if (!signedTxRequest.txRequestId) { - throw new Error('txRequestId missing from signed transaction'); - } - - if (signedTxRequest.apiVersion !== 'full') { - throw new Error('Only TxRequest API version full is supported.'); - } - - bitgo.setRequestTracer(reqId); - if (signedTxRequest.state === 'pendingApproval') { - const pendingApprovals = new PendingApprovals(bitgo, wallet.baseCoin); - const pendingApproval = await pendingApprovals.get({ id: signedTxRequest.pendingApprovalId }); - return { - pendingApproval: pendingApproval.toJSON(), - txRequest: signedTxRequest, - }; - } - return { - txRequest: signedTxRequest, - txid: (signedTxRequest.transactions ?? [])[0]?.signedTx?.id, - tx: (signedTxRequest.transactions ?? [])[0]?.signedTx?.tx, - }; -} From caec81e356240f197c955c757ea7d192331b4dd6 Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Thu, 10 Jul 2025 15:54:42 -0400 Subject: [PATCH 06/10] feat(mbe): final logic update --- src/api/master/handlers/eddsa.ts | 1 + src/api/master/handlers/handleConsolidate.ts | 51 ++++++++++---------- src/api/master/handlers/handleSendMany.ts | 17 +++---- 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/api/master/handlers/eddsa.ts b/src/api/master/handlers/eddsa.ts index edc76f23..6a1d3e99 100644 --- a/src/api/master/handlers/eddsa.ts +++ b/src/api/master/handlers/eddsa.ts @@ -9,6 +9,7 @@ import { EddsaUtils, BaseCoin, ApiKeyShare, + TxRequest, } from '@bitgo/sdk-core'; import { EnclavedExpressClient } from '../clients/enclavedExpressClient'; import { exchangeEddsaCommitments } from '@bitgo/sdk-core/dist/src/bitgo/tss/common'; diff --git a/src/api/master/handlers/handleConsolidate.ts b/src/api/master/handlers/handleConsolidate.ts index 809ab8d8..611792f7 100644 --- a/src/api/master/handlers/handleConsolidate.ts +++ b/src/api/master/handlers/handleConsolidate.ts @@ -2,13 +2,11 @@ import { RequestTracer, KeyIndices, BuildConsolidationTransactionOptions, - PrebuildAndSignTransactionOptions, getTxRequest, } from '@bitgo/sdk-core'; import logger from '../../../logger'; import { MasterApiSpecRouteRequest } from '../routers/masterApiSpec'; -import { getWalletAndSigningKeychain } from '../handlerUtils'; -import { signAndSendMultisig } from './handleSendMany'; +import { getWalletAndSigningKeychain, makeCustomSigningFunction } from '../handlerUtils'; import { signAndSendTxRequests } from './transactionRequests'; export async function handleConsolidate( @@ -55,42 +53,43 @@ export async function handleConsolidate( const unsignedBuilds = await wallet.buildAccountConsolidations(consolidationParams); - logger.info( + logger.debug( `Consolidation request for wallet ${walletId} with ${unsignedBuilds.length} unsigned builds`, ); if (unsignedBuilds && unsignedBuilds.length > 0) { for (const unsignedBuild of unsignedBuilds) { - const unsignedBuildWithOptions: PrebuildAndSignTransactionOptions = Object.assign( - {}, - consolidationParams, - ); - unsignedBuildWithOptions.apiVersion = consolidationParams.apiVersion; - unsignedBuildWithOptions.prebuildTx = unsignedBuild; - - const txRequest = await getTxRequest(bitgo, wallet.id(), unsignedBuild.txRequestId!, reqId); - try { - const sendTx = isMPC + const result = isMPC ? await signAndSendTxRequests( bitgo, wallet, - txRequest, + await getTxRequest( + bitgo, + wallet.id(), + (() => { + if (!unsignedBuild.txRequestId) { + throw new Error('Missing txRequestId in unsigned build'); + } + return unsignedBuild.txRequestId; + })(), + reqId, + ), enclavedExpressClient, signingKeychain, reqId, ) - : await signAndSendMultisig( - wallet, - params.source, - unsignedBuild, - unsignedBuildWithOptions, - enclavedExpressClient, - signingKeychain, - reqId, - ); - - successfulTxs.push(sendTx); + : await wallet.sendAccountConsolidation({ + ...consolidationParams, + prebuildTx: unsignedBuild, + customSigningFunction: makeCustomSigningFunction({ + enclavedExpressClient, + source: params.source, + pub: signingKeychain.pub!, + }), + }); + + successfulTxs.push(result); } catch (e) { console.dir(e); failedTxs.push(e as any); diff --git a/src/api/master/handlers/handleSendMany.ts b/src/api/master/handlers/handleSendMany.ts index e2581b18..31299dea 100644 --- a/src/api/master/handlers/handleSendMany.ts +++ b/src/api/master/handlers/handleSendMany.ts @@ -214,21 +214,16 @@ export async function signAndSendMultisig( pub: signingKeychain.pub, }); - console.log('Signed transaction:', JSON.stringify(signedTx, null, 2)); - // Get extra prebuild parameters - const extraParams = - Array.isArray(params.recipients) && params.recipients.length > 0 - ? await wallet.baseCoin.getExtraPrebuildParams({ - ...params, - wallet, - }) - : {}; + const extraParams = await wallet.baseCoin.getExtraPrebuildParams({ + ...params, + wallet, + }); // Combine the signed transaction with extra parameters const finalTxParams = { ...signedTx, ...extraParams }; - console.log('Final transaction parameters:', JSON.stringify(finalTxParams, null, 2)); // Submit the half signed transaction - return await wallet.submitTransaction(finalTxParams, reqId); + const result = (await wallet.submitTransaction(finalTxParams, reqId)) as any; + return result; } From 0338d1b6789ed1dbfafa8eaecf066cac9d465578 Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Fri, 11 Jul 2025 11:03:09 -0400 Subject: [PATCH 07/10] feat(mbe): complete integration test --- src/__tests__/api/master/consolidate.test.ts | 201 +++++-------------- 1 file changed, 52 insertions(+), 149 deletions(-) diff --git a/src/__tests__/api/master/consolidate.test.ts b/src/__tests__/api/master/consolidate.test.ts index 352a9269..3665c53e 100644 --- a/src/__tests__/api/master/consolidate.test.ts +++ b/src/__tests__/api/master/consolidate.test.ts @@ -5,11 +5,10 @@ import nock from 'nock'; import { app as expressApp } from '../../../masterExpressApp'; import { AppMode, MasterExpressConfig, TlsMode } from '../../../shared/types'; import { Environments, Wallet } from '@bitgo/sdk-core'; -import { Hteth } from '@bitgo/sdk-coin-eth'; -describe('POST /api/:coin/wallet/:walletId/consolidate', () => { +describe('POST /api/:coin/wallet/:walletId/consolidateunspents', () => { let agent: request.SuperAgentTest; - const coin = 'hteth'; + const coin = 'btc'; const walletId = 'test-wallet-id'; const accessToken = 'test-access-token'; const bitgoApiUrl = Environments.test.uri; @@ -21,7 +20,7 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { const config: MasterExpressConfig = { appMode: AppMode.MASTER_EXPRESS, - port: 0, // Let OS assign a free port + port: 0, bind: 'localhost', timeout: 30000, logFile: '', @@ -44,8 +43,7 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { sinon.restore(); }); - it('should consolidate account addresses by calling the enclaved express service', async () => { - // Mock wallet get request + it('should return transfer, txid, tx, and status on success', async () => { const walletGetNock = nock(bitgoApiUrl) .get(`/api/v2/${coin}/wallet/${walletId}`) .matchHeader('any', () => true) @@ -56,7 +54,6 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], }); - // Mock keychain get request const keychainGetNock = nock(bitgoApiUrl) .get(`/api/v2/${coin}/key/user-key-id`) .matchHeader('any', () => true) @@ -65,99 +62,52 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { pub: 'xpub_user', }); - // Mock sendAccountConsolidations - const sendConsolidationsStub = sinon - .stub(Wallet.prototype, 'sendAccountConsolidations') - .resolves({ - success: [ - { - txid: 'consolidation-tx-1', - status: 'signed', - }, + const mockResult = { + transfer: { + entries: [ + { address: 'tb1qu...', value: -4000 }, + { address: 'tb1qle...', value: -4000 }, + { address: 'tb1qtw...', value: 2714, isChange: true }, ], - failure: [], - }); - - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - pubkey: 'xpub_user', - consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], - }); - - response.status.should.equal(200); - response.body.should.have.property('success'); - response.body.success.should.have.length(1); - response.body.success[0].should.have.property('txid', 'consolidation-tx-1'); - - walletGetNock.done(); - keychainGetNock.done(); - sinon.assert.calledOnce(sendConsolidationsStub); - }); - - it('should handle partial consolidation failures', async () => { - // Mock wallet get request - const walletGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/wallet/${walletId}`) - .matchHeader('any', () => true) - .reply(200, { - id: walletId, - type: 'cold', - subType: 'onPrem', - keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], - }); - - // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/key/user-key-id`) - .matchHeader('any', () => true) - .reply(200, { - id: 'user-key-id', - pub: 'xpub_user', - }); + id: '685ac2f3c2f8a2a5d9cc18d3593f1751', + coin: 'tbtc', + wallet: '685abbf19ca95b79f88e0b41d9337109', + txid: '239d143cdfc6d6c83a935da4f3d610b2364a956c7b6dcdc165eb706f62c4432a', + status: 'signed', + }, + txid: '239d143cdfc6d6c83a935da4f3d610b2364a956c7b6dcdc165eb706f62c4432a', + tx: '01000000000102580b...', + status: 'signed', + }; - // Mock sendAccountConsolidations with partial failures - const sendConsolidationsStub = sinon - .stub(Wallet.prototype, 'sendAccountConsolidations') - .resolves({ - success: [ - { - txid: 'consolidation-tx-1', - status: 'signed', - }, - ], - failure: [ - { - error: 'Insufficient funds', - address: '0xfedcba0987654321', - }, - ], - }); + const consolidateUnspentsStub = sinon + .stub(Wallet.prototype, 'consolidateUnspents') + .resolves(mockResult); const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .post(`/api/${coin}/wallet/${walletId}/consolidateunspents`) .set('Authorization', `Bearer ${accessToken}`) .send({ source: 'user', pubkey: 'xpub_user', - consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], + feeRate: 1000, }); - response.status.should.equal(500); - response.body.should.have.property('error', 'Internal Server Error'); - response.body.should.have - .property('details') - .which.match(/Consolidations failed: 1 and succeeded: 1/); + response.status.should.equal(200); + response.body.should.have.property('transfer'); + response.body.should.have.property('txid', mockResult.txid); + response.body.should.have.property('tx', mockResult.tx); + response.body.should.have.property('status', mockResult.status); + response.body.transfer.should.have.property('txid', mockResult.transfer.txid); + response.body.transfer.should.have.property('status', mockResult.transfer.status); + response.body.transfer.should.have.property('entries').which.is.Array(); walletGetNock.done(); keychainGetNock.done(); - sinon.assert.calledOnce(sendConsolidationsStub); + sinon.assert.calledOnce(consolidateUnspentsStub); }); - it('should throw error when all consolidations fail', async () => { - // Mock wallet get request + it('should return error, name, and details on failure', async () => { const walletGetNock = nock(bitgoApiUrl) .get(`/api/v2/${coin}/wallet/${walletId}`) .matchHeader('any', () => true) @@ -168,7 +118,6 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], }); - // Mock keychain get request const keychainGetNock = nock(bitgoApiUrl) .get(`/api/v2/${coin}/key/user-key-id`) .matchHeader('any', () => true) @@ -177,83 +126,37 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { pub: 'xpub_user', }); - // Mock sendAccountConsolidations with all failures - const sendConsolidationsStub = sinon - .stub(Wallet.prototype, 'sendAccountConsolidations') - .resolves({ - success: [], - failure: [ - { - error: 'All consolidations failed', - address: '0x1234567890abcdef', - }, - { - error: 'All consolidations failed', - address: '0xfedcba0987654321', - }, - ], - }); - - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - pubkey: 'xpub_user', - consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], - }); - - response.status.should.equal(500); - response.body.should.have.property('error'); - response.body.should.have.property('details').which.match(/All consolidations failed/); - - walletGetNock.done(); - keychainGetNock.done(); - sinon.assert.calledOnce(sendConsolidationsStub); - }); - - it('should throw error when coin does not support account consolidations', async () => { - // Mock wallet get request - const walletGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/wallet/${walletId}`) - .matchHeader('any', () => true) - .reply(200, { - id: walletId, - type: 'cold', - subType: 'onPrem', - keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], - }); - // Mock keychain get request - const keychainGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/key/user-key-id`) - .matchHeader('any', () => true) - .reply(200, { - id: 'user-key-id', - pub: 'xpub_user', - }); + const mockError = { + error: 'Internal Server Error', + name: 'ApiResponseError', + details: + 'There are too few unspents that meet the given parameters to consolidate (1 available).', + }; - // Mock allowsAccountConsolidations to return false - const allowsConsolidationsStub = sinon - .stub(Hteth.prototype, 'allowsAccountConsolidations') - .returns(false); + const consolidateUnspentsStub = sinon + .stub(Wallet.prototype, 'consolidateUnspents') + .throws(Object.assign(new Error(mockError.details), mockError)); const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .post(`/api/${coin}/wallet/${walletId}/consolidateunspents`) .set('Authorization', `Bearer ${accessToken}`) .send({ source: 'user', pubkey: 'xpub_user', + feeRate: 1000, }); response.status.should.equal(500); + response.body.should.have.property('error', mockError.error); + response.body.should.have.property('name', mockError.name); + response.body.should.have.property('details', mockError.details); walletGetNock.done(); keychainGetNock.done(); - sinon.assert.calledOnce(allowsConsolidationsStub); + sinon.assert.calledOnce(consolidateUnspentsStub); }); it('should throw error when provided pubkey does not match wallet keychain', async () => { - // Mock wallet get request const walletGetNock = nock(bitgoApiUrl) .get(`/api/v2/${coin}/wallet/${walletId}`) .matchHeader('any', () => true) @@ -264,7 +167,6 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], }); - // Mock keychain get request const keychainGetNock = nock(bitgoApiUrl) .get(`/api/v2/${coin}/key/user-key-id`) .matchHeader('any', () => true) @@ -274,11 +176,12 @@ describe('POST /api/:coin/wallet/:walletId/consolidate', () => { }); const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .post(`/api/${coin}/wallet/${walletId}/consolidateunspents`) .set('Authorization', `Bearer ${accessToken}`) .send({ source: 'user', pubkey: 'wrong_pubkey', + feeRate: 1000, }); response.status.should.equal(500); From 22ea917ac2b9f3406250870e4b1ca55cef2cf1c2 Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Fri, 11 Jul 2025 11:09:15 -0400 Subject: [PATCH 08/10] feat(mbe): complete integration test --- src/__tests__/api/master/consolidate.test.ts | 459 ++++++++++++++----- 1 file changed, 348 insertions(+), 111 deletions(-) diff --git a/src/__tests__/api/master/consolidate.test.ts b/src/__tests__/api/master/consolidate.test.ts index 3665c53e..408b4fac 100644 --- a/src/__tests__/api/master/consolidate.test.ts +++ b/src/__tests__/api/master/consolidate.test.ts @@ -5,6 +5,9 @@ import nock from 'nock'; import { app as expressApp } from '../../../masterExpressApp'; import { AppMode, MasterExpressConfig, TlsMode } from '../../../shared/types'; import { Environments, Wallet } from '@bitgo/sdk-core'; +import { Hteth } from '@bitgo/sdk-coin-eth'; +import * as transactionRequests from '../../../api/master/handlers/transactionRequests'; +import * as handlerUtils from '../../../api/master/handlerUtils'; describe('POST /api/:coin/wallet/:walletId/consolidateunspents', () => { let agent: request.SuperAgentTest; @@ -43,8 +46,9 @@ describe('POST /api/:coin/wallet/:walletId/consolidateunspents', () => { sinon.restore(); }); - it('should return transfer, txid, tx, and status on success', async () => { - const walletGetNock = nock(bitgoApiUrl) + // Helper functions to reduce duplication + const mockWalletGet = (multisigType: 'onchain' | 'tss') => { + return nock(bitgoApiUrl) .get(`/api/v2/${coin}/wallet/${walletId}`) .matchHeader('any', () => true) .reply(200, { @@ -52,141 +56,374 @@ describe('POST /api/:coin/wallet/:walletId/consolidateunspents', () => { type: 'cold', subType: 'onPrem', keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], + multisigType, }); + }; - const keychainGetNock = nock(bitgoApiUrl) + const mockKeychainGet = (commonKeychain?: string) => { + return nock(bitgoApiUrl) .get(`/api/v2/${coin}/key/user-key-id`) .matchHeader('any', () => true) .reply(200, { id: 'user-key-id', pub: 'xpub_user', + ...(commonKeychain && { commonKeychain }), }); + }; - const mockResult = { - transfer: { - entries: [ - { address: 'tb1qu...', value: -4000 }, - { address: 'tb1qle...', value: -4000 }, - { address: 'tb1qtw...', value: 2714, isChange: true }, + const mockTxRequest = (txRequestId: string) => { + return nock(bitgoApiUrl) + .get(`/api/v2/wallet/${walletId}/txrequests`) + .query({ txRequestIds: txRequestId, latest: 'true' }) + .matchHeader('any', () => true) + .reply(200, { + txRequests: [ + { + txRequestId, + version: 1, + latest: true, + state: 'pendingUserSignature', + transactions: [], + walletId: walletId, + walletType: 'cold', + date: new Date().toISOString(), + userId: 'test-user-id', + enterpriseId: 'test-enterprise-id', + intent: { intentType: 'payment' }, + txHashes: [], + policiesChecked: false, + unsignedTxs: [], + }, ], - id: '685ac2f3c2f8a2a5d9cc18d3593f1751', - coin: 'tbtc', - wallet: '685abbf19ca95b79f88e0b41d9337109', - txid: '239d143cdfc6d6c83a935da4f3d610b2364a956c7b6dcdc165eb706f62c4432a', - status: 'signed', - }, - txid: '239d143cdfc6d6c83a935da4f3d610b2364a956c7b6dcdc165eb706f62c4432a', - tx: '01000000000102580b...', - status: 'signed', - }; - - const consolidateUnspentsStub = sinon - .stub(Wallet.prototype, 'consolidateUnspents') - .resolves(mockResult); - - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidateunspents`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - pubkey: 'xpub_user', - feeRate: 1000, }); + }; - response.status.should.equal(200); - response.body.should.have.property('transfer'); - response.body.should.have.property('txid', mockResult.txid); - response.body.should.have.property('tx', mockResult.tx); - response.body.should.have.property('status', mockResult.status); - response.body.transfer.should.have.property('txid', mockResult.transfer.txid); - response.body.transfer.should.have.property('status', mockResult.transfer.status); - response.body.transfer.should.have.property('entries').which.is.Array(); - - walletGetNock.done(); - keychainGetNock.done(); - sinon.assert.calledOnce(consolidateUnspentsStub); + const createMultisigBuild = (index: number) => ({ + walletId, + txHex: `unsigned-tx-hex-${index}`, + txInfo: { unspents: [] }, + feeInfo: { fee: 1000 + index * 500 }, }); - it('should return error, name, and details on failure', async () => { - const walletGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/wallet/${walletId}`) - .matchHeader('any', () => true) - .reply(200, { - id: walletId, - type: 'cold', - subType: 'onPrem', - keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], - }); + const createMpcBuild = (index: number) => ({ + walletId, + txHex: `unsigned-mpc-tx-hex-${index}`, + txInfo: { unspents: [] }, + feeInfo: { fee: 2000 + index * 500 }, + txRequestId: `mpc-tx-request-${index}`, + }); - const keychainGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/key/user-key-id`) - .matchHeader('any', () => true) - .reply(200, { - id: 'user-key-id', - pub: 'xpub_user', - }); + describe('Multisig Wallets (onchain)', () => { + it('should consolidate multisig wallet addresses successfully', async () => { + // Mock wallet and keychain requests + const walletGetNock = mockWalletGet('onchain'); + const keychainGetNock = mockKeychainGet(); - const mockError = { - error: 'Internal Server Error', - name: 'ApiResponseError', - details: - 'There are too few unspents that meet the given parameters to consolidate (1 available).', - }; + // Mock buildAccountConsolidations + const buildConsolidationsStub = sinon + .stub(Wallet.prototype, 'buildAccountConsolidations') + .resolves([createMultisigBuild(1), createMultisigBuild(2)]); + + // Mock sendAccountConsolidation for multisig wallets + const sendAccountConsolidationStub = sinon + .stub(Wallet.prototype, 'sendAccountConsolidation') + .resolves({ + txid: 'consolidation-tx-1', + status: 'signed', + }); + + // Mock makeCustomSigningFunction + const makeCustomSigningFunctionStub = sinon + .stub(handlerUtils, 'makeCustomSigningFunction') + .returns(() => Promise.resolve({ txHex: 'signed-tx-hex' })); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + pubkey: 'xpub_user', + consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], + }); + + response.status.should.equal(200); + response.body.should.have.property('success'); + response.body.success.should.have.length(2); // Two successful builds + response.body.should.have.property('failure'); + response.body.failure.should.have.length(0); - const consolidateUnspentsStub = sinon - .stub(Wallet.prototype, 'consolidateUnspents') - .throws(Object.assign(new Error(mockError.details), mockError)); - - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidateunspents`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - pubkey: 'xpub_user', - feeRate: 1000, + walletGetNock.done(); + keychainGetNock.done(); + sinon.assert.calledOnce(buildConsolidationsStub); + sinon.assert.calledTwice(sendAccountConsolidationStub); // Called for each build + sinon.assert.calledTwice(makeCustomSigningFunctionStub); + }); + + it('should handle partial multisig consolidation failures', async () => { + // Mock wallet and keychain requests + const walletGetNock = mockWalletGet('onchain'); + const keychainGetNock = mockKeychainGet(); + + // Mock buildAccountConsolidations with multiple builds + const buildConsolidationsStub = sinon + .stub(Wallet.prototype, 'buildAccountConsolidations') + .resolves([createMultisigBuild(1), createMultisigBuild(2)]); + + // Mock sendAccountConsolidation - first succeeds, second fails + const sendAccountConsolidationStub = sinon.stub(Wallet.prototype, 'sendAccountConsolidation'); + sendAccountConsolidationStub.onFirstCall().resolves({ + txid: 'consolidation-tx-1', + status: 'signed', }); + sendAccountConsolidationStub.onSecondCall().rejects(new Error('Insufficient funds')); + + // Mock makeCustomSigningFunction + const makeCustomSigningFunctionStub = sinon + .stub(handlerUtils, 'makeCustomSigningFunction') + .returns(() => Promise.resolve({ txHex: 'signed-tx-hex' })); - response.status.should.equal(500); - response.body.should.have.property('error', mockError.error); - response.body.should.have.property('name', mockError.name); - response.body.should.have.property('details', mockError.details); + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + pubkey: 'xpub_user', + consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], + }); - walletGetNock.done(); - keychainGetNock.done(); - sinon.assert.calledOnce(consolidateUnspentsStub); + response.status.should.equal(500); + response.body.should.have.property('error', 'Internal Server Error'); + response.body.should.have + .property('details') + .which.match(/Consolidations failed: 1 and succeeded: 1/); + + walletGetNock.done(); + keychainGetNock.done(); + sinon.assert.calledOnce(buildConsolidationsStub); + sinon.assert.calledTwice(sendAccountConsolidationStub); + sinon.assert.calledTwice(makeCustomSigningFunctionStub); + }); + + it('should throw error when all multisig consolidations fail', async () => { + // Mock wallet and keychain requests + const walletGetNock = mockWalletGet('onchain'); + const keychainGetNock = mockKeychainGet(); + + // Mock buildAccountConsolidations with multiple builds + const buildConsolidationsStub = sinon + .stub(Wallet.prototype, 'buildAccountConsolidations') + .resolves([createMultisigBuild(1), createMultisigBuild(2)]); + + // Mock sendAccountConsolidation to always fail + const sendAccountConsolidationStub = sinon + .stub(Wallet.prototype, 'sendAccountConsolidation') + .rejects(new Error('All consolidations failed')); + + // Mock makeCustomSigningFunction + const makeCustomSigningFunctionStub = sinon + .stub(handlerUtils, 'makeCustomSigningFunction') + .returns(() => Promise.resolve({ txHex: 'signed-tx-hex' })); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + pubkey: 'xpub_user', + consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], + }); + + response.status.should.equal(500); + response.body.should.have.property('error'); + response.body.should.have.property('details').which.match(/All consolidations failed/); + + walletGetNock.done(); + keychainGetNock.done(); + sinon.assert.calledOnce(buildConsolidationsStub); + sinon.assert.calledTwice(sendAccountConsolidationStub); + sinon.assert.calledTwice(makeCustomSigningFunctionStub); + }); }); - it('should throw error when provided pubkey does not match wallet keychain', async () => { - const walletGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/wallet/${walletId}`) - .matchHeader('any', () => true) - .reply(200, { - id: walletId, - type: 'cold', - subType: 'onPrem', - keys: ['user-key-id', 'backup-key-id', 'bitgo-key-id'], - }); + describe('MPC Wallets (tss)', () => { + it('should consolidate MPC wallet using signAndSendTxRequests', async () => { + // Mock wallet and keychain requests for MPC wallet + const walletGetNock = mockWalletGet('tss'); + const keychainGetNock = mockKeychainGet('user-common-key'); - const keychainGetNock = nock(bitgoApiUrl) - .get(`/api/v2/${coin}/key/user-key-id`) - .matchHeader('any', () => true) - .reply(200, { - id: 'user-key-id', - pub: 'xpub_user', - }); + // Mock buildAccountConsolidations for MPC + const buildConsolidationsStub = sinon + .stub(Wallet.prototype, 'buildAccountConsolidations') + .resolves([createMpcBuild(1)]); + + // Mock the HTTP request for getTxRequest + const getTxRequestNock = mockTxRequest('mpc-tx-request-1'); + + // Mock signAndSendTxRequests for MPC wallets + const signAndSendTxRequestsStub = sinon + .stub(transactionRequests, 'signAndSendTxRequests') + .resolves({ + txid: 'mpc-consolidation-tx-1', + status: 'signed', + state: 'signed', + }); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + commonKeychain: 'user-common-key', + consolidateAddresses: ['0x1234567890abcdef'], + }); + + response.status.should.equal(200); + response.body.should.have.property('success'); + response.body.success.should.have.length(1); + response.body.success[0].should.have.property('txid', 'mpc-consolidation-tx-1'); + response.body.should.have.property('failure'); + response.body.failure.should.have.length(0); + + walletGetNock.done(); + keychainGetNock.done(); + getTxRequestNock.done(); + sinon.assert.calledOnce(buildConsolidationsStub); + sinon.assert.calledOnce(signAndSendTxRequestsStub); + + // Verify MPC-specific parameters + sinon.assert.calledWith(buildConsolidationsStub, sinon.match.hasNested('apiVersion', 'full')); + }); + + it('should handle partial MPC consolidation failures', async () => { + // Mock wallet and keychain requests for MPC wallet + const walletGetNock = mockWalletGet('tss'); + const keychainGetNock = mockKeychainGet('user-common-key'); + + // Mock buildAccountConsolidations with multiple builds for MPC + const buildConsolidationsStub = sinon + .stub(Wallet.prototype, 'buildAccountConsolidations') + .resolves([createMpcBuild(1), createMpcBuild(2)]); - const response = await agent - .post(`/api/${coin}/wallet/${walletId}/consolidateunspents`) - .set('Authorization', `Bearer ${accessToken}`) - .send({ - source: 'user', - pubkey: 'wrong_pubkey', - feeRate: 1000, + // Mock the HTTP requests for getTxRequest (both tx requests) + const getTxRequestNock1 = mockTxRequest('mpc-tx-request-1'); + const getTxRequestNock2 = mockTxRequest('mpc-tx-request-2'); + + // Mock signAndSendTxRequests - first succeeds, second fails + const signAndSendTxRequestsStub = sinon.stub(transactionRequests, 'signAndSendTxRequests'); + signAndSendTxRequestsStub.onFirstCall().resolves({ + txid: 'mpc-consolidation-tx-1', + status: 'signed', + state: 'signed', }); + signAndSendTxRequestsStub.onSecondCall().rejects(new Error('MPC signing failed')); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + commonKeychain: 'user-common-key', + consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], + }); + + response.status.should.equal(500); + response.body.should.have.property('error', 'Internal Server Error'); + response.body.should.have + .property('details') + .which.match(/Consolidations failed: 1 and succeeded: 1/); + + walletGetNock.done(); + keychainGetNock.done(); + getTxRequestNock1.done(); + getTxRequestNock2.done(); + sinon.assert.calledOnce(buildConsolidationsStub); + sinon.assert.calledTwice(signAndSendTxRequestsStub); + }); + + it('should throw error when all MPC consolidations fail', async () => { + // Mock wallet and keychain requests for MPC wallet + const walletGetNock = mockWalletGet('tss'); + const keychainGetNock = mockKeychainGet('user-common-key'); + + // Mock buildAccountConsolidations with multiple builds for MPC + const buildConsolidationsStub = sinon + .stub(Wallet.prototype, 'buildAccountConsolidations') + .resolves([createMpcBuild(1), createMpcBuild(2)]); + + // Mock the HTTP requests for getTxRequest (both tx requests) + const getTxRequestNock1 = mockTxRequest('mpc-tx-request-1'); + const getTxRequestNock2 = mockTxRequest('mpc-tx-request-2'); + + // Mock signAndSendTxRequests to always fail for MPC + const signAndSendTxRequestsStub = sinon + .stub(transactionRequests, 'signAndSendTxRequests') + .rejects(new Error('All MPC consolidations failed')); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + commonKeychain: 'user-common-key', + consolidateAddresses: ['0x1234567890abcdef', '0xfedcba0987654321'], + }); + + response.status.should.equal(500); + response.body.should.have.property('error'); + response.body.should.have.property('details').which.match(/All consolidations failed/); + + walletGetNock.done(); + keychainGetNock.done(); + getTxRequestNock1.done(); + getTxRequestNock2.done(); + sinon.assert.calledOnce(buildConsolidationsStub); + sinon.assert.calledTwice(signAndSendTxRequestsStub); + }); + }); + + describe('Common Error Cases', () => { + it('should throw error when coin does not support account consolidations', async () => { + // Mock wallet and keychain requests + const walletGetNock = mockWalletGet('onchain'); + const keychainGetNock = mockKeychainGet(); + + // Mock allowsAccountConsolidations to return false + const allowsConsolidationsStub = sinon + .stub(Hteth.prototype, 'allowsAccountConsolidations') + .returns(false); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + pubkey: 'xpub_user', + }); + + response.status.should.equal(500); + + walletGetNock.done(); + keychainGetNock.done(); + sinon.assert.calledOnce(allowsConsolidationsStub); + }); + + it('should throw error when provided pubkey does not match wallet keychain', async () => { + // Mock wallet and keychain requests + const walletGetNock = mockWalletGet('onchain'); + const keychainGetNock = mockKeychainGet(); + + const response = await agent + .post(`/api/${coin}/wallet/${walletId}/consolidate`) + .set('Authorization', `Bearer ${accessToken}`) + .send({ + source: 'user', + pubkey: 'wrong_pubkey', + }); - response.status.should.equal(500); + response.status.should.equal(500); - walletGetNock.done(); - keychainGetNock.done(); + walletGetNock.done(); + keychainGetNock.done(); + }); }); }); From 177838ea4a4fbdccd590fef20c888b76c6091e09 Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Fri, 11 Jul 2025 13:34:56 -0400 Subject: [PATCH 09/10] feat(mbe): fix testing --- src/__tests__/api/master/consolidate.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/__tests__/api/master/consolidate.test.ts b/src/__tests__/api/master/consolidate.test.ts index 408b4fac..2e983b7a 100644 --- a/src/__tests__/api/master/consolidate.test.ts +++ b/src/__tests__/api/master/consolidate.test.ts @@ -11,7 +11,7 @@ import * as handlerUtils from '../../../api/master/handlerUtils'; describe('POST /api/:coin/wallet/:walletId/consolidateunspents', () => { let agent: request.SuperAgentTest; - const coin = 'btc'; + const coin = 'hteth'; const walletId = 'test-wallet-id'; const accessToken = 'test-access-token'; const bitgoApiUrl = Environments.test.uri; From 71634e3281a6439c7b8bda1d6001a30792b8a75e Mon Sep 17 00:00:00 2001 From: Cesar Patino Date: Mon, 14 Jul 2025 13:16:34 -0400 Subject: [PATCH 10/10] feat(mbe) fix coment --- src/api/master/handlers/handleConsolidate.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/api/master/handlers/handleConsolidate.ts b/src/api/master/handlers/handleConsolidate.ts index 611792f7..bd4f9311 100644 --- a/src/api/master/handlers/handleConsolidate.ts +++ b/src/api/master/handlers/handleConsolidate.ts @@ -38,7 +38,7 @@ export async function handleConsolidate( throw new Error('consolidateAddresses must be an array of addresses'); } - const isMPC = wallet._wallet.multisigType === 'tss'; + const isMPC = wallet.multisigType() === 'tss'; try { const consolidationParams: BuildConsolidationTransactionOptions = { @@ -91,7 +91,7 @@ export async function handleConsolidate( successfulTxs.push(result); } catch (e) { - console.dir(e); + logger.error('Error during account consolidation: %s', (e as Error).message, e); failedTxs.push(e as any); } } @@ -107,7 +107,7 @@ export async function handleConsolidate( msg = `Consolidations failed: ${failedTxs.length} and succeeded: ${successfulTxs.length}`; } else { // All failed - status = 400; + status = 500; msg = 'All consolidations failed'; }