diff --git a/modules/bitgo/test/v2/unit/staking/stakingWalletNonTSS.ts b/modules/bitgo/test/v2/unit/staking/stakingWalletNonTSS.ts index bbaf906e81..26eefeb867 100644 --- a/modules/bitgo/test/v2/unit/staking/stakingWalletNonTSS.ts +++ b/modules/bitgo/test/v2/unit/staking/stakingWalletNonTSS.ts @@ -400,7 +400,7 @@ describe('non-TSS Staking Wallet', function () { await topethWctStakingWallet.buildAndSign({ walletPassphrase: 'passphrase' }, stakingTransaction); }); - it('should throw an error if unsigned transaction does not match the staking transaction', async function () { + it('should log an error if a transaction is failed to be validated transaction if unsigned transaction does not match the staking transaction', async function () { const unsignedTransaction: PrebuildTransactionResult = { walletId: topethWctStakingWallet.walletId, ...opethFixtures.unsignedStakingTransaction, @@ -409,6 +409,8 @@ describe('non-TSS Staking Wallet', function () { } as PrebuildTransactionResult; const stakingTransaction: StakingTransaction = opethFixtures.updatedStakingRequest; + const consoleStub = sinon.stub(console, 'error'); + nock(microservicesUri) .get( `/api/staking/v1/${topethWctStakingWallet.coin}/wallets/${topethWctStakingWallet.walletId}/requests/${stakingTransaction.stakingRequestId}/transactions/${stakingTransaction.id}` @@ -424,14 +426,11 @@ describe('non-TSS Staking Wallet', function () { .post(`/api/v2/topeth/wallet/${topethWctStakingWallet.walletId}/tx/build`) .reply(200, unsignedTransaction); - const expectedErrorMessage = - 'Staking transaction validation failed before signing: ' + - 'Unexpected recipient address found in built transaction: 0x86bb6dca2cd6f9a0189c478bbb8f7ee2fef07c89; ' + - 'Expected recipient address not found in built transaction: 0x75bb6dca2cd6f9a0189c478bbb8f7ee2fef07c78'; + await topethWctStakingWallet.buildAndSign({ walletPassphrase: 'passphrase' }, stakingTransaction); - await topethWctStakingWallet - .buildAndSign({ walletPassphrase: 'passphrase' }, stakingTransaction) - .should.be.rejectedWith(expectedErrorMessage); + consoleStub.calledWith( + 'Invalid recipient address: 0x86bb6dca2cd6f9a0189c478bbb8f7ee2fef07c89, Missing recipient address(es): 0x75bb6dca2cd6f9a0189c478bbb8f7ee2fef07c78' + ); }); }); }); diff --git a/modules/sdk-core/src/bitgo/staking/stakingWallet.ts b/modules/sdk-core/src/bitgo/staking/stakingWallet.ts index ebf14462a5..fb94a35209 100644 --- a/modules/sdk-core/src/bitgo/staking/stakingWallet.ts +++ b/modules/sdk-core/src/bitgo/staking/stakingWallet.ts @@ -2,7 +2,6 @@ * @prettier */ import { CoinFamily } from '@bitgo/statics'; -import isEqual from 'lodash/isEqual'; import { DelegationOptions, @@ -282,7 +281,7 @@ export class StakingWallet implements IStakingWallet { (signOptions.transactionVerificationOptions?.skipTransactionVerification || this.isBtcUndelegate(transaction)) ?? false; if (!isStakingTxRequestPrebuildResult(builtTx.result) && !skipVerification) { - await this.validateBuiltStakingTransaction(builtTx.transaction, builtTx); + await this.validateBuiltStakingTransaction(transaction, builtTx); } return await this.sign(signOptions, builtTx); } @@ -382,97 +381,55 @@ export class StakingWallet implements IStakingWallet { } const explainedTransaction = await coin.explainTransaction(result); - const mismatchErrors: string[] = []; - if (buildParams?.recipients && buildParams.recipients.length > 0) { + if (buildParams?.recipients) { const userRecipientMap = new Map( buildParams.recipients.map((recipient) => [recipient.address.toLowerCase(), recipient]) ); const platformRecipientMap = new Map( (explainedTransaction?.outputs ?? []).map((recipient) => [recipient.address.toLowerCase(), recipient]) ); - for (const [address] of platformRecipientMap) { - if (!userRecipientMap.has(address)) { - mismatchErrors.push(`Unexpected recipient address found in built transaction: ${address}`); - } - } - - for (const [address, userRecipient] of userRecipientMap) { - const platformRecipient = platformRecipientMap.get(address); - if (!platformRecipient) { - mismatchErrors.push(`Expected recipient address not found in built transaction: ${address}`); - continue; - } - - const matchResult = transactionRecipientsMatch(userRecipient, platformRecipient); - if (!matchResult.amountMatch) { - mismatchErrors.push( - `Recipient ${address} amount mismatch. Expected: ${userRecipient.amount}, Got: ${platformRecipient.amount}` - ); - } - if (!matchResult.tokenMatch) { - mismatchErrors.push( - `Recipient ${address} token mismatch. Expected: ${userRecipient.tokenName ?? 'native coin'}, Got: ${ - platformRecipient.tokenName ?? 'native coin' - }` - ); + const mismatchErrors: string[] = []; + + for (const [recipientAddress, recipientInfo] of platformRecipientMap) { + if (userRecipientMap.has(recipientAddress)) { + const userRecipient = userRecipientMap.get(recipientAddress); + if (!userRecipient) { + console.error('Unable to determine recipient address'); + return; + } + const matchResult = transactionRecipientsMatch(userRecipient, recipientInfo); + if (!matchResult.exactMatch) { + if (!matchResult.tokenMatch) { + mismatchErrors.push( + `Invalid token ${recipientInfo.tokenName} transfer with amount ${recipientInfo.amount} to ${recipientInfo.address} found in built transaction, specified ${userRecipient.tokenName}` + ); + } + if (!matchResult.amountMatch) { + mismatchErrors.push( + `Invalid recipient amount for ${recipientInfo.address}, specified ${userRecipient.amount} got ${recipientInfo.amount}` + ); + } + } + } else { + mismatchErrors.push(`Invalid recipient address: ${recipientAddress}`); } } - } - - if (buildParams?.memo && (explainedTransaction as any).memo !== buildParams.memo) { - mismatchErrors.push( - `Memo mismatch. Expected: '${JSON.stringify(buildParams.memo)}', Got: '${JSON.stringify( - (explainedTransaction as any).memo - )}'` - ); - } - - if (buildParams?.gasLimit && String((explainedTransaction as any).gasLimit) !== String(buildParams.gasLimit)) { - mismatchErrors.push( - `Gas Limit mismatch. Expected: ${buildParams.gasLimit}, Got: ${(explainedTransaction as any).gasLimit}` - ); - } - if (buildParams?.type && (explainedTransaction as any).type !== buildParams.type) { - mismatchErrors.push( - `Transaction type mismatch. Expected: '${buildParams.type}', Got: '${(explainedTransaction as any).type}'` + const missingRecipientAddresses = Array.from(userRecipientMap.keys()).filter( + (address) => !platformRecipientMap.has(address) ); - } - if (buildParams?.solInstructions) { - if (!isEqual((explainedTransaction as any).solInstructions, buildParams.solInstructions)) { - mismatchErrors.push( - `Solana instructions mismatch. Expected: ${JSON.stringify( - buildParams.solInstructions - )}, Got: ${JSON.stringify((explainedTransaction as any).solInstructions)}` - ); + if (missingRecipientAddresses.length > 0) { + mismatchErrors.push(`Missing recipient address(es): ${missingRecipientAddresses.join(', ')}`); } - } - - if (buildParams?.aptosCustomTransactionParams) { - if ( - !isEqual((explainedTransaction as any).aptosCustomTransactionParams, buildParams.aptosCustomTransactionParams) - ) { - mismatchErrors.push( - `Aptos custom transaction parameters mismatch. Expected: ${JSON.stringify( - buildParams.aptosCustomTransactionParams - )}, Got: ${JSON.stringify((explainedTransaction as any).aptosCustomTransactionParams)}` - ); + if (mismatchErrors.length > 0) { + console.error(mismatchErrors.join(', ')); + return; } - } - - if (mismatchErrors.length > 0) { - const errorMessage = `Staking transaction validation failed before signing: ${mismatchErrors.join('; ')}`; - debug(errorMessage); - throw new Error(errorMessage); - } - - if (!buildParams) { - debug( - `Cannot perform deep validation for staking transaction ${transaction.stakingRequestId} without specified build params` - ); + } else { + debug(`Cannot validate staking transaction ${transaction.stakingRequestId} without specified build params`); } } }