Skip to content

Commit 465f4a2

Browse files
authored
Merge pull request #6990 from BitGo/validate-data-on-tx
fix(bitgo): add data validation on tx Ticket: SC-1436
2 parents 1c5fd30 + 98ed8b5 commit 465f4a2

File tree

2 files changed

+93
-9
lines changed

2 files changed

+93
-9
lines changed

modules/bitgo/test/v2/fixtures/staking/stakingWallet.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,42 @@ export default {
7777
},
7878
senderWalletId: 'btcDescriptorWalletId',
7979
},
80+
81+
matchingDataBuildParams: {
82+
memo: 'matching-memo',
83+
gasLimit: 200000,
84+
type: 'pay',
85+
solInstructions: [{ programId: 'Stake111', keys: [], data: 'delegate' }],
86+
aptosCustomTransactionParams: { moduleName: '0x1::staking', functionName: 'stake' },
87+
},
88+
89+
mismatchMemoBuildParams: {
90+
memo: 'user-memo',
91+
},
92+
93+
mismatchGasLimitBuildParams: {
94+
gasLimit: 100000,
95+
},
96+
97+
mismatchTypeBuildParams: {
98+
type: 'stake',
99+
},
100+
101+
mismatchSolInstructionsBuildParams: {
102+
solInstructions: [
103+
{
104+
programId: 'UserStakeProgram1111111111111111111111111111',
105+
keys: [{ pubkey: 'user-key', isSigner: true, isWritable: true }],
106+
data: 'user-delegate-data',
107+
},
108+
],
109+
},
110+
111+
mismatchAptosParamsBuildParams: {
112+
aptosCustomTransactionParams: {
113+
moduleName: '0x1::staking',
114+
functionName: 'stake',
115+
functionArguments: ['10000'],
116+
},
117+
},
80118
};

modules/sdk-core/src/bitgo/staking/stakingWallet.ts

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @prettier
33
*/
44
import { CoinFamily } from '@bitgo/statics';
5+
import isEqual from 'lodash/isEqual';
56

67
import {
78
DelegationOptions,
@@ -381,6 +382,7 @@ export class StakingWallet implements IStakingWallet {
381382
}
382383

383384
const explainedTransaction = await coin.explainTransaction(result);
385+
const mismatchErrors: string[] = [];
384386

385387
if (buildParams?.recipients && buildParams.recipients.length > 0) {
386388
const userRecipientMap = new Map(
@@ -389,9 +391,6 @@ export class StakingWallet implements IStakingWallet {
389391
const platformRecipientMap = new Map(
390392
(explainedTransaction?.outputs ?? []).map((recipient) => [recipient.address.toLowerCase(), recipient])
391393
);
392-
393-
const mismatchErrors: string[] = [];
394-
395394
for (const [address] of platformRecipientMap) {
396395
if (!userRecipientMap.has(address)) {
397396
mismatchErrors.push(`Unexpected recipient address found in built transaction: ${address}`);
@@ -420,13 +419,60 @@ export class StakingWallet implements IStakingWallet {
420419
);
421420
}
422421
}
423-
if (mismatchErrors.length > 0) {
424-
const errorMessage = `Staking transaction validation failed before signing: ${mismatchErrors.join('; ')}`;
425-
debug(errorMessage);
426-
throw new Error(errorMessage);
422+
}
423+
424+
if (buildParams?.memo && (explainedTransaction as any).memo !== buildParams.memo) {
425+
mismatchErrors.push(
426+
`Memo mismatch. Expected: '${JSON.stringify(buildParams.memo)}', Got: '${JSON.stringify(
427+
(explainedTransaction as any).memo
428+
)}'`
429+
);
430+
}
431+
432+
if (buildParams?.gasLimit && String((explainedTransaction as any).gasLimit) !== String(buildParams.gasLimit)) {
433+
mismatchErrors.push(
434+
`Gas Limit mismatch. Expected: ${buildParams.gasLimit}, Got: ${(explainedTransaction as any).gasLimit}`
435+
);
436+
}
437+
438+
if (buildParams?.type && (explainedTransaction as any).type !== buildParams.type) {
439+
mismatchErrors.push(
440+
`Transaction type mismatch. Expected: '${buildParams.type}', Got: '${(explainedTransaction as any).type}'`
441+
);
442+
}
443+
444+
if (buildParams?.solInstructions) {
445+
if (!isEqual((explainedTransaction as any).solInstructions, buildParams.solInstructions)) {
446+
mismatchErrors.push(
447+
`Solana instructions mismatch. Expected: ${JSON.stringify(
448+
buildParams.solInstructions
449+
)}, Got: ${JSON.stringify((explainedTransaction as any).solInstructions)}`
450+
);
451+
}
452+
}
453+
454+
if (buildParams?.aptosCustomTransactionParams) {
455+
if (
456+
!isEqual((explainedTransaction as any).aptosCustomTransactionParams, buildParams.aptosCustomTransactionParams)
457+
) {
458+
mismatchErrors.push(
459+
`Aptos custom transaction parameters mismatch. Expected: ${JSON.stringify(
460+
buildParams.aptosCustomTransactionParams
461+
)}, Got: ${JSON.stringify((explainedTransaction as any).aptosCustomTransactionParams)}`
462+
);
427463
}
428-
} else {
429-
debug(`Cannot validate staking transaction ${transaction.stakingRequestId} without specified build params`);
464+
}
465+
466+
if (mismatchErrors.length > 0) {
467+
const errorMessage = `Staking transaction validation failed before signing: ${mismatchErrors.join('; ')}`;
468+
debug(errorMessage);
469+
throw new Error(errorMessage);
470+
}
471+
472+
if (!buildParams) {
473+
debug(
474+
`Cannot perform deep validation for staking transaction ${transaction.stakingRequestId} without specified build params`
475+
);
430476
}
431477
}
432478
}

0 commit comments

Comments
 (0)