From ac1e04ca18e7e9c301703a4c286a8b105a282ef7 Mon Sep 17 00:00:00 2001 From: Derran Wijesinghe Date: Tue, 3 Mar 2026 11:34:58 -0500 Subject: [PATCH 1/5] =?UTF-8?q?feat(sdk-core):=20pass=20server=20buildPara?= =?UTF-8?q?ms=20through=20prebuild=E2=86=92sign=E2=86=92send=20pipeline=20?= =?UTF-8?q?for=20UTXO=20coins?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preserve server-validated buildParams from /tx/build response and forward them in the /tx/send request body, enabling platform intent verification for UTXO coins (BTC, LTC, BCH, DOGE, ZEC). WP-8015 TICKET: WP-8015 --- modules/bitgo/test/v2/unit/wallet.ts | 105 ++++++++++++++++++++ modules/sdk-core/src/bitgo/wallet/wallet.ts | 15 ++- 2 files changed, 117 insertions(+), 3 deletions(-) diff --git a/modules/bitgo/test/v2/unit/wallet.ts b/modules/bitgo/test/v2/unit/wallet.ts index 047c82063d..070331aeab 100644 --- a/modules/bitgo/test/v2/unit/wallet.ts +++ b/modules/bitgo/test/v2/unit/wallet.ts @@ -1848,6 +1848,111 @@ describe('V2 Wallet:', function () { response.isDone().should.be.true(); }); + + it('should preserve server-returned buildParams on prebuild result', async function () { + const serverBuildParams = { + recipients: [{ address: 'addr1', amount: '5000' }], + feeRate: 10000, + }; + const params = { offlineVerification: true }; + nock(bgUrl) + .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/build`, tbtcHotWalletDefaultParams) + .query(params) + .reply(200, { buildParams: serverBuildParams }); + const blockHeight = 100; + sinon.stub(basecoin, 'getLatestBlockHeight').resolves(blockHeight); + sinon.stub(basecoin, 'postProcessPrebuild').callsFake((params) => Promise.resolve(params)); + const txPrebuild = await wallet.prebuildTransaction({ ...params, reqId: reqId }); + txPrebuild.buildParams.should.deepEqual(serverBuildParams); + }); + + it('should fall back to request params when server does not return buildParams', async function () { + const params = { offlineVerification: true }; + nock(bgUrl) + .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/build`, tbtcHotWalletDefaultParams) + .query(params) + .reply(200, {}); + const blockHeight = 100; + sinon.stub(basecoin, 'getLatestBlockHeight').resolves(blockHeight); + sinon.stub(basecoin, 'postProcessPrebuild').callsFake((params) => Promise.resolve(params)); + const txPrebuild = await wallet.prebuildTransaction({ ...params, reqId: reqId }); + txPrebuild.buildParams.should.deepEqual(tbtcHotWalletDefaultParams); + }); + }); + + describe('sendMany buildParams pass-through for UTXO coins', function () { + it('should include recipients from server buildParams in /tx/send request body', async function () { + const recipients = [{ address: 'addr1', amount: '5000' }]; + const serverBuildParams = { recipients, feeRate: 10000 }; + const prebuildReturn = { txHex: 'deadbeef', buildParams: serverBuildParams }; + sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); + + const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; + const sendNock = nock(bgUrl) + .post(path, _.matches({ txHex: 'deadbeef', recipients, feeRate: 10000 })) + .reply(200, { status: 'signed' }); + + const result = await wallet.sendMany({ recipients }); + sendNock.isDone().should.be.true(); + result.should.have.property('status', 'signed'); + }); + + it('should include RBF fields from server buildParams in /tx/send request body', async function () { + const recipients = [{ address: 'addr1', amount: '5000' }]; + const serverBuildParams = { + recipients, + rbfTxIds: ['txid1'], + maxFee: 50000, + feeMultiplier: 2, + }; + const prebuildReturn = { txHex: 'deadbeef', buildParams: serverBuildParams }; + sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); + + const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; + const sendNock = nock(bgUrl) + .post(path, _.matches({ txHex: 'deadbeef', rbfTxIds: ['txid1'], maxFee: 50000, feeMultiplier: 2 })) + .reply(200, { status: 'signed' }); + + const result = await wallet.sendMany({ recipients }); + sendNock.isDone().should.be.true(); + result.should.have.property('status', 'signed'); + }); + + it('should work when server returns no buildParams in build response', async function () { + const recipients = [{ address: 'addr1', amount: '5000' }]; + const prebuildReturn = { txHex: 'deadbeef' }; + sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); + + const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; + const sendNock = nock(bgUrl) + .post(path, _.matches({ txHex: 'deadbeef' })) + .reply(200, { status: 'signed' }); + + const result = await wallet.sendMany({ recipients }); + sendNock.isDone().should.be.true(); + result.should.have.property('status', 'signed'); + }); + + it('should let explicit user params override server buildParams', async function () { + const userRecipients = [{ address: 'user-addr', amount: '9999' }]; + const serverRecipients = [{ address: 'server-addr', amount: '5000' }]; + const serverBuildParams = { recipients: serverRecipients, feeRate: 10000 }; + const prebuildReturn = { txHex: 'deadbeef', buildParams: serverBuildParams }; + sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); + + const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; + const sendNock = nock(bgUrl) + .post(path, _.matches({ txHex: 'deadbeef', recipients: userRecipients })) + .reply(200, { status: 'signed' }); + + const result = await wallet.sendMany({ recipients: userRecipients }); + sendNock.isDone().should.be.true(); + result.should.have.property('status', 'signed'); + }); }); describe('Maximum Spendable', function maximumSpendable() { diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 2e08856a2a..85f921819e 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -2021,6 +2021,7 @@ export class Wallet implements IWallet { if (!_.isUndefined(blockHeight)) { buildResponse.blockHeight = blockHeight; } + const serverBuildParams = buildResponse.buildParams; let prebuild: TransactionPrebuild = (await this.baseCoin.postProcessPrebuild( Object.assign(buildResponse, { wallet: this, @@ -2028,7 +2029,8 @@ export class Wallet implements IWallet { }) )) as any; delete prebuild.wallet; - delete prebuild.buildParams; + // Preserve buildParams: prefer server-validated; fall back to request params + prebuild.buildParams = serverBuildParams ?? whitelistedParams; prebuild = _.extend({}, prebuild, { walletId: this.id() }); if (this._wallet && this._wallet.coinSpecific && !params.walletContractAddress) { prebuild = _.extend({}, prebuild, { @@ -2503,7 +2505,11 @@ export class Wallet implements IWallet { } try { - return await this.signTransaction(signingParams); + const signedTransaction = await this.signTransaction(signingParams); + if (txPrebuild.buildParams) { + (signedTransaction as any).buildParams = txPrebuild.buildParams; + } + return signedTransaction; } catch (error) { if (error.message.includes('insufficient funds')) { error.code = 'insufficient_funds'; @@ -2887,8 +2893,11 @@ export class Wallet implements IWallet { } const halfSignedTransaction = await this.prebuildAndSignTransaction(params); + const { buildParams: prebuildBuildParams, ...signedTxFields } = halfSignedTransaction as any; const extraParams = await this.baseCoin.getExtraPrebuildParams(Object.assign(params, { wallet: this })); - const finalTxParams = _.extend({}, halfSignedTransaction, selectParams, extraParams); + // Merge order: signed tx fields, then server buildParams, then user selectParams, then extraParams. + // selectParams overrides buildParams so explicit user values take precedence. + const finalTxParams = _.extend({}, signedTxFields, prebuildBuildParams, selectParams, extraParams); return this.sendTransaction(finalTxParams, reqId); } From 5125153dd14db3e2d0952f5ad5ea0ddf6a87239e Mon Sep 17 00:00:00 2001 From: Derran Wijesinghe Date: Tue, 3 Mar 2026 11:52:08 -0500 Subject: [PATCH 2/5] fix(bitgo): add non-null assertions for buildParams in tests TypeScript strict checks require null guards before accessing the optional buildParams property on PrebuildTransactionResult. WP-8015 TICKET: WP-8015 --- modules/bitgo/test/v2/unit/wallet.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/bitgo/test/v2/unit/wallet.ts b/modules/bitgo/test/v2/unit/wallet.ts index 070331aeab..4a447337f1 100644 --- a/modules/bitgo/test/v2/unit/wallet.ts +++ b/modules/bitgo/test/v2/unit/wallet.ts @@ -1863,7 +1863,8 @@ describe('V2 Wallet:', function () { sinon.stub(basecoin, 'getLatestBlockHeight').resolves(blockHeight); sinon.stub(basecoin, 'postProcessPrebuild').callsFake((params) => Promise.resolve(params)); const txPrebuild = await wallet.prebuildTransaction({ ...params, reqId: reqId }); - txPrebuild.buildParams.should.deepEqual(serverBuildParams); + should.exist(txPrebuild.buildParams); + txPrebuild.buildParams!.should.deepEqual(serverBuildParams); }); it('should fall back to request params when server does not return buildParams', async function () { @@ -1876,7 +1877,8 @@ describe('V2 Wallet:', function () { sinon.stub(basecoin, 'getLatestBlockHeight').resolves(blockHeight); sinon.stub(basecoin, 'postProcessPrebuild').callsFake((params) => Promise.resolve(params)); const txPrebuild = await wallet.prebuildTransaction({ ...params, reqId: reqId }); - txPrebuild.buildParams.should.deepEqual(tbtcHotWalletDefaultParams); + should.exist(txPrebuild.buildParams); + txPrebuild.buildParams!.should.deepEqual(tbtcHotWalletDefaultParams); }); }); From ac58a01dfb0b11edaf46e0ba61874bc860b830e2 Mon Sep 17 00:00:00 2001 From: Derran Wijesinghe Date: Tue, 3 Mar 2026 12:12:30 -0500 Subject: [PATCH 3/5] fix(sdk-coin-eth): update test to expect buildParams on prebuild result prebuildTransaction now preserves buildParams on the result instead of deleting them, so update the ETH hop transaction test assertion from should.not.exist to should.exist. WP-8015 TICKET: WP-8015 --- modules/sdk-coin-eth/test/unit/ethWallet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/sdk-coin-eth/test/unit/ethWallet.ts b/modules/sdk-coin-eth/test/unit/ethWallet.ts index 3e36160e4c..f2b6990e47 100644 --- a/modules/sdk-coin-eth/test/unit/ethWallet.ts +++ b/modules/sdk-coin-eth/test/unit/ethWallet.ts @@ -282,7 +282,7 @@ describe('Ethereum Hop Transactions', function () { should.exist(res.hopTransaction.id); should.exist(res.hopTransaction.signature); should.not.exist(res.wallet); - should.not.exist(res.buildParams); + should.exist(res.buildParams); feeScope.isDone().should.equal(true); const feeReq = (feeScope as any).interceptors[0].req; feeReq.path.should.containEql('hop=true'); From e1c5a9aedfd7ade54cd8c47979ef10c37f7859f8 Mon Sep 17 00:00:00 2001 From: Derran Wijesinghe Date: Tue, 3 Mar 2026 20:07:25 -0500 Subject: [PATCH 4/5] test(bitgo): add account-based coin tests for buildParams pass-through Verify that recipients appears at the top level of /tx/send request body for account-based coins (ETH) alongside halfSigned, confirming WP-8014 is covered by the coin-agnostic WP-8015 changes. Tests cover: - recipients at top level with server buildParams (ETH halfSigned) - recipients at top level via selectParams without server buildParams - nonce and other fields from server buildParams at top level WP-8014 TICKET: WP-8015 --- modules/bitgo/test/v2/unit/wallet.ts | 111 +++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/modules/bitgo/test/v2/unit/wallet.ts b/modules/bitgo/test/v2/unit/wallet.ts index 4a447337f1..4688b85bd1 100644 --- a/modules/bitgo/test/v2/unit/wallet.ts +++ b/modules/bitgo/test/v2/unit/wallet.ts @@ -1957,6 +1957,117 @@ describe('V2 Wallet:', function () { }); }); + describe('sendMany buildParams pass-through for account-based coins', function () { + let ethBasecoin; + let ethWalletForBuildParams: Wallet; + + before(function () { + ethBasecoin = bitgo.coin('teth'); + ethWalletForBuildParams = new Wallet(bitgo, ethBasecoin, { + id: '598f606cd8fc24710d2ebadb1d9459bb', + coin: 'teth', + keys: [ + '598f606cd8fc24710d2ebad89dce86c2', + '598f606cc8e43aef09fcb785221d9dd2', + '5935d59cf660764331bafcade1855fd7', + ], + multisigType: 'onchain', + type: 'hot', + }); + }); + + it('should include top-level recipients alongside halfSigned in /tx/send for ETH', async function () { + const recipients = [{ address: '0x5208d8e80c6d1aef9be37b4bd19a9cf75ed93dc8', amount: '200000000000000' }]; + const serverBuildParams = { recipients }; + const prebuildReturn = { + halfSigned: { + recipients, + txHex: '0xdeadbeef', + expireTime: 1234567890, + contractSequenceId: 1001, + }, + buildParams: serverBuildParams, + }; + sinon.stub(ethWalletForBuildParams, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(ethBasecoin, 'getExtraPrebuildParams').resolves({}); + + const path = `/api/v2/${ethWalletForBuildParams.coin()}/wallet/${ethWalletForBuildParams.id()}/tx/send`; + const sendNock = nock(bgUrl) + .post(path, (body) => { + // recipients must be at top level (not just inside halfSigned) + return ( + body.recipients !== undefined && + body.recipients[0].address === recipients[0].address && + body.halfSigned !== undefined + ); + }) + .reply(200, { status: 'signed' }); + + const result = await ethWalletForBuildParams.sendMany({ recipients }); + sendNock.isDone().should.be.true(); + result.should.have.property('status', 'signed'); + }); + + it('should include top-level recipients from selectParams even without server buildParams for ETH', async function () { + const recipients = [{ address: '0x5208d8e80c6d1aef9be37b4bd19a9cf75ed93dc8', amount: '200000000000000' }]; + // No buildParams — simulates old server that doesn't return buildParams + const prebuildReturn = { + halfSigned: { + recipients, + txHex: '0xdeadbeef', + expireTime: 1234567890, + contractSequenceId: 1001, + }, + }; + sinon.stub(ethWalletForBuildParams, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(ethBasecoin, 'getExtraPrebuildParams').resolves({}); + + const path = `/api/v2/${ethWalletForBuildParams.coin()}/wallet/${ethWalletForBuildParams.id()}/tx/send`; + const sendNock = nock(bgUrl) + .post(path, (body) => { + // recipients should still be at top level via selectParams (user's original params) + return ( + body.recipients !== undefined && + body.recipients[0].address === recipients[0].address && + body.halfSigned !== undefined + ); + }) + .reply(200, { status: 'signed' }); + + const result = await ethWalletForBuildParams.sendMany({ recipients }); + sendNock.isDone().should.be.true(); + result.should.have.property('status', 'signed'); + }); + + it('should include eip1559 and nonce from server buildParams at top level for ETH', async function () { + const recipients = [{ address: '0x5208d8e80c6d1aef9be37b4bd19a9cf75ed93dc8', amount: '200000000000000' }]; + const eip1559 = { maxPriorityFeePerGas: '57500000', maxFeePerGas: '510214356' }; + const serverBuildParams = { recipients, nonce: 42, memo: { value: 'test' } }; + const prebuildReturn = { + halfSigned: { + recipients, + txHex: '0xdeadbeef', + eip1559, + contractSequenceId: 3371928, + }, + buildParams: serverBuildParams, + }; + sinon.stub(ethWalletForBuildParams, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(ethBasecoin, 'getExtraPrebuildParams').resolves({}); + + const path = `/api/v2/${ethWalletForBuildParams.coin()}/wallet/${ethWalletForBuildParams.id()}/tx/send`; + const sendNock = nock(bgUrl) + .post(path, (body) => { + return body.recipients !== undefined && body.halfSigned !== undefined && body.nonce === 42; + }) + .reply(200, { status: 'signed' }); + + const result = await ethWalletForBuildParams.sendMany({ recipients }); + sendNock.isDone().should.be.true(); + result.should.have.property('status', 'signed'); + }); + }); + describe('Maximum Spendable', function maximumSpendable() { let bgUrl; From b794c916e54d1b22e3c3a8fb81c545105b5d76dc Mon Sep 17 00:00:00 2001 From: Derran Wijesinghe Date: Wed, 4 Mar 2026 09:49:55 -0500 Subject: [PATCH 5/5] fix(sdk-core): revert server buildParams forwarding, keep user-param tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert the production changes that forwarded server-returned buildParams through the prebuild→sign→send pipeline. Per BitGo security architecture (mutual distrust principle), the SDK must not forward WP-provided data back to WP — a compromised WP could rewrite recipients. Recipients already appear at the top level of /tx/send via selectParams (the user's original params) for both UTXO and account-based coins. New tests verify this existing behavior. WP-8015, WP-8014 TICKET: WP-8015 --- modules/bitgo/test/v2/unit/wallet.ts | 182 +++----------------- modules/sdk-coin-eth/test/unit/ethWallet.ts | 2 +- modules/sdk-core/src/bitgo/wallet/wallet.ts | 15 +- 3 files changed, 31 insertions(+), 168 deletions(-) diff --git a/modules/bitgo/test/v2/unit/wallet.ts b/modules/bitgo/test/v2/unit/wallet.ts index 4688b85bd1..d3626ed057 100644 --- a/modules/bitgo/test/v2/unit/wallet.ts +++ b/modules/bitgo/test/v2/unit/wallet.ts @@ -1848,81 +1848,10 @@ describe('V2 Wallet:', function () { response.isDone().should.be.true(); }); - - it('should preserve server-returned buildParams on prebuild result', async function () { - const serverBuildParams = { - recipients: [{ address: 'addr1', amount: '5000' }], - feeRate: 10000, - }; - const params = { offlineVerification: true }; - nock(bgUrl) - .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/build`, tbtcHotWalletDefaultParams) - .query(params) - .reply(200, { buildParams: serverBuildParams }); - const blockHeight = 100; - sinon.stub(basecoin, 'getLatestBlockHeight').resolves(blockHeight); - sinon.stub(basecoin, 'postProcessPrebuild').callsFake((params) => Promise.resolve(params)); - const txPrebuild = await wallet.prebuildTransaction({ ...params, reqId: reqId }); - should.exist(txPrebuild.buildParams); - txPrebuild.buildParams!.should.deepEqual(serverBuildParams); - }); - - it('should fall back to request params when server does not return buildParams', async function () { - const params = { offlineVerification: true }; - nock(bgUrl) - .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/build`, tbtcHotWalletDefaultParams) - .query(params) - .reply(200, {}); - const blockHeight = 100; - sinon.stub(basecoin, 'getLatestBlockHeight').resolves(blockHeight); - sinon.stub(basecoin, 'postProcessPrebuild').callsFake((params) => Promise.resolve(params)); - const txPrebuild = await wallet.prebuildTransaction({ ...params, reqId: reqId }); - should.exist(txPrebuild.buildParams); - txPrebuild.buildParams!.should.deepEqual(tbtcHotWalletDefaultParams); - }); }); - describe('sendMany buildParams pass-through for UTXO coins', function () { - it('should include recipients from server buildParams in /tx/send request body', async function () { - const recipients = [{ address: 'addr1', amount: '5000' }]; - const serverBuildParams = { recipients, feeRate: 10000 }; - const prebuildReturn = { txHex: 'deadbeef', buildParams: serverBuildParams }; - sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); - sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); - - const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; - const sendNock = nock(bgUrl) - .post(path, _.matches({ txHex: 'deadbeef', recipients, feeRate: 10000 })) - .reply(200, { status: 'signed' }); - - const result = await wallet.sendMany({ recipients }); - sendNock.isDone().should.be.true(); - result.should.have.property('status', 'signed'); - }); - - it('should include RBF fields from server buildParams in /tx/send request body', async function () { - const recipients = [{ address: 'addr1', amount: '5000' }]; - const serverBuildParams = { - recipients, - rbfTxIds: ['txid1'], - maxFee: 50000, - feeMultiplier: 2, - }; - const prebuildReturn = { txHex: 'deadbeef', buildParams: serverBuildParams }; - sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); - sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); - - const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; - const sendNock = nock(bgUrl) - .post(path, _.matches({ txHex: 'deadbeef', rbfTxIds: ['txid1'], maxFee: 50000, feeMultiplier: 2 })) - .reply(200, { status: 'signed' }); - - const result = await wallet.sendMany({ recipients }); - sendNock.isDone().should.be.true(); - result.should.have.property('status', 'signed'); - }); - - it('should work when server returns no buildParams in build response', async function () { + describe('sendMany includes user-provided recipients at top level in /tx/send', function () { + it('should include top-level recipients from user params for UTXO coins', async function () { const recipients = [{ address: 'addr1', amount: '5000' }]; const prebuildReturn = { txHex: 'deadbeef' }; sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); @@ -1930,7 +1859,14 @@ describe('V2 Wallet:', function () { const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; const sendNock = nock(bgUrl) - .post(path, _.matches({ txHex: 'deadbeef' })) + .post(path, (body) => { + // recipients from user params must appear at top level via selectParams + return ( + body.recipients !== undefined && + body.recipients[0].address === recipients[0].address && + body.txHex === 'deadbeef' + ); + }) .reply(200, { status: 'signed' }); const result = await wallet.sendMany({ recipients }); @@ -1938,32 +1874,9 @@ describe('V2 Wallet:', function () { result.should.have.property('status', 'signed'); }); - it('should let explicit user params override server buildParams', async function () { - const userRecipients = [{ address: 'user-addr', amount: '9999' }]; - const serverRecipients = [{ address: 'server-addr', amount: '5000' }]; - const serverBuildParams = { recipients: serverRecipients, feeRate: 10000 }; - const prebuildReturn = { txHex: 'deadbeef', buildParams: serverBuildParams }; - sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); - sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); - - const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; - const sendNock = nock(bgUrl) - .post(path, _.matches({ txHex: 'deadbeef', recipients: userRecipients })) - .reply(200, { status: 'signed' }); - - const result = await wallet.sendMany({ recipients: userRecipients }); - sendNock.isDone().should.be.true(); - result.should.have.property('status', 'signed'); - }); - }); - - describe('sendMany buildParams pass-through for account-based coins', function () { - let ethBasecoin; - let ethWalletForBuildParams: Wallet; - - before(function () { - ethBasecoin = bitgo.coin('teth'); - ethWalletForBuildParams = new Wallet(bitgo, ethBasecoin, { + it('should include top-level recipients alongside halfSigned for account-based coins', async function () { + const ethBasecoin = bitgo.coin('teth'); + const ethWalletLocal = new Wallet(bitgo, ethBasecoin, { id: '598f606cd8fc24710d2ebadb1d9459bb', coin: 'teth', keys: [ @@ -1974,11 +1887,7 @@ describe('V2 Wallet:', function () { multisigType: 'onchain', type: 'hot', }); - }); - - it('should include top-level recipients alongside halfSigned in /tx/send for ETH', async function () { const recipients = [{ address: '0x5208d8e80c6d1aef9be37b4bd19a9cf75ed93dc8', amount: '200000000000000' }]; - const serverBuildParams = { recipients }; const prebuildReturn = { halfSigned: { recipients, @@ -1986,15 +1895,14 @@ describe('V2 Wallet:', function () { expireTime: 1234567890, contractSequenceId: 1001, }, - buildParams: serverBuildParams, }; - sinon.stub(ethWalletForBuildParams, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(ethWalletLocal, 'prebuildAndSignTransaction').resolves(prebuildReturn); sinon.stub(ethBasecoin, 'getExtraPrebuildParams').resolves({}); - const path = `/api/v2/${ethWalletForBuildParams.coin()}/wallet/${ethWalletForBuildParams.id()}/tx/send`; + const path = `/api/v2/${ethWalletLocal.coin()}/wallet/${ethWalletLocal.id()}/tx/send`; const sendNock = nock(bgUrl) .post(path, (body) => { - // recipients must be at top level (not just inside halfSigned) + // recipients from user params must be at top level (not just inside halfSigned) return ( body.recipients !== undefined && body.recipients[0].address === recipients[0].address && @@ -2003,66 +1911,30 @@ describe('V2 Wallet:', function () { }) .reply(200, { status: 'signed' }); - const result = await ethWalletForBuildParams.sendMany({ recipients }); + const result = await ethWalletLocal.sendMany({ recipients }); sendNock.isDone().should.be.true(); result.should.have.property('status', 'signed'); }); - it('should include top-level recipients from selectParams even without server buildParams for ETH', async function () { - const recipients = [{ address: '0x5208d8e80c6d1aef9be37b4bd19a9cf75ed93dc8', amount: '200000000000000' }]; - // No buildParams — simulates old server that doesn't return buildParams - const prebuildReturn = { - halfSigned: { - recipients, - txHex: '0xdeadbeef', - expireTime: 1234567890, - contractSequenceId: 1001, - }, - }; - sinon.stub(ethWalletForBuildParams, 'prebuildAndSignTransaction').resolves(prebuildReturn); - sinon.stub(ethBasecoin, 'getExtraPrebuildParams').resolves({}); + it('should not forward server-returned buildParams to /tx/send', async function () { + const userRecipients = [{ address: 'user-addr', amount: '9999' }]; + const prebuildReturn = { txHex: 'deadbeef' }; + sinon.stub(wallet, 'prebuildAndSignTransaction').resolves(prebuildReturn); + sinon.stub(basecoin, 'getExtraPrebuildParams').resolves({}); - const path = `/api/v2/${ethWalletForBuildParams.coin()}/wallet/${ethWalletForBuildParams.id()}/tx/send`; + const path = `/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`; const sendNock = nock(bgUrl) .post(path, (body) => { - // recipients should still be at top level via selectParams (user's original params) + // only user-provided recipients should appear, no server buildParams key return ( body.recipients !== undefined && - body.recipients[0].address === recipients[0].address && - body.halfSigned !== undefined + body.recipients[0].address === 'user-addr' && + body.buildParams === undefined ); }) .reply(200, { status: 'signed' }); - const result = await ethWalletForBuildParams.sendMany({ recipients }); - sendNock.isDone().should.be.true(); - result.should.have.property('status', 'signed'); - }); - - it('should include eip1559 and nonce from server buildParams at top level for ETH', async function () { - const recipients = [{ address: '0x5208d8e80c6d1aef9be37b4bd19a9cf75ed93dc8', amount: '200000000000000' }]; - const eip1559 = { maxPriorityFeePerGas: '57500000', maxFeePerGas: '510214356' }; - const serverBuildParams = { recipients, nonce: 42, memo: { value: 'test' } }; - const prebuildReturn = { - halfSigned: { - recipients, - txHex: '0xdeadbeef', - eip1559, - contractSequenceId: 3371928, - }, - buildParams: serverBuildParams, - }; - sinon.stub(ethWalletForBuildParams, 'prebuildAndSignTransaction').resolves(prebuildReturn); - sinon.stub(ethBasecoin, 'getExtraPrebuildParams').resolves({}); - - const path = `/api/v2/${ethWalletForBuildParams.coin()}/wallet/${ethWalletForBuildParams.id()}/tx/send`; - const sendNock = nock(bgUrl) - .post(path, (body) => { - return body.recipients !== undefined && body.halfSigned !== undefined && body.nonce === 42; - }) - .reply(200, { status: 'signed' }); - - const result = await ethWalletForBuildParams.sendMany({ recipients }); + const result = await wallet.sendMany({ recipients: userRecipients }); sendNock.isDone().should.be.true(); result.should.have.property('status', 'signed'); }); diff --git a/modules/sdk-coin-eth/test/unit/ethWallet.ts b/modules/sdk-coin-eth/test/unit/ethWallet.ts index f2b6990e47..3e36160e4c 100644 --- a/modules/sdk-coin-eth/test/unit/ethWallet.ts +++ b/modules/sdk-coin-eth/test/unit/ethWallet.ts @@ -282,7 +282,7 @@ describe('Ethereum Hop Transactions', function () { should.exist(res.hopTransaction.id); should.exist(res.hopTransaction.signature); should.not.exist(res.wallet); - should.exist(res.buildParams); + should.not.exist(res.buildParams); feeScope.isDone().should.equal(true); const feeReq = (feeScope as any).interceptors[0].req; feeReq.path.should.containEql('hop=true'); diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 85f921819e..2e08856a2a 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -2021,7 +2021,6 @@ export class Wallet implements IWallet { if (!_.isUndefined(blockHeight)) { buildResponse.blockHeight = blockHeight; } - const serverBuildParams = buildResponse.buildParams; let prebuild: TransactionPrebuild = (await this.baseCoin.postProcessPrebuild( Object.assign(buildResponse, { wallet: this, @@ -2029,8 +2028,7 @@ export class Wallet implements IWallet { }) )) as any; delete prebuild.wallet; - // Preserve buildParams: prefer server-validated; fall back to request params - prebuild.buildParams = serverBuildParams ?? whitelistedParams; + delete prebuild.buildParams; prebuild = _.extend({}, prebuild, { walletId: this.id() }); if (this._wallet && this._wallet.coinSpecific && !params.walletContractAddress) { prebuild = _.extend({}, prebuild, { @@ -2505,11 +2503,7 @@ export class Wallet implements IWallet { } try { - const signedTransaction = await this.signTransaction(signingParams); - if (txPrebuild.buildParams) { - (signedTransaction as any).buildParams = txPrebuild.buildParams; - } - return signedTransaction; + return await this.signTransaction(signingParams); } catch (error) { if (error.message.includes('insufficient funds')) { error.code = 'insufficient_funds'; @@ -2893,11 +2887,8 @@ export class Wallet implements IWallet { } const halfSignedTransaction = await this.prebuildAndSignTransaction(params); - const { buildParams: prebuildBuildParams, ...signedTxFields } = halfSignedTransaction as any; const extraParams = await this.baseCoin.getExtraPrebuildParams(Object.assign(params, { wallet: this })); - // Merge order: signed tx fields, then server buildParams, then user selectParams, then extraParams. - // selectParams overrides buildParams so explicit user values take precedence. - const finalTxParams = _.extend({}, signedTxFields, prebuildBuildParams, selectParams, extraParams); + const finalTxParams = _.extend({}, halfSignedTransaction, selectParams, extraParams); return this.sendTransaction(finalTxParams, reqId); }