From 816eb19a78b6fd22539c874c07241b7873e01963 Mon Sep 17 00:00:00 2001 From: Prabhsharan Singh Date: Wed, 28 Oct 2026 11:47:29 +0530 Subject: [PATCH] fix(sdk-core): send empty recipients for ERC-7984 builds (CHALO-1957) The /tx/build OpenAPI contract requires the recipients KEY on every build body (TransactionBuildRequest.required) - omitting the key 400s with SchemaValidationError before the handler runs (CHALO-1991). An EMPTY array is the documented server-derived shape (see the recipients description: acceleration, token enablement). For the ERC-7984 intent types (wrapApprove/wrap/unwrap/finalizeUnwrap) the real recipients and calldata are derived server-side from the intent, so: - recreateAndSignTransaction forwards an EMPTY recipients array instead of deleting the key; the /tx/build PA-rebuild carve-out re-derives the real inputs from the pendingApprovalId approve() injects. Other PA types keep forwarding their stored recipients unchanged. - prebuildTransaction defaults the OUTGOING body's recipients to [] for direct intent builds (the drawer flow), after the TSS txrequests early-return; the caller's params object is never mutated (a prebuild + prebuildTx sign flow would otherwise trip the prebuildTx check). - the prebuildAndSignTransaction deny-list guard now rejects only a POPULATED recipients list for the intent types; the empty-array shape passes. fillNonce/acceleration behaviour is unchanged. Shared const ERC7984_INTENT_TX_TYPES moves to wallet/BuildParams.ts so the guard, prebuild and PA rebuild agree on the type set. Tests: request-shape assertions flipped to recipients: [] for direct wrapApprove/unwrap (and wrap/finalizeUnwrap) prebuilds and the PA approve rebuilds; guard cases (empty array allowed for wrapApprove, still rejected for fillNonce); no caller-params mutation pinned; transfer-type PA regression unchanged. Ticket: CHALO-1957 --- modules/bitgo/test/v2/unit/pendingApproval.ts | 6 +- modules/bitgo/test/v2/unit/wallet.ts | 78 ++++++++++++++++++- .../bitgo/pendingApproval/pendingApproval.ts | 22 ++---- .../sdk-core/src/bitgo/wallet/BuildParams.ts | 9 +++ modules/sdk-core/src/bitgo/wallet/wallet.ts | 12 ++- 5 files changed, 105 insertions(+), 22 deletions(-) diff --git a/modules/bitgo/test/v2/unit/pendingApproval.ts b/modules/bitgo/test/v2/unit/pendingApproval.ts index e8c0b09dd2..3a71b58d1d 100644 --- a/modules/bitgo/test/v2/unit/pendingApproval.ts +++ b/modules/bitgo/test/v2/unit/pendingApproval.ts @@ -481,7 +481,7 @@ describe('Pending Approvals:', () => { } (['wrap', 'finalizeUnwrap'] as const).forEach((legType) => { - it(`strips stored recipients when approving a ${legType} intent pending approval`, async () => { + it(`sends empty recipients when approving a ${legType} intent pending approval`, async () => { const { pendingApproval, htethCoin, paData } = makeEthIntentPendingApproval(legType); const prebuildAndSignStub = sandbox @@ -502,7 +502,7 @@ describe('Pending Approvals:', () => { paScope.isDone().should.be.true(); prebuildAndSignStub.calledOnce.should.be.true(); const prebuildParamsArg = prebuildAndSignStub.getCall(0).args[0]!; - should.not.exist(prebuildParamsArg.recipients); + prebuildParamsArg.recipients!.should.deepEqual([]); prebuildParamsArg.type!.should.equal(legType); (prebuildParamsArg.pendingApprovalId as string).should.equal(`pa-${legType}`); }); @@ -633,7 +633,7 @@ describe('Pending Approvals:', () => { .post(`/api/v2/hteth/wallet/${walletId}/tx/build`, (body) => { body.type.should.equal(legType); body.pendingApprovalId.should.equal(paId); - should.not.exist(body.recipients); + body.recipients.should.deepEqual([]); should.not.exist(body.wrapParams); if (legType === 'unwrap') { body.unwrapParams.should.deepEqual(unwrapParams); diff --git a/modules/bitgo/test/v2/unit/wallet.ts b/modules/bitgo/test/v2/unit/wallet.ts index 1047b21ee1..c03a683c0b 100644 --- a/modules/bitgo/test/v2/unit/wallet.ts +++ b/modules/bitgo/test/v2/unit/wallet.ts @@ -654,21 +654,28 @@ describe('V2 Wallet:', function () { it('should send wrapParams to /tx/build when prebuilding wrapApprove on an onchain multi-sig wallet', async function () { const wrapParams = { tokenName: 'hteth:cusdt', amount: '1000000' }; + const callerParams: { type: string; wrapParams: { tokenName: string; amount: string }; recipients?: unknown } = { + type: 'wrapApprove', + wrapParams, + }; const buildNock = nock(bgUrl) .post(`/api/v2/${ethWallet.coin()}/wallet/${ethWallet.id()}/tx/build`, (body) => { body.type.should.equal('wrapApprove'); body.wrapParams.should.deepEqual(wrapParams); - should.not.exist(body.recipients); + body.recipients.should.deepEqual([]); return true; }) .reply(200, { txHex: '0xfaketxhex', feeInfo: { fee: '1000' } }); try { - await ethWallet.prebuildTransaction({ type: 'wrapApprove', wrapParams }); + await ethWallet.prebuildTransaction(callerParams); } catch (e) { // post-processing of the fake prebuild may fail; the contract under test is the request body } buildNock.isDone().should.be.true(); + // the caller's params must stay untouched: they flow into prebuildAndSignTransaction + // (prebuildTx + sign flows), where a truthy recipients trips the prebuildTx check + should.not.exist(callerParams.recipients); }); it('should send unwrapParams to /tx/build when prebuilding unwrap on an onchain multi-sig wallet', async function () { @@ -677,7 +684,7 @@ describe('V2 Wallet:', function () { .post(`/api/v2/${ethWallet.coin()}/wallet/${ethWallet.id()}/tx/build`, (body) => { body.type.should.equal('unwrap'); body.unwrapParams.should.deepEqual(unwrapParams); - should.not.exist(body.recipients); + body.recipients.should.deepEqual([]); return true; }) .reply(200, { txHex: '0xfaketxhex', feeInfo: { fee: '1000' } }); @@ -690,6 +697,40 @@ describe('V2 Wallet:', function () { buildNock.isDone().should.be.true(); }); + it('should send empty recipients to /tx/build when prebuilding wrap (request-shape contract)', async function () { + const buildNock = nock(bgUrl) + .post(`/api/v2/${ethWallet.coin()}/wallet/${ethWallet.id()}/tx/build`, (body) => { + body.type.should.equal('wrap'); + body.recipients.should.deepEqual([]); + return true; + }) + .reply(200, { txHex: '0xfaketxhex', feeInfo: { fee: '1000' } }); + + try { + await ethWallet.prebuildTransaction({ type: 'wrap' }); + } catch (e) { + // the server rejects direct wrap builds; the contract under test is the request body + } + buildNock.isDone().should.be.true(); + }); + + it('should send empty recipients to /tx/build when prebuilding finalizeUnwrap (request-shape contract)', async function () { + const buildNock = nock(bgUrl) + .post(`/api/v2/${ethWallet.coin()}/wallet/${ethWallet.id()}/tx/build`, (body) => { + body.type.should.equal('finalizeUnwrap'); + body.recipients.should.deepEqual([]); + return true; + }) + .reply(200, { txHex: '0xfaketxhex', feeInfo: { fee: '1000' } }); + + try { + await ethWallet.prebuildTransaction({ type: 'finalizeUnwrap' }); + } catch (e) { + // the server rejects direct finalizeUnwrap builds; the contract under test is the request body + } + buildNock.isDone().should.be.true(); + }); + it('should not pass recipients in sendMany when transaction type is finalizeUnwrap', async function () { const recipients = [ { @@ -724,6 +765,37 @@ describe('V2 Wallet:', function () { caught!.message.should.equal('cannot provide recipients for transaction type wrapApprove'); caught!.code!.should.equal('recipients_not_allowed_for_fillnonce_and_acceleration_tx_type'); }); + + it('allows an empty recipients array when prebuilding wrapApprove (server-derived shape)', async function () { + const nockKeychain = nock(bgUrl).get(`/api/v2/${ethWallet.coin()}/key/${ethWallet.keyIds()[0]}`).reply(200, { + id: '598f606cd8fc24710d2ebad89dce86c2', + pub: 'xpub661MyMwAqRbcFXDcWD2vxuebcT1ZpTF4Vke6qmMW8yzddwNYpAPjvYEEL5jLfyYXW2fuxtAxY8TgjPUJLcf1C8qz9N6VgZxArKX4EwB8rH5', + ethAddress: '0x26a163ba9739529720c0914c583865dec0d37278', + source: 'user', + encryptedPrv: + '{"iv":"15FsbDVI1zG9OggD8YX+Hg==","v":1,"iter":10000,"ks":256,"ts":64,"mode":"ccm","adata":"","cipher":"aes","salt":"hHbNH3Sz/aU=","ct":"WoNVKz7afiRxXI2w/YkzMdMyoQg/B15u1Q8aQgi96jJZ9wk6TIaSEc6bXFH3AHzD9MdJCWJQUpRhoQc/rgytcn69scPTjKeeyVMElGCxZdFVS/psQcNE+lue3//2Zlxj+6t1NkvYO+8yAezSMRBK5OdftXEjNQI="}', + coinSpecific: {}, + }); + + // the guard must let the empty-array shape through; the failure below is the passphrase check + await ethWallet + .prebuildAndSignTransaction({ type: 'wrapApprove', recipients: [], walletPassphrase: 'wrongPassphrase' }) + .should.be.rejectedWith('unable to decrypt keychain with the given wallet passphrase'); + nockKeychain.isDone().should.be.true(); + }); + + it('still rejects an empty recipients array for fillNonce (guard regression)', async function () { + let caught: (Error & { code?: string }) | undefined; + try { + await ethWallet.prebuildAndSignTransaction({ type: 'fillNonce', recipients: [] }); + } catch (e) { + caught = e; + } + + should.exist(caught); + caught!.message.should.equal('cannot provide recipients for transaction type fillNonce'); + caught!.code!.should.equal('recipients_not_allowed_for_fillnonce_and_acceleration_tx_type'); + }); }); describe('OFC Create Address', () => { diff --git a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts index b2899d633b..4f18b59d69 100644 --- a/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts +++ b/modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts @@ -18,7 +18,7 @@ import { import { RequestTracer, RequestType } from '../utils'; import { IWallet } from '../wallet'; import { isSafeChildPublicOnlyKeychain } from '../wallet/safeKeychain'; -import { BuildParams } from '../wallet/BuildParams'; +import { BuildParams, ERC7984_INTENT_TX_TYPES } from '../wallet/BuildParams'; import { IRequestTracer } from '../../api'; import BaseTssUtils from '../utils/tss/baseTSSUtils'; import EddsaUtils, { EddsaMPCv2Utils } from '../utils/tss/eddsa'; @@ -40,15 +40,6 @@ type ApprovePendingApprovalRequestBody = { halfSigned?: string | Omit; }; -// ERC-7984 shield/unshield intent types: calldata is built server-side from the intent, so a -// pending approval's stored recipients are display-only for the approval UI, never build inputs. -const ERC7984_INTENT_TX_TYPES: Record = { - wrapApprove: true, - wrap: true, - unwrap: true, - finalizeUnwrap: true, -}; - export class PendingApproval implements IPendingApproval { private readonly bitgo: BitGoBase; private readonly baseCoin: IBaseCoin; @@ -313,11 +304,12 @@ export class PendingApproval implements IPendingApproval { const recipients = transactionRequest.recipients; let prebuildParams = _.extend({}, params, { recipients: recipients }, transactionRequest.buildParams); - // Stored recipients must not be forwarded for these intents — the prebuild guard rejects - // recipients for them, and the /tx/build PA-rebuild carve-out re-derives the real inputs - // server-side from the pendingApprovalId approve() injects. - if (ERC7984_INTENT_TX_TYPES[prebuildParams.type]) { - delete prebuildParams.recipients; + // The stored recipients merged above are display-only for the approval UI. For the ERC-7984 + // intent types the real recipients are derived server-side, so the rebuild forwards an empty + // array (the server-derived /tx/build shape, as with acceleration). Other PA types forward + // their stored recipients unchanged. + if (ERC7984_INTENT_TX_TYPES[prebuildParams.type] === true) { + prebuildParams.recipients = []; } if (!_.isUndefined(originalPrebuild.hopTransaction)) { diff --git a/modules/sdk-core/src/bitgo/wallet/BuildParams.ts b/modules/sdk-core/src/bitgo/wallet/BuildParams.ts index 3b1eb18aeb..332e88481e 100644 --- a/modules/sdk-core/src/bitgo/wallet/BuildParams.ts +++ b/modules/sdk-core/src/bitgo/wallet/BuildParams.ts @@ -4,6 +4,15 @@ import * as t from 'io-ts'; import { unifiedAddressPreference } from '@bitgo/public-types'; import { getCodecProperties } from '../utils/codecProps'; +// ERC-7984 intent tx types (ETH family): calldata is derived server-side from the intent, so +// callers never supply recipients - an empty array is the server-derived build shape. +export const ERC7984_INTENT_TX_TYPES: Record = { + wrapApprove: true, + wrap: true, + unwrap: true, + finalizeUnwrap: true, +}; + export const Bip322Message = t.type({ message: t.string, address: t.string, diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 97306290fd..f9cc163c8b 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -58,7 +58,7 @@ import { EcdsaMPCv2Utils, EcdsaUtils } from '../utils/tss/ecdsa'; import EddsaUtils, { EddsaMPCv2Utils } from '../utils/tss/eddsa'; import { RedpallasMPCv2Utils } from '../utils/tss/redpallas'; import { getTxRequestApiVersion, validateTxRequestApiVersion } from '../utils/txRequest'; -import { buildParamKeys, BuildParams } from './BuildParams'; +import { buildParamKeys, BuildParams, ERC7984_INTENT_TX_TYPES } from './BuildParams'; import { fetchRootKeychainForSafeChild, isSafeChildPublicOnlyKeychain, @@ -2262,6 +2262,12 @@ export class Wallet implements IWallet { } const extraParams = await this.baseCoin.getExtraPrebuildParams(Object.assign(params, { wallet: this })); Object.assign(whitelistedParams, extraParams); + // ERC-7984 intent builds derive recipients server-side; /tx/build requires the key (empty + // array). Default on the outgoing body only - the caller's params flow into + // prebuildAndSignTransaction, where a truthy recipients trips the prebuildTx check. + if (params.type !== undefined && ERC7984_INTENT_TX_TYPES[params.type] === true) { + whitelistedParams.recipients = whitelistedParams.recipients ?? []; + } const queryParams = { offlineVerification: params.offlineVerification ? true : undefined, }; @@ -2780,8 +2786,12 @@ export class Wallet implements IWallet { throw error; } + // ERC-7984 intent types derive recipients server-side, so an empty array is a valid + // server-derived shape here (mirroring acceleration) - only populated lists are rejected. + const isErc7984IntentType = params.type !== undefined && ERC7984_INTENT_TX_TYPES[params.type] === true; if ( params.recipients && + !(isErc7984IntentType && params.recipients.length === 0) && (params.type === 'fillNonce' || params.type === 'acceleration' || params.type === 'wrapApprove' ||