Skip to content

fix: call verifytransaction coin method after fetching txrequest from… #6014

New issue

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

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

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
17 changes: 10 additions & 7 deletions modules/bitgo/test/v2/unit/internal/tssUtils/eddsa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,12 @@ describe('TSS Utils:', async function () {
},
};

const validTxHex =
'020104096f306776291ac74cd6da1787f7b5de1f103b92d21c240d30f608f974cadb33a8051a34bc8acd438763976f96876115050f73828553566d111d7ac8bffebf587c40a0c8fc599f0defd59b130ea61bf75f441683124b8f74ec9bd3ab44f5b0c2df4f5f5987bfe26aa66013efd96d36360f2b4336c91f993259fb56051305614d429b3d6e69f97bc63846ba524986cc3a04d006898dee0c8e72a0dbbb95edaf9ca800000000000000000000000000000000000000000000000000000000000000008001d74c358d532e3cb5a42cbcb4b6d15c7d93165375427b5ef1a092234b74ff06a7d517192c568ee08a845f73d29788cf035c3145b21ab344d8062ea940000006ddf6e1d765a193d9cbe146ceeb79ac1cb485ed5f5b37913a8cf5857eff00a90549d7ae1e24cd8bb16e36e0519c66fd8b7c5223d660a46a6e0fed7a5ab262be02050303070104040000000804040602000a0c009435770000000009';

const txRequest = {
txRequestId: 'randomId',
unsignedTxs: [{ signableHex: 'MPC on a Friday night', serializedTxHex: 'MPC on a Friday night' }],
unsignedTxs: [{ signableHex: validTxHex, serializedTxHex: validTxHex }],
signatureShares: [
{
from: 'bitgo',
Expand Down Expand Up @@ -554,9 +557,9 @@ describe('TSS Utils:', async function () {
transactions: [],
unsignedTxs: [
{
serializedTxHex: 'MPC on a Friday night',
signableHex: 'MPC on a Friday night',
derivationPath: 'm/0',
serializedTxHex: validTxHex,
signableHex: validTxHex,
derivationPath: 'm/4',
},
],
date: new Date().toISOString(),
Expand Down Expand Up @@ -643,9 +646,9 @@ describe('TSS Utils:', async function () {
transactions: [],
unsignedTxs: [
{
serializedTxHex: 'MPC on a Friday night',
signableHex: 'MPC on a Friday night',
derivationPath: 'm/0',
serializedTxHex: validTxHex,
signableHex: validTxHex,
derivationPath: 'm/4',
},
],
date: new Date().toISOString(),
Expand Down
39 changes: 39 additions & 0 deletions modules/sdk-coin-algo/src/algo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,45 @@ export class Algo extends BaseCoin {
}

async verifyTransaction(params: VerifyTransactionOptions): Promise<boolean> {
const { txPrebuild, txParams } = params;

// Get raw transaction from either txHex or txRequest
const rawTx =
txPrebuild.txHex ||
(txPrebuild.txRequest?.apiVersion === 'full'
? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex
: txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex);

if (!rawTx) {
throw new Error('missing required transaction hex');
}

const decodedTx = this.decodeTx(Buffer.from(rawTx, 'hex'));

// Verify recipients if provided
if (txParams?.recipients) {
const recipients = txParams.recipients;
const txAmount = (decodedTx as any).amount;
const txToAddress = (decodedTx as any).to;

// For Algorand, we currently only support single recipient transactions
if (recipients.length !== 1) {
throw new Error('Algorand only supports single recipient transactions');
}

const recipient = recipients[0];

// Verify recipient address matches
if (recipient.address !== txToAddress) {
throw new Error('transaction recipient address does not match expected address');
}

// Verify amount matches
if (new BigNumber(recipient.amount).isEqualTo(txAmount)) {
throw new Error('transaction amount does not match expected amount');
}
}

return true;
}

Expand Down
37 changes: 35 additions & 2 deletions modules/sdk-coin-dot/src/dot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,12 +646,45 @@ export class Dot extends BaseCoin {
}

async verifyTransaction(params: VerifyTransactionOptions): Promise<boolean> {
const { txParams } = params;
if (Array.isArray(txParams.recipients) && txParams.recipients.length > 1) {
const { txPrebuild, txParams } = params;

// Verify single recipient constraint
if (Array.isArray(txParams?.recipients) && txParams.recipients.length > 1) {
throw new Error(
`${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.`
);
}

// Get raw transaction from either txHex or txRequest
const rawTx =
txPrebuild.txHex ||
(txPrebuild.txRequest?.apiVersion === 'full'
? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex
: txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex);

if (!rawTx) {
throw new Error('missing required transaction hex');
}

// Verify recipient and amount if provided
if (txParams?.recipients?.[0]) {
const recipient = txParams.recipients[0];
const txBuilder = this.getBuilder();
const transaction = txBuilder.from(rawTx);
const decodedTx = await transaction.build();
const explainedTx = decodedTx.explainTransaction();

// Verify recipient address matches
if (recipient.address !== explainedTx.to) {
throw new Error('transaction recipient address does not match expected address');
}

// Verify amount matches
if (!new BigNumber(recipient.amount).isEqualTo(explainedTx.amount)) {
throw new Error('transaction amount does not match expected amount');
}
}

return true;
}

Expand Down
38 changes: 36 additions & 2 deletions modules/sdk-coin-stx/src/stx.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,46 @@ export class Stx extends BaseCoin {
}

async verifyTransaction(params: VerifyTransactionOptions): Promise<boolean> {
const { txParams } = params;
if (Array.isArray(txParams.recipients) && txParams.recipients.length > 1) {
const { txPrebuild, txParams } = params;

// Verify single recipient constraint
if (Array.isArray(txParams?.recipients) && txParams.recipients.length > 1) {
throw new Error(
`${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.`
);
}

// Get raw transaction from either txHex or txRequest
const rawTx =
txPrebuild.txHex ||
(txPrebuild.txRequest?.apiVersion === 'full'
? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex
: txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex);

if (!rawTx) {
throw new Error('missing required transaction hex');
}

// Verify recipient and amount if provided
if (txParams?.recipients?.[0]) {
const recipient = txParams.recipients[0];
const coinConfig = coins.get(this.getChain());
const txBuilder = new TransactionBuilderFactory(coinConfig).getTransferBuilder();
txBuilder.from(rawTx);
const transaction = await txBuilder.build();
const explainedTx = transaction.explainTransaction();

// Verify recipient address matches
if (recipient.address !== explainedTx.to) {
throw new Error('transaction recipient address does not match expected address');
}

// Verify amount matches
if (!new BigNumber(recipient.amount).isEqualTo(explainedTx.amount)) {
throw new Error('transaction amount does not match expected amount');
}
}

return true;
}

Expand Down
37 changes: 35 additions & 2 deletions modules/sdk-coin-xtz/src/xtz.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,45 @@ export class Xtz extends BaseCoin {
}

async verifyTransaction(params: VerifyTransactionOptions): Promise<boolean> {
const { txParams } = params;
if (Array.isArray(txParams.recipients) && txParams.recipients.length > 1) {
const { txPrebuild, txParams } = params;

// Verify single recipient constraint
if (Array.isArray(txParams?.recipients) && txParams.recipients.length > 1) {
throw new Error(
`${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.`
);
}

// Get raw transaction from either txHex or txRequest
const rawTx =
txPrebuild.txHex ||
(txPrebuild.txRequest?.apiVersion === 'full'
? txPrebuild.txRequest.transactions?.[0]?.unsignedTx?.signableHex
: txPrebuild.txRequest?.unsignedTxs?.[0]?.signableHex);

if (!rawTx) {
throw new Error('missing required transaction hex');
}

// Verify recipient and amount if provided
if (txParams?.recipients?.[0]) {
const recipient = txParams.recipients[0];
const txBuilder = new TransactionBuilder(coins.get(this.getChain()));
txBuilder.from(rawTx);
const transaction = await txBuilder.build();
const explainedTx = transaction.explainTransaction();

// Verify recipient address matches
if (recipient.address !== explainedTx.to) {
throw new Error('transaction recipient address does not match expected address');
}

// Verify amount matches
if (!new BigNumber(recipient.amount).isEqualTo(explainedTx.amount)) {
throw new Error('transaction amount does not match expected amount');
}
}

return true;
}

Expand Down
1 change: 1 addition & 0 deletions modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ export interface TransactionPrebuild extends BaseSignable {
txBase64?: string;
txHex?: string;
txInfo?: unknown;
txRequest?: TxRequest;
}

export interface Message extends BaseSignable {
Expand Down
34 changes: 33 additions & 1 deletion modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
CustomRShareGeneratingFunction,
EncryptedSignerShareRecord,
EncryptedSignerShareType,
PopulatedIntent,
SignatureShareRecord,
SignatureShareType,
TSSParamsWithPrv,
Expand All @@ -37,6 +38,7 @@ import { KeychainsTriplet } from '../../../baseCoin';
import { exchangeEddsaCommitments } from '../../../tss/common';
import { Ed25519Bip32HdTree } from '@bitgo/sdk-lib-mpc';
import { IRequestTracer } from '../../../../api';
import { Wallet } from '../../../wallet';

/**
* Utility functions for TSS work flows.
Expand Down Expand Up @@ -574,6 +576,31 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
txRequestId = txRequest.txRequestId;
}

// Verify the transaction before signing
const isVerified = await this.baseCoin.verifyTransaction({
txParams: {
recipients: (txRequestResolved.intent as PopulatedIntent)?.recipients?.map((r) => ({
address: r.address.address,
amount: r.amount.value.toString(),
})),
},
txPrebuild: {
txHex:
txRequestResolved.apiVersion === 'full'
? txRequestResolved.transactions?.[0]?.unsignedTx?.signableHex
: txRequestResolved.unsignedTxs[0]?.signableHex,
txInfo:
txRequestResolved.apiVersion === 'full'
? txRequestResolved.transactions?.[0]?.unsignedTx
: txRequestResolved.unsignedTxs[0],
},
wallet: this.wallet,
});

if (!isVerified) {
throw new Error('Transaction verification failed');
}

const hdTree = await Ed25519Bip32HdTree.initialize();
const MPC = await Eddsa.initialize(hdTree);

Expand All @@ -586,7 +613,12 @@ export class EddsaUtils extends baseTSSUtils<KeyShare> {
assert(txRequestResolved.transactions || txRequestResolved.unsignedTxs, 'Unable to find transactions in txRequest');
const unsignedTx =
apiVersion === 'full' ? txRequestResolved.transactions![0].unsignedTx : txRequestResolved.unsignedTxs[0];

await this.baseCoin.verifyTransaction({
txPrebuild: { txHex: unsignedTx.signableHex },
wallet: this.wallet as unknown as Wallet,
txParams: (params as any).txParams || { recipients: [] },
walletType: this.wallet.multisigType(),
});
const signingKey = MPC.keyDerive(
userSigningMaterial.uShare,
[userSigningMaterial.bitgoYShare, userSigningMaterial.backupYShare],
Expand Down
Loading