From 74ae57a95569b5c9934efc31794eb7dc467ee707 Mon Sep 17 00:00:00 2001 From: bitgobot Date: Mon, 12 Oct 2026 07:39:42 +0000 Subject: [PATCH] fix(abstract-cosmos): verify memo-suffixed recipients against tx memo verifyTransaction compared txParams.recipients to the decoded outputs as raw {address, amount} strings. Recipients using the BitGo combined address+memo convention (address?memoId=X) therefore never matched the plain on-chain address reported by explainTransaction(), so every withdrawal to a memo-tagged recipient failed local validation with "Tx outputs does not match with expected txParams recipients" even though the prebuild was correct. This blocked TIA testnet withdrawals in COINS-1760. Normalize both sides to a {address, amount, memo?} shape via getAddressDetails before comparing, which also checks the recipient memoId against the transaction memo. This lifts the fix previously applied per-coin in sdk-coin-zeta (COINS-1676) into the shared base class so TIA and all other Cosmos coins behave consistently. The now-redundant Zeta override and its unused lodash dependency are removed, and regression tests cover the memo-suffixed recipient cases for TIA. Ticket: COINS-1760 Session-Id: 59955df8-a88a-4a5d-b7b4-b42d2da9596e Task-Id: a5b99c75-8301-4e58-8dae-43b673a64970 --- modules/abstract-cosmos/src/cosmosCoin.ts | 20 ++++++-- modules/sdk-coin-tia/test/unit/tia.ts | 62 +++++++++++++++++++++++ modules/sdk-coin-zeta/package.json | 6 +-- modules/sdk-coin-zeta/src/zeta.ts | 49 +----------------- 4 files changed, 82 insertions(+), 55 deletions(-) diff --git a/modules/abstract-cosmos/src/cosmosCoin.ts b/modules/abstract-cosmos/src/cosmosCoin.ts index cd7ef8de6c4..7345bdbc3fe 100644 --- a/modules/abstract-cosmos/src/cosmosCoin.ts +++ b/modules/abstract-cosmos/src/cosmosCoin.ts @@ -545,7 +545,18 @@ export class CosmosCoin extends BaseCoin { return { serializedTx: serializedTx }; } - /** @inheritDoc **/ + /** + * @inheritDoc + * + * Recipients can carry their memo folded into the address as `address?memoId=X` + * (the BitGo combined address+memo convention - see getAddressDetails), while + * explainTransaction() reports the plain on-chain address plus a separate `memo` + * field on each output. Comparing the two sides as raw strings made every + * memo-tagged recipient fail verification even when the transaction was built + * correctly (COINS-1760). Normalize both sides to the same { address, amount, + * memo? } shape before comparing, which also checks the recipient memoId + * against the transaction memo. + */ async verifyTransaction(params: VerifyTransactionOptions): Promise { let totalAmount = new BigNumber(0); const { txPrebuild, txParams } = params; @@ -556,8 +567,11 @@ export class CosmosCoin extends BaseCoin { const transaction = await this.getBuilder().from(rawTx).build(); const explainedTx = transaction.explainTransaction(); if (txParams.recipients && txParams.recipients.length > 0) { - const filteredRecipients = txParams.recipients?.map((recipient) => _.pick(recipient, ['address', 'amount'])); - const filteredOutputs = explainedTx.outputs.map((output) => _.pick(output, ['address', 'amount'])); + const filteredRecipients = txParams.recipients.map((recipient) => { + const { address, memoId } = this.getAddressDetails(recipient.address); + return { address, amount: recipient.amount, ...(memoId !== undefined ? { memo: memoId } : {}) }; + }); + const filteredOutputs = explainedTx.outputs.map((output) => _.pick(output, ['address', 'amount', 'memo'])); if (!_.isEqual(filteredOutputs, filteredRecipients)) { throw new Error('Tx outputs does not match with expected txParams recipients'); diff --git a/modules/sdk-coin-tia/test/unit/tia.ts b/modules/sdk-coin-tia/test/unit/tia.ts index 7c41b3d201c..71118b71be3 100644 --- a/modules/sdk-coin-tia/test/unit/tia.ts +++ b/modules/sdk-coin-tia/test/unit/tia.ts @@ -209,6 +209,68 @@ describe('TIA', function () { }) .should.rejectedWith('missing required tx prebuild property txHex'); }); + + it('should succeed to verify transaction with a memo-suffixed recipient address', async function () { + // Regression test for COINS-1760: recipients can carry their memo folded into the + // address as `address?memoId=X` (the BitGo combined address+memo convention), but + // explainTransaction() reports the plain on-chain address plus a separate memo + // field. Comparing raw strings made every memo-tagged recipient - like the + // `?memoId=0` withdrawal recipient in COINS-1760 - fail verification even though + // the transaction was built correctly. + const txPrebuild = { + txHex: TEST_TX_WITH_MEMO.signedTxBase64, + txInfo: {}, + }; + const txParams = { + recipients: [ + { + address: `${TEST_TX_WITH_MEMO.to}?memoId=${TEST_TX_WITH_MEMO.memo}`, + amount: TEST_TX_WITH_MEMO.sendAmount, + }, + ], + }; + const verification = {}; + const isTransactionVerified = await basecoin.verifyTransaction({ txParams, txPrebuild, verification }); + isTransactionVerified.should.equal(true); + }); + + it('should fail to verify transaction when the recipient memoId does not match the tx memo', async function () { + const txPrebuild = { + txHex: TEST_TX_WITH_MEMO.signedTxBase64, + txInfo: {}, + }; + const txParams = { + recipients: [ + { + address: `${TEST_TX_WITH_MEMO.to}?memoId=999999`, + amount: TEST_TX_WITH_MEMO.sendAmount, + }, + ], + }; + const verification = {}; + await basecoin + .verifyTransaction({ txParams, txPrebuild, verification }) + .should.be.rejectedWith('Tx outputs does not match with expected txParams recipients'); + }); + + it('should fail to verify transaction when the recipient has no memoId but the tx has one', async function () { + const txPrebuild = { + txHex: TEST_TX_WITH_MEMO.signedTxBase64, + txInfo: {}, + }; + const txParams = { + recipients: [ + { + address: TEST_TX_WITH_MEMO.to, + amount: TEST_TX_WITH_MEMO.sendAmount, + }, + ], + }; + const verification = {}; + await basecoin + .verifyTransaction({ txParams, txPrebuild, verification }) + .should.be.rejectedWith('Tx outputs does not match with expected txParams recipients'); + }); }); describe('Explain Transaction: ', () => { diff --git a/modules/sdk-coin-zeta/package.json b/modules/sdk-coin-zeta/package.json index 415baf93f52..6dad94e57ea 100644 --- a/modules/sdk-coin-zeta/package.json +++ b/modules/sdk-coin-zeta/package.json @@ -47,13 +47,11 @@ "@cosmjs/amino": "^0.29.5", "@cosmjs/encoding": "^0.29.5", "@cosmjs/stargate": "^0.29.5", - "bignumber.js": "^9.1.1", - "lodash": "^4.18.0" + "bignumber.js": "^9.1.1" }, "devDependencies": { "@bitgo/sdk-api": "^3.0.7", - "@bitgo/sdk-test": "^9.1.84", - "@types/lodash": "^4.14.183" + "@bitgo/sdk-test": "^9.1.84" }, "gitHead": "18e460ddf02de2dbf13c2aa243478188fb539f0c", "files": [ diff --git a/modules/sdk-coin-zeta/src/zeta.ts b/modules/sdk-coin-zeta/src/zeta.ts index 01bbf33ee95..8858116db14 100644 --- a/modules/sdk-coin-zeta/src/zeta.ts +++ b/modules/sdk-coin-zeta/src/zeta.ts @@ -1,8 +1,6 @@ import { CosmosCoin, CosmosKeyPair, GasAmountDetails } from '@bitgo/abstract-cosmos'; -import { BaseCoin, BitGoBase, Environments, TransactionType, VerifyTransactionOptions } from '@bitgo/sdk-core'; +import { BaseCoin, BitGoBase, Environments } from '@bitgo/sdk-core'; import { BaseCoin as StaticsBaseCoin, BaseUnit, coins } from '@bitgo/statics'; -import BigNumber from 'bignumber.js'; -import * as _ from 'lodash'; import { KeyPair, TransactionBuilderFactory } from './lib'; import { GAS_AMOUNT, GAS_LIMIT } from './lib/constants'; import utils from './lib/utils'; @@ -74,49 +72,4 @@ export class Zeta extends CosmosCoin { } return [response.body.account.base_account.account_number, response.body.account.base_account.sequence]; } - - /** - * @inheritDoc - * - * Overrides CosmosCoin.verifyTransaction to fix a ZETA-specific gap: recipients can carry - * their memo folded into the address as `address?memoId=X` (the BitGo combined - * address+memo convention - see getAddressDetails), but explainTransaction() always - * reports the plain on-chain address plus a separate `memo` field. The base class - * compares the two sides as raw strings, so any memo-tagged recipient fails verification - * even when the transaction was built correctly - this reproduced a live incident where a - * client could never sign a correctly-built, pending ZETA transaction to a memo'd - * recipient. Normalize both sides to the same {address, amount, memo?} shape before - * comparing. - */ - async verifyTransaction(params: VerifyTransactionOptions): Promise { - let totalAmount = new BigNumber(0); - const { txPrebuild, txParams } = params; - const rawTx = txPrebuild.txHex; - if (!rawTx) { - throw new Error('missing required tx prebuild property txHex'); - } - const transaction = await this.getBuilder().from(rawTx).build(); - const explainedTx = transaction.explainTransaction(); - if (txParams.recipients && txParams.recipients.length > 0) { - const filteredRecipients = txParams.recipients.map((recipient) => { - const { address, memoId } = this.getAddressDetails(recipient.address); - return { address, amount: recipient.amount, ...(memoId !== undefined ? { memo: memoId } : {}) }; - }); - const filteredOutputs = explainedTx.outputs.map((output) => _.pick(output, ['address', 'amount', 'memo'])); - - if (!_.isEqual(filteredOutputs, filteredRecipients)) { - throw new Error('Tx outputs does not match with expected txParams recipients'); - } - // WithdrawDelegatorRewards and ContractCall transaction don't have amount - if (transaction.type !== TransactionType.StakingWithdraw && transaction.type !== TransactionType.ContractCall) { - for (const recipient of txParams.recipients) { - totalAmount = totalAmount.plus(recipient.amount); - } - if (!totalAmount.isEqualTo(explainedTx.outputAmount)) { - throw new Error('Tx total amount does not match with expected total amount field'); - } - } - } - return true; - } }