Skip to content
Open
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
13 changes: 2 additions & 11 deletions modules/abstract-eth/src/abstractEthLikeNewCoins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
verifyMPCWalletAddress,
TssVerifyAddressOptions,
isTssVerifyAddressOptions,
NO_RECIPIENT_TX_TYPES,
} from '@bitgo/sdk-core';
import { getDerivationPath } from '@bitgo/sdk-lib-mpc';
import { bip32 } from '@bitgo/secp256k1';
Expand Down Expand Up @@ -3108,17 +3109,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
!(
txParams.prebuildTx?.consolidateId ||
txPrebuild?.consolidateId ||
(txParams.type &&
[
'acceleration',
'fillNonce',
'transferToken',
'tokenApproval',
'consolidate',
'bridgeFunds',
'enableToken',
'customTx',
].includes(txParams.type))
(txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Same issue as in bsc.ts: NO_RECIPIENT_TX_TYPES does not contain staking types (delegate, undelegate, stake, unstake, etc.). When a staking transaction arrives here with txParams = { type: 'delegate' } (returned by resolveEffectiveTxParams), this check throws 'missing txParams'.

Note that this parent class is also what bsc.ts would fall through to if the override were removed (as the commit message for 467ff30d states). Either way, the staking bypass needs to be present at this layer too.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrdanish26 let's also add the stakingRequestId, it should be in the prebuildTx object. @noel-bitgo if you can confirm.

)
) {
throw new Error('missing txParams');
Expand Down
15 changes: 10 additions & 5 deletions modules/sdk-coin-bsc/src/bsc.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { BaseCoin, BitGoBase, common, MPCAlgorithm, MultisigType, multisigTypes } from '@bitgo/sdk-core';
import {
BaseCoin,
BitGoBase,
common,
MPCAlgorithm,
MultisigType,
multisigTypes,
NO_RECIPIENT_TX_TYPES,
} from '@bitgo/sdk-core';
import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics';
import {
AbstractEthLikeNewCoins,
Expand Down Expand Up @@ -68,10 +76,7 @@ export class Bsc extends AbstractEthLikeNewCoins {
const { txParams, txPrebuild, wallet } = params;
if (
!txParams?.recipients &&
!(
txParams.prebuildTx?.consolidateId ||
(txParams.type && ['acceleration', 'fillNonce', 'transferToken', 'tokenApproval'].includes(txParams.type))
)
!(txParams.prebuildTx?.consolidateId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] This guard still throws for BSC staking transactions.

After resolveEffectiveTxParams returns effectiveTxParams = { type: 'delegate' } (no recipients), this evaluates as:

  • !txParams?.recipientstrue
  • txParams.prebuildTx?.consolidateIdundefined (no prebuildTx on staking tx)
  • NO_RECIPIENT_TX_TYPES.has('delegate')false
  • Result: throws 'missing txParams'

The staking types propagated via intent.intentType are not in NO_RECIPIENT_TX_TYPES. Same issue exists in bscToken.ts, xdc.ts, xdcToken.ts, and evmCoin.ts:119.

Possible fixes:

  1. Check for a staking signal here too (e.g. set a flag on effectiveTxParams like effectiveTxParams.isStakingIntent = true in resolveEffectiveTxParams, then check it here), or
  2. Pass staking types through NO_RECIPIENT_TX_TYPES with a comment, or
  3. Remove this override entirely and let the parent handle it (as the commit message 467ff30d says was intended — but the override is still present in the PR)

) {
throw new Error(`missing txParams`);
}
Expand Down
8 changes: 3 additions & 5 deletions modules/sdk-coin-bsc/src/bscToken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/

import { EthLikeTokenConfig, coins } from '@bitgo/statics';
import { BitGoBase, CoinConstructor, NamedCoinConstructor, MPCAlgorithm } from '@bitgo/sdk-core';
import { BitGoBase, CoinConstructor, NamedCoinConstructor, MPCAlgorithm, NO_RECIPIENT_TX_TYPES } from '@bitgo/sdk-core';
import { CoinNames, EthLikeToken, VerifyEthTransactionOptions } from '@bitgo/abstract-eth';
import { TransactionBuilder } from './lib';

Expand Down Expand Up @@ -43,6 +43,7 @@ export class BscToken extends EthLikeToken {
getFullName(): string {
return 'Bsc Token';
}

/**
* Verify if a tss transaction is valid
*
Expand All @@ -56,10 +57,7 @@ export class BscToken extends EthLikeToken {
const { txParams, txPrebuild, wallet } = params;
if (
!txParams?.recipients &&
!(
txParams.prebuildTx?.consolidateId ||
(txParams.type && ['acceleration', 'fillNonce', 'transferToken'].includes(txParams.type))
)
!(txParams.prebuildTx?.consolidateId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)))
) {
throw new Error(`missing txParams`);
}
Expand Down
7 changes: 2 additions & 5 deletions modules/sdk-coin-evm/src/evmCoin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
MPCAlgorithm,
MultisigType,
multisigTypes,
NO_RECIPIENT_TX_TYPES,
} from '@bitgo/sdk-core';
import { BaseCoin as StaticsBaseCoin, CoinFeature, coins, CoinFamily } from '@bitgo/statics';
import {
Expand Down Expand Up @@ -115,11 +116,7 @@ export class EvmCoin extends AbstractEthLikeNewCoins {
// Basic validation for legacy transactions only
if (
!txParams?.recipients &&
!(
txParams.prebuildTx?.consolidateId ||
(txParams.type &&
['acceleration', 'fillNonce', 'transferToken', 'tokenApproval', 'bridgeFunds'].includes(txParams.type))
)
!(txParams.prebuildTx?.consolidateId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)))
) {
throw new Error(`missing txParams`);
}
Expand Down
16 changes: 11 additions & 5 deletions modules/sdk-coin-xdc/src/xdc.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
import { BaseCoin, BitGoBase, common, MPCAlgorithm, MultisigType, multisigTypes } from '@bitgo/sdk-core';
import {
BaseCoin,
BitGoBase,
common,
MPCAlgorithm,
MultisigType,
multisigTypes,
NO_RECIPIENT_TX_TYPES,
} from '@bitgo/sdk-core';
import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics';
import {
AbstractEthLikeNewCoins,
Expand Down Expand Up @@ -47,6 +55,7 @@ export class Xdc extends AbstractEthLikeNewCoins {
const explorerUrl = common.Environments[this.bitgo.getEnv()].xdcExplorerBaseUrl;
return await recoveryBlockchainExplorerQuery(query, explorerUrl as string, apiToken);
}

/**
* Verify if a tss transaction is valid
*
Expand All @@ -60,10 +69,7 @@ export class Xdc extends AbstractEthLikeNewCoins {
const { txParams, txPrebuild, wallet } = params;
if (
!txParams?.recipients &&
!(
txParams.prebuildTx?.consolidateId ||
(txParams.type && ['acceleration', 'fillNonce', 'transferToken'].includes(txParams.type))
)
!(txParams.prebuildTx?.consolidateId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)))
) {
throw new Error(`missing txParams`);
}
Expand Down
14 changes: 9 additions & 5 deletions modules/sdk-coin-xdc/src/xdcToken.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@
* @prettier
*/
import { EthLikeTokenConfig, coins } from '@bitgo/statics';
import { BitGoBase, CoinConstructor, NamedCoinConstructor, common, MPCAlgorithm } from '@bitgo/sdk-core';
import {
BitGoBase,
CoinConstructor,
NamedCoinConstructor,
common,
MPCAlgorithm,
NO_RECIPIENT_TX_TYPES,
} from '@bitgo/sdk-core';
import {
CoinNames,
EthLikeToken,
Expand Down Expand Up @@ -71,10 +78,7 @@ export class XdcToken extends EthLikeToken {
const { txParams, txPrebuild, wallet } = params;
if (
!txParams?.recipients &&
!(
txParams.prebuildTx?.consolidateId ||
(txParams.type && ['acceleration', 'fillNonce', 'transferToken'].includes(txParams.type))
)
!(txParams.prebuildTx?.consolidateId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)))
) {
throw new Error(`missing txParams`);
}
Expand Down
1 change: 1 addition & 0 deletions modules/sdk-core/src/bitgo/utils/tss/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ export { ITssUtils, IEddsaUtils, TxRequest, EddsaUnsignedTransaction } from './e
export * as BaseTssUtils from './baseTSSUtils';
export * from './baseTypes';
export * from './addressVerification';
export * from './recipientUtils';
28 changes: 24 additions & 4 deletions modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { PopulatedIntent, TxRequest } from './baseTypes';

/**
* Transaction types that legitimately carry no explicit recipients.
* verifyTransaction handles no-recipient validation for these internally.
* These are non-staking ECDSA types where verifyTransaction handles
* no-recipient validation internally.
* Mirrors the bypass list in abstractEthLikeNewCoins.ts verifyTssTransaction.
*/
export const NO_RECIPIENT_TX_TYPES = new Set([
Expand All @@ -15,7 +16,7 @@ export const NO_RECIPIENT_TX_TYPES = new Set([
'consolidate',
'bridgeFunds',
'enableToken',
'customTx', // DeFi/WalletConnect smart contract interactions have no traditional recipients
'customTx',
]);

/**
Expand All @@ -25,8 +26,13 @@ export const NO_RECIPIENT_TX_TYPES = new Set([
* (native amount = 0, so buildParams is empty). Falls back to intent recipients
* mapped to ITransactionRecipient shape when txParams.recipients is absent.
*
* Staking intents (BSC delegate/undelegate, CELO stake/unstake, etc.) are
* identified generically by the presence of `stakingRequestId` on the intent —
* a required field on BaseStakeIntent in @bitgo/public-types. These intents
* have no txParams recipients by design; validation is done at the coin layer.
*
* Throws InvalidTransactionError if no recipients can be resolved and the
* transaction type is not a known no-recipient type.
* transaction is not a known no-recipient type.
*/
export function resolveEffectiveTxParams(
txRequest: TxRequest,
Expand All @@ -43,7 +49,21 @@ export function resolveEffectiveTxParams(
recipients: txParams?.recipients?.length ? txParams.recipients : intentRecipients,
};

if (!effectiveTxParams.recipients?.length && !NO_RECIPIENT_TX_TYPES.has(effectiveTxParams.type ?? '')) {
// Fall back to intent.intentType when txParams.type is not explicitly set.
// Staking wallets call signTransaction without txParams, so the type lives only in the intent.
const txType = effectiveTxParams.type ?? (txRequest.intent as PopulatedIntent)?.intentType ?? '';

// Propagate the resolved type so downstream callers (e.g. verifyTssTransaction) can use it.
if (!effectiveTxParams.type && txType) {
effectiveTxParams.type = txType;
}

// All staking intents (BSC delegate/undelegate, CELO stake/unstake, etc.) carry
// stakingRequestId as a required field on BaseStakeIntent (@bitgo/public-types).
// Use its presence as a generic staking signal — no need to enumerate every intentType.
const isStakingIntent = !!(txRequest.intent as any)?.stakingRequestId;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[High] as any cast is unsafe. PopulatedIntent in baseTypes.ts does not define stakingRequestId, so TypeScript cannot verify this field exists at compile time. If BaseStakeIntent in @bitgo/public-types renames or removes stakingRequestId, this silently returns false at runtime with no compile-time error.

Consider importing BaseStakeIntent from @bitgo/public-types and writing a proper type guard:

function isBaseStakeIntent(intent: unknown): intent is BaseStakeIntent {
  return typeof (intent as any)?.stakingRequestId === 'string';
}
const isStakingIntent = isBaseStakeIntent(txRequest.intent);

Or extend the PopulatedIntent union in baseTypes.ts to include a staking variant.


if (!effectiveTxParams.recipients?.length && !isStakingIntent && !NO_RECIPIENT_TX_TYPES.has(txType)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] This fix prevents InvalidTransactionError here, but the staking bypass does not propagate to verifyTssTransaction.

The full call chain after this function returns for a staking intent (stakingRequestId present, no recipients) is:

resolveEffectiveTxParams returns { type: 'delegate' }  ← fixed here
  → baseCoin.verifyTransaction({ txParams: { type: 'delegate' } })
  → verifyTssTransaction: !txParams?.recipients && !NO_RECIPIENT_TX_TYPES.has('delegate')
  → throws 'missing txParams'  ← NOT fixed

'delegate', 'undelegate', 'stake', 'unstake' are not in NO_RECIPIENT_TX_TYPES, so every verifyTssTransaction override (bsc.ts:79, bscToken.ts:57, xdc.ts:79, xdcToken.ts:81, evmCoin.ts:119, abstractEthLikeNewCoins.ts:3112) will still throw for BSC staking.

The fix moves the error from InvalidTransactionError (here) to Error('missing txParams') (in verifyTssTransaction) but does not eliminate it. The staking flow remains broken end-to-end.

throw new InvalidTransactionError(
'Recipient details are required to verify this transaction before signing. Pass txParams with at least one recipient.'
);
Expand Down
53 changes: 48 additions & 5 deletions modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,15 @@ import * as assert from 'assert';
const getModule = () => require('../../../../../src/bitgo/utils/tss/recipientUtils');

function makeTxRequest(
intentRecipients?: { address: { address: string }; amount: { value: string }; data?: string }[]
intentRecipients?: { address: { address: string }; amount: { value: string }; data?: string }[],
intentType = 'payment',
stakingRequestId?: string
): any {
return {
txRequestId: 'test-req-id',
intent: intentRecipients ? { intentType: 'contractCall', recipients: intentRecipients } : { intentType: 'payment' },
intent: intentRecipients
? { intentType: 'contractCall', recipients: intentRecipients }
: { intentType, ...(stakingRequestId ? { stakingRequestId } : {}) },
transactions: [],
unsignedTxs: [],
state: 'pendingUserSignature',
Expand All @@ -23,7 +27,7 @@ function makeTxRequest(

describe('recipientUtils', function () {
describe('NO_RECIPIENT_TX_TYPES', function () {
it('contains exactly the 8 expected exempted types', function () {
it('contains all expected exempted types', function () {
const { NO_RECIPIENT_TX_TYPES } = getModule();
const expected = [
'acceleration',
Expand Down Expand Up @@ -105,8 +109,8 @@ describe('recipientUtils', function () {
'tokenApproval',
'consolidate',
'bridgeFunds',
'enableToken', // TSS wallets do not populate recipients for token enablement
'customTx', // DeFi/WalletConnect smart contract interactions have no traditional recipients
'enableToken',
'customTx',
];

NO_RECIPIENT_TYPES.forEach((type) => {
Expand All @@ -117,5 +121,44 @@ describe('recipientUtils', function () {
result.type.should.equal(type);
});
});

it('allows staking intent with no recipients when stakingRequestId is present (BSC delegate)', function () {
const { resolveEffectiveTxParams } = getModule();
// Simulate BSC staking wallet: no txParams, intent has stakingRequestId
const txRequest = makeTxRequest(undefined, 'delegate', 'staking-req-123');
const result = resolveEffectiveTxParams(txRequest, undefined);
result.type.should.equal('delegate');
});

it('allows staking intent with no recipients when stakingRequestId is present (CELO stake)', function () {
const { resolveEffectiveTxParams } = getModule();
const txRequest = makeTxRequest(undefined, 'stake', 'staking-req-456');
const result = resolveEffectiveTxParams(txRequest, undefined);
result.type.should.equal('stake');
});

it('throws for unknown staking-like type without stakingRequestId', function () {
const { resolveEffectiveTxParams } = getModule();
// No stakingRequestId, no recipients, unknown type — should throw
const txRequest = makeTxRequest(undefined, 'delegate');
assert.throws(
() => resolveEffectiveTxParams(txRequest, undefined),
/Recipient details are required to verify this transaction before signing/
);
});

it('propagates intent.intentType into effectiveTxParams.type when txParams.type is absent', function () {
const { resolveEffectiveTxParams } = getModule();
const txRequest = makeTxRequest(undefined, 'stake', 'staking-req-789');
const result = resolveEffectiveTxParams(txRequest, {});
result.type.should.equal('stake');
});

it('does not override txParams.type when already set', function () {
const { resolveEffectiveTxParams } = getModule();
const txRequest = makeTxRequest(undefined, 'delegate', 'staking-req-000');
const result = resolveEffectiveTxParams(txRequest, { type: 'acceleration' });
result.type.should.equal('acceleration');
});
});
});
Loading