Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
372 changes: 8 additions & 364 deletions modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -484,53 +484,20 @@ describe('signTxRequest:', function () {
});
});

describe('consolidate intent', function () {
// Intent recipient amount is a build-time snapshot that drifts from the swept balance in the tx.
it('does not throw for allowlisted no-recipient intentType (consolidate)', async function () {
sandbox.stub(baseCoin, 'verifyTransaction').resolves(true);
const nockPromises = await getNockPromisesForEddsaSigning(txRequest);
await Promise.all(nockPromises);

const consolidateTxRequest: TxRequest = {
...txRequest,
intent: {
intentType: 'consolidate',
consolidateId: '68a7d5d0c66e74e216b97173bd558c6d',
recipients: [
{
address: { address: 'HMEgbR4S2hLKfst2VZUVpHVUu4FioFPyW5iUuJvZdMvs' },
amount: { value: '999985000', symbol: 'sol' },
},
],
},
intent: { intentType: 'consolidate' } as any,
};

it('verifies sweep-to-root instead of snapshot intent recipients', async function () {
// Fee payer stays the original root, so this also covers the consolidation fee payer exemption.
sandbox
.stub(wallet, 'coinSpecific')
.returns({ rootAddress: 'HMEgbR4S2hLKfst2VZUVpHVUu4FioFPyW5iUuJvZdMvs', customChangeWalletId: '' });
const verifySpy = sandbox.spy(baseCoin, 'verifyTransaction');
const nockPromises = await getNockPromisesForEddsaSigning(consolidateTxRequest);
await Promise.all(nockPromises);

await tssUtils.signTxRequest({
txRequest: consolidateTxRequest,
prv: Buffer.from(userKeyShare).toString('base64'),
reqId,
});

verifySpy.calledOnce.should.be.true();
verifySpy.firstCall.args[0].should.containDeep({
txPrebuild: { consolidateId: '68a7d5d0c66e74e216b97173bd558c6d' },
verification: { consolidationToBaseAddress: true },
});
verifySpy.firstCall.args[0].txParams.should.not.have.property('recipients');
});

it('rejects a consolidation that does not sweep to the wallet root address', async function () {
await tssUtils
.signTxRequest({
txRequest: consolidateTxRequest,
prv: Buffer.from(userKeyShare).toString('base64'),
reqId,
})
.should.be.rejectedWith('tx outputs does not match with expected address');
const userPrvBase64 = Buffer.from(userKeyShare).toString('base64');
await tssUtils.signTxRequest({
txRequest: consolidateTxRequest,
prv: userPrvBase64,
reqId,
});
});

Expand Down
23 changes: 1 addition & 22 deletions modules/bitgo/test/v2/unit/signTransactionVerification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,7 @@ import 'should';
import { BitGoAPI } from '@bitgo/sdk-api';
import { TestBitGo } from '@bitgo/sdk-test';
import { Tbtc } from '@bitgo/sdk-coin-btc';
import {
common,
BaseCoin,
BitGoBase,
InvalidTransactionError,
Wallet,
WalletSignTransactionOptions,
} from '@bitgo/sdk-core';
import { common, BaseCoin, BitGoBase, Wallet, WalletSignTransactionOptions } from '@bitgo/sdk-core';

describe('Wallet signTransaction with verifyTxParams', function () {
let wallet: Wallet;
Expand Down Expand Up @@ -158,18 +151,4 @@ describe('Wallet signTransaction with verifyTxParams', function () {
assert.strictEqual(verifyParams.txPrebuild.txHex, 'mock-tx-hex');
assert.deepStrictEqual(verifyParams.txParams, verifyTxParams.txParams);
});

it('should throw when verifyTxParams is provided without txHex or TSS txRequestId', async function () {
const signParams: WalletSignTransactionOptions = {
txPrebuild: {},
verifyTxParams: {
txParams: {
recipients: [{ address: 'test-address', amount: '1000' }],
},
},
};

await wallet.signTransaction(signParams).should.be.rejectedWith(InvalidTransactionError);
sinon.assert.notCalled(verifyTransactionStub);
});
});
6 changes: 2 additions & 4 deletions modules/sdk-core/src/bitgo/utils/tss/baseTSSUtils.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { EncryptionVersion, IRequestTracer } from '../../../api';
import * as openpgp from 'openpgp';
import { Key, readKey, SerializedKeyPair } from 'openpgp';
import { IBaseCoin, KeychainsTriplet, TransactionParams } from '../../baseCoin';
import { IBaseCoin, KeychainsTriplet } from '../../baseCoin';
import { BitGoBase } from '../../bitgoBase';
import { Keychain, KeyIndices, WebauthnKeyEncryptionInfo } from '../../keychain';
import { getTxRequest } from '../../tss';
Expand Down Expand Up @@ -269,9 +269,7 @@ export default class BaseTssUtils<KeyShare> extends MpcUtils implements ITssUtil
txRequest: string | TxRequest,
externalSignerCommitmentGenerator: CustomCommitmentGeneratingFunction,
externalSignerRShareGenerator: CustomRShareGeneratingFunction,
externalSignerGShareGenerator: CustomGShareGeneratingFunction,
_reqId?: IRequestTracer,
_txParams?: TransactionParams
externalSignerGShareGenerator: CustomGShareGeneratingFunction
): Promise<TxRequest> {
throw new Error('Method not implemented.');
}
Expand Down
4 changes: 1 addition & 3 deletions modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -934,9 +934,7 @@ export interface ITssUtils<KeyShare = EDDSA.KeyShare> {
txRequest: string | TxRequest,
externalSignerCommitmentGenerator: CustomCommitmentGeneratingFunction,
externalSignerRShareGenerator: CustomRShareGeneratingFunction,
externalSignerGShareGenerator: CustomGShareGeneratingFunction,
reqId?: IRequestTracer,
txParams?: TransactionParams
externalSignerGShareGenerator: CustomGShareGeneratingFunction
): Promise<TxRequest>;
signEcdsaTssUsingExternalSigner(
params: TSSParams | TSSParamsForMessage,
Expand Down
32 changes: 2 additions & 30 deletions modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,14 @@ import { InvalidTransactionError } from '../../../errors';
import { CreateEddsaBitGoKeychainParams, CreateEddsaKeychainParams, KeyShare, YShare } from './types';
import baseTSSUtils from '../baseTSSUtils';
import { BaseEddsaUtils } from './base';
import { KeychainsTriplet, TransactionParams } from '../../../baseCoin';
import { KeychainsTriplet } from '../../../baseCoin';
import { exchangeEddsaCommitments } from '../../../tss/common';
import { Ed25519Bip32HdTree } from '@bitgo/sdk-lib-mpc';
import { EncryptionVersion, IRequestTracer } from '../../../../api';
import { envRequiresBitgoPubGpgKeyConfig, getBitgoMpcGpgPubKey, isBitgoMpcPubKey } from '../../../tss/bitgoPubKeys';
import { EnvironmentName } from '../../../environments';
import { readKey } from 'openpgp';
import type { EddsaKeyGenCallbacks } from '../../../wallet/iWallets';
import { resolveTssVerifyTransactionOptions } from '../recipientUtils';

/**
* Utility functions for TSS work flows.
Expand Down Expand Up @@ -658,8 +657,7 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
externalSignerCommitmentGenerator: CustomCommitmentGeneratingFunction,
externalSignerRShareGenerator: CustomRShareGeneratingFunction,
externalSignerGShareGenerator: CustomGShareGeneratingFunction,
reqId?: IRequestTracer,
txParams?: TransactionParams
reqId?: IRequestTracer
): Promise<TxRequest> {
let txRequestResolved: TxRequest;
let txRequestId: string;
Expand All @@ -671,8 +669,6 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
txRequestId = txRequest.txRequestId;
}

await this.verifyEdDsaTxRequestBeforeSigning(txRequestResolved, txParams);

const { apiVersion } = txRequestResolved;
const bitgoGpgKey = await this.pickBitgoPubGpgKeyForSigning(false, reqId, txRequestResolved.enterpriseId);

Expand Down Expand Up @@ -770,8 +766,6 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
);
unsignedTx =
apiVersion === 'full' ? txRequestResolved.transactions![0].unsignedTx : txRequestResolved.unsignedTxs[0];
const txParams = 'txParams' in params ? params.txParams : undefined;
await this.verifyEdDsaTxRequestBeforeSigning(txRequestResolved, txParams);
} else if (requestType === RequestType.message) {
assert(txRequestResolved.messages?.length, 'Unable to find messages in txRequest for message signing');
const message = txRequestResolved.messages[0];
Expand Down Expand Up @@ -878,28 +872,6 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
return BaseEddsaUtils.getPublicKeyFromCommonKeychain(commonKeychain);
}

private async verifyEdDsaTxRequestBeforeSigning(
txRequestResolved: TxRequest,
txParams?: TransactionParams
): Promise<void> {
assert(txRequestResolved.transactions || txRequestResolved.unsignedTxs, 'Unable to find transactions in txRequest');
const unsignedTx =
txRequestResolved.apiVersion === 'full'
? txRequestResolved.transactions![0].unsignedTx
: txRequestResolved.unsignedTxs[0];
assert(unsignedTx.signableHex, 'Missing signableHex in unsignedTx');
await this.baseCoin.verifyTransaction({
...resolveTssVerifyTransactionOptions(
txRequestResolved,
unsignedTx.serializedTxHex ?? unsignedTx.signableHex,
txParams,
this.baseCoin.getChain()
),
wallet: this.wallet,
walletType: this.wallet.multisigType(),
});
}

createUserToBitgoCommitmentShare(commitment: string): CommitmentShareRecord {
return {
from: SignatureShareType.USER,
Expand Down
10 changes: 3 additions & 7 deletions modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import {
import { EncryptionVersion } from '../../../../api';
import { BitGoBase } from '../../../bitgoBase';
import { BaseEddsaUtils } from './base';
import { resolveTssVerifyTransactionOptions } from '../recipientUtils';
import { resolveEffectiveTxParams } from '../recipientUtils';
import { EddsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './eddsaMPCv2KeyGenSender';
import { EddsaMPCv2RecoveryKeyShares } from './types';
import { parseMpcV2KeyShareEnvelope } from '../keyShareEnvelope';
Expand Down Expand Up @@ -598,12 +598,8 @@ export class EddsaMPCv2Utils extends BaseEddsaUtils {
derivationPath = unsignedTx.derivationPath;
bufferContent = Buffer.from(txOrMessageToSign, 'hex');
await this.baseCoin.verifyTransaction({
...resolveTssVerifyTransactionOptions(
txRequest,
unsignedTx.serializedTxHex ?? txOrMessageToSign,
params.txParams,
this.baseCoin.getChain()
),
txPrebuild: { txHex: unsignedTx.serializedTxHex ?? txOrMessageToSign },
txParams: resolveEffectiveTxParams(txRequest, params.txParams, this.baseCoin.getChain()),
wallet: this.wallet,
walletType: this.wallet.multisigType(),
});
Expand Down
36 changes: 1 addition & 35 deletions modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as t from 'io-ts';
import { TransactionParams, VerifyTransactionOptions } from '../../baseCoin';
import { TransactionParams } from '../../baseCoin';
import { InvalidTransactionError } from '../../errors';
import { PopulatedIntent, TxRequest } from './baseTypes';

Expand Down Expand Up @@ -191,36 +190,3 @@ export function resolveEffectiveTxParams(

return effectiveTxParams;
}

const ConsolidateIntent = t.intersection([
t.type({ intentType: t.literal('consolidate') }),
t.partial({ consolidateId: t.string }),
]);

/**
* Resolves the verifyTransaction options for signing-time verification of a TSS txRequest.
*
* Consolidation intent recipients are a server-generated, build-time balance snapshot that drifts
* from the swept amount, so they are not backfilled; sweep-to-base-address is verified instead,
* mirroring wallet.sendAccountConsolidations.
*
* @param txRequest - the transaction request containing the persisted intent
* @param txHex - the unsigned transaction to verify
* @param txParams - the caller-supplied transaction parameters (may be undefined)
* @param chainName - the base chain name; pass baseCoin.getChain()
*/
export function resolveTssVerifyTransactionOptions(
txRequest: TxRequest,
txHex: string,
txParams: TransactionParams | undefined,
chainName: string
): Pick<VerifyTransactionOptions, 'txPrebuild' | 'txParams' | 'verification'> {
if (ConsolidateIntent.is(txRequest.intent)) {
return {
txPrebuild: { txHex, consolidateId: txRequest.intent.consolidateId },
txParams: { ...txParams },
verification: { consolidationToBaseAddress: true },
};
}
return { txPrebuild: { txHex }, txParams: resolveEffectiveTxParams(txRequest, txParams, chainName) };
}
2 changes: 0 additions & 2 deletions modules/sdk-core/src/bitgo/wallet/iWallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,6 @@ export interface WalletSignTransactionOptions extends WalletSignBaseOptions {
txParams: TransactionParams;
verification?: VerificationOptions;
};
/** Populated by wallet.verifyTxParams TSS path so signing uses the same txRequest that was verified. */
resolvedTxRequestForSigning?: TxRequest;
[index: string]: unknown;
}

Expand Down
54 changes: 13 additions & 41 deletions modules/sdk-core/src/bitgo/wallet/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import { getSharedSecret } from '../ecdh';
import {
AddressGenerationError,
IncorrectPasswordError,
InvalidTransactionError,
MethodNotImplementedError,
MissingEncryptedKeychainError,
NeedUserSignupError,
Expand Down Expand Up @@ -57,7 +56,6 @@ import { decodeWithCodec } from '../utils/codecs';
import { postWithCodec } from '../utils/postWithCodec';
import { EcdsaMPCv2Utils, EcdsaUtils } from '../utils/tss/ecdsa';
import EddsaUtils, { EddsaMPCv2Utils } from '../utils/tss/eddsa';
import { resolveEffectiveTxParams } from '../utils/tss/recipientUtils';
import { RedpallasMPCv2Utils } from '../utils/tss/redpallas';
import { getTxRequestApiVersion, validateTxRequestApiVersion } from '../utils/txRequest';
import { buildParamKeys, BuildParams } from './BuildParams';
Expand Down Expand Up @@ -2388,41 +2386,18 @@ export class Wallet implements IWallet {
params.txPrebuild = { txRequestId };
}

// Verify transaction if verifyTxParams is provided (fail closed — never skip silently).
if (params.verifyTxParams) {
const prebuild = params.txPrebuild;
if (prebuild?.txHex) {
const verifyParams = {
txPrebuild: { ...prebuild },
txParams: params.verifyTxParams.txParams,
wallet: this,
verification: params.verifyTxParams.verification,
reqId: params.reqId,
walletType: this.multisigType(),
};
// Verify transaction if verifyTxParams is provided
if (params.verifyTxParams && txPrebuild?.txHex) {
const verifyParams = {
txPrebuild: { ...txPrebuild },
txParams: params.verifyTxParams.txParams,
wallet: this as IWallet,
verification: params.verifyTxParams.verification,
reqId: params.reqId,
walletType: this.multisigType() as 'onchain' | 'tss',
};

await this.baseCoin.verifyTransaction(verifyParams);
} else if (this.multisigType() === 'tss' && prebuild?.txRequestId && typeof prebuild.txRequestId === 'string') {
const txRequest = await getTxRequest(this.bitgo, this.id(), prebuild.txRequestId, params.reqId);
assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
const unsignedTx =
txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs![0];
assert(unsignedTx.signableHex, 'Missing signableHex in unsignedTx');
await this.baseCoin.verifyTransaction({
txPrebuild: { txHex: unsignedTx.serializedTxHex ?? unsignedTx.signableHex },
txParams: resolveEffectiveTxParams(txRequest, params.verifyTxParams.txParams, this.baseCoin.getChain()),
wallet: this,
verification: params.verifyTxParams.verification,
reqId: params.reqId,
walletType: this.multisigType(),
});
// Sign the same resolved txRequest (avoid TOCTOU re-fetch before signing).
params.resolvedTxRequestForSigning = txRequest;
} else {
throw new InvalidTransactionError(
'verifyTxParams was provided but txPrebuild does not include txHex or a TSS txRequestId.'
);
}
await this.baseCoin.verifyTransaction(verifyParams);
}

if (
Expand Down Expand Up @@ -5039,16 +5014,13 @@ export class Wallet implements IWallet {
const reqId = params.reqId || undefined;
await this.tssUtils.deleteSignatureShares(txRequestId, reqId);

const txParams = params.verifyTxParams?.txParams ?? params.txPrebuild?.buildParams;

try {
return await this.tssUtils.signEddsaTssUsingExternalSigner(
txRequestId,
params.customCommitmentGeneratingFunction,
params.customRShareGeneratingFunction,
params.customGShareGeneratingFunction,
reqId,
txParams
reqId
);
} catch (e) {
debug('failed to sign transaction %O', e);
Expand Down Expand Up @@ -5288,7 +5260,7 @@ export class Wallet implements IWallet {
throw new Error('prv required to sign transactions with TSS');
}

const txRequest: string | TxRequest = params.resolvedTxRequestForSigning ?? params.txPrebuild.txRequestId;
const txRequest: string | TxRequest = params.txPrebuild.txRequestId;
const txParams: TransactionParams | undefined = params.txPrebuild.buildParams;

try {
Expand Down
Loading
Loading