From 9a4ecf53f6506e725b8b983e03617b5c4eb5b610 Mon Sep 17 00:00:00 2001 From: bitgobot Date: Wed, 14 Oct 2026 10:20:30 +0000 Subject: [PATCH 1/2] feat(sdk-coin-icp): allow account consolidations sdk-core's wallet.buildAccountConsolidations, sendAccountConsolidation and sendAccountConsolidations reject every coin whose base coin does not override allowsAccountConsolidations() to true. sdk-coin-icp inherited the false default, so bitgo-retail's consolidate flow fails for ICP before any transaction is built and V2 consolidation from the UI cannot work. Override the flag to true so receive-address -> root consolidations can be initiated. The platform enforces V2-only consolidation eligibility (CECHO-2443). Ticket: CECHO-2439 Session-Id: 84ade726-5312-4794-ba4c-fa887b5aabc5 Task-Id: cabd6df4-ceda-4d9d-abc8-8b4945964ba0 --- modules/sdk-coin-icp/src/icp.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/modules/sdk-coin-icp/src/icp.ts b/modules/sdk-coin-icp/src/icp.ts index 09576760969..3f8e2f8cf74 100644 --- a/modules/sdk-coin-icp/src/icp.ts +++ b/modules/sdk-coin-icp/src/icp.ts @@ -256,6 +256,14 @@ export class Icp extends BaseCoin { return multisigTypes.tss; } + /** + * Account consolidations (receive address -> root address) are supported; + * the platform enforces V2-only consolidations (CECHO-2443). + */ + allowsAccountConsolidations(): boolean { + return true; + } + /** @inheritDoc */ getMPCAlgorithm(): MPCAlgorithm { return 'ecdsa'; From 1f1931cb511e09ab379882bbeff64ea13b499b97 Mon Sep 17 00:00:00 2001 From: bitgobot Date: Wed, 14 Oct 2026 10:20:38 +0000 Subject: [PATCH 2/2] test(sdk-coin-icp): pin V2 derived-address and bare-address behavior The V2 (non-memoId) wallet contract and the builder address pass-through were only covered at derivation index 0 with bare test vectors, so a regression in index handling or memo-suffix stripping could ship unnoticed ahead of the V2 rollout (CECHO-2445). Derive the index-1 receive address from the existing commonKeychain fixture exactly as deriveMPCWalletAddress does (secp256k1 m/1, first 33 bytes) and pin the V2 contract: a derived address verifies only at its own index, and memo-suffixed addresses are never accepted. Also pin that builders pass sender/receiver strings through verbatim (the ?memoId=0 recipient survives unchanged), that an omitted memo defaults to 0, and that verifyTransaction accepts the derived->root consolidation prebuild shape sent by sendAccountConsolidation. Ticket: CECHO-2439 Session-Id: 84ade726-5312-4794-ba4c-fa887b5aabc5 Task-Id: cabd6df4-ceda-4d9d-abc8-8b4945964ba0 --- modules/sdk-coin-icp/test/unit/icp.ts | 106 +++++++++++++++++- modules/sdk-coin-icp/test/unit/transaction.ts | 12 ++ .../transactionBuilder/transactionBuilder.ts | 33 ++++++ 3 files changed, 150 insertions(+), 1 deletion(-) diff --git a/modules/sdk-coin-icp/test/unit/icp.ts b/modules/sdk-coin-icp/test/unit/icp.ts index b44873664b6..b4c7d50df56 100644 --- a/modules/sdk-coin-icp/test/unit/icp.ts +++ b/modules/sdk-coin-icp/test/unit/icp.ts @@ -1,5 +1,6 @@ import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; +import { Ecdsa } from '@bitgo/sdk-core'; import utils from '../../src/lib/utils'; import { getBuilderFactory } from './getBuilderFactory'; @@ -49,6 +50,7 @@ describe('Internet computer', function () { icp.getFullName().should.equal('Internet Computer'); icp.getBaseFactor().should.equal(1e8); icp.supportsTss().should.equal(true); + icp.allowsAccountConsolidations().should.equal(true); ticp.getChain().should.equal('ticp'); ticp.getFamily().should.equal('icp'); @@ -259,6 +261,38 @@ describe('Internet computer', function () { }); assert(response); }); + + it('should successfully verify a derived-to-root consolidation prebuild', async () => { + // Consolidation prebuild shape (sendAccountConsolidation): single output + // from a derived receive address (sender) to the bare root recipient. + const consolidationTxBuilder = factory.getTransferBuilder(); + consolidationTxBuilder.sender(testData.Accounts.account1.address, testData.Accounts.account1.publicKey); + consolidationTxBuilder.receiverId(testData.Accounts.account2.address); + consolidationTxBuilder.amount('10'); + await consolidationTxBuilder.build(); + + const payloadsData = consolidationTxBuilder.transaction.payloadsData; + const serializedTxFormat = { + serializedTxHex: payloadsData, + publicKey: testData.Accounts.account1.publicKey, + }; + const serializedTxHex = Buffer.from(JSON.stringify(serializedTxFormat), 'utf-8').toString('hex'); + const txParams = { + recipients: [ + { + address: testData.Accounts.account2.address, + amount: '10', + }, + ], + }; + const response = await basecoin.verifyTransaction({ + txPrebuild: { + txHex: serializedTxHex, + }, + txParams: txParams, + }); + assert(response); + }); }); describe('Address Verification', () => { @@ -268,6 +302,19 @@ describe('Internet computer', function () { rootAddress: 'fd3eaed3e2064bd30ab497e22e8ac5a0dcadd81fa5353879dbab64e259ec70c0', }; + // V2 (HD-derived) receive address at index 1, derived exactly the way + // verifyMPCWalletAddress/deriveMPCWalletAddress derives it: secp256k1 path + // 'm/1', first 33 bytes of the derived public key. + let derivedAddressAtIndex1: string; + + before(function () { + derivedAddressAtIndex1 = utils.getAddressFromPublicKey( + Buffer.from(new Ecdsa().deriveUnhardened(addressVerificationData.commonKeychain, 'm/1'), 'hex') + .subarray(0, 33) + .toString('hex') + ); + }); + describe('Wallet VersionKey 1', () => { let keychains; @@ -430,7 +477,7 @@ describe('Internet computer', function () { }); }); - describe('Wallet VersionKey 2+', () => { + describe('V2 non-memoId wallets', () => { let keychains; before(function () { @@ -492,6 +539,63 @@ describe('Internet computer', function () { const result = await basecoin.isWalletAddress(params); result.should.equal(true); }); + + it('should verify the V2 root address at index 0', async function () { + const params = { + address: addressVerificationData.rootAddress, + rootAddress: addressVerificationData.rootAddress, + keychains: keychains, + index: 0, + walletVersion: 2, + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(true); + }); + + it('should verify a derived address at its own index', async function () { + const params = { + address: derivedAddressAtIndex1, + rootAddress: addressVerificationData.rootAddress, + keychains: keychains, + index: 1, + walletVersion: 2, + }; + + const result = await basecoin.isWalletAddress(params); + result.should.equal(true); + }); + + it('should reject a derived address checked against a different index', async function () { + const params = { + address: derivedAddressAtIndex1, + rootAddress: addressVerificationData.rootAddress, + keychains: keychains, + index: 0, + walletVersion: 2, + }; + + await basecoin + .isWalletAddress(params) + .should.be.rejectedWith( + `address validation failure: address ${derivedAddressAtIndex1} is not a wallet address` + ); + }); + + it('should reject memo-suffixed addresses for V2 non-memoId wallets', async function () { + const addressWithMemo = `${addressVerificationData.rootAddress}?memoId=5`; + const params = { + address: addressWithMemo, + rootAddress: addressVerificationData.rootAddress, + keychains: keychains, + index: 0, + walletVersion: 2, + }; + + await basecoin + .isWalletAddress(params) + .should.be.rejectedWith(`address validation failure: address ${addressWithMemo} is not a wallet address`); + }); }); describe('Address validation', () => { diff --git a/modules/sdk-coin-icp/test/unit/transaction.ts b/modules/sdk-coin-icp/test/unit/transaction.ts index b92217c99d1..5be4b560ff1 100644 --- a/modules/sdk-coin-icp/test/unit/transaction.ts +++ b/modules/sdk-coin-icp/test/unit/transaction.ts @@ -60,6 +60,18 @@ describe('ICP Transaction', () => { payloadsData.payloads.length.should.equal(1); }); + it('should keep a memo-suffixed receiver address verbatim in the transfer operations', async () => { + const factory = getBuilderFactory('ticp'); + const txBuilder = factory.getTransferBuilder(); + txBuilder.sender(testData.Accounts.account1.address, testData.Accounts.account1.publicKey); + txBuilder.receiverId(testData.Accounts.account6.address); + txBuilder.amount('10'); + await txBuilder.build(); + + const icpTransaction = txBuilder.transaction.icpTransaction; + should.equal(icpTransaction.operations[1].account.address, testData.Accounts.account6.address); + }); + it('build a json transaction from raw hex', async () => { await tx.fromRawTransaction(serializedTxHex); const json = tx.toJson(); diff --git a/modules/sdk-coin-icp/test/unit/transactionBuilder/transactionBuilder.ts b/modules/sdk-coin-icp/test/unit/transactionBuilder/transactionBuilder.ts index 22d4ea4c768..eaeee063c8b 100644 --- a/modules/sdk-coin-icp/test/unit/transactionBuilder/transactionBuilder.ts +++ b/modules/sdk-coin-icp/test/unit/transactionBuilder/transactionBuilder.ts @@ -266,3 +266,36 @@ describe('ICP Transaction Builder with default memo as 0', async () => { signedTxn.should.be.a.String(); }); }); + +describe('ICP Transaction Builder with bare (non-memo) addresses', async () => { + const factory = getBuilderFactory('ticp'); + + beforeEach(async function () { + // Echo back the memo the builder passes in so `metadata.memo` reflects the + // DEFAULT_MEMO fill-in in TransferBuilder.buildImplementation. + sinon.stub(utils, 'getMetaData').callsFake((memo) => ({ + metaData: { ...testData.MetaDataWithDefaultMemo, memo: Number(memo) }, + ingressEndTime: testData.MetaDataWithDefaultMemo.ingress_end ?? 0, + })); + sinon.stub(utils, 'validateExpireTime').returns(true); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('should keep bare sender and receiver addresses verbatim and default the memo to 0', async () => { + const txBuilder = factory.getTransferBuilder(); + txBuilder.sender(testData.Accounts.account1.address, testData.Accounts.account1.publicKey); + txBuilder.receiverId(testData.Accounts.account2.address); + txBuilder.amount('10'); + + await txBuilder.build(); + + const icpTransaction = txBuilder.transaction.icpTransaction; + should.equal(icpTransaction.operations[0].account.address, testData.Accounts.account1.address); + should.equal(icpTransaction.operations[2].account.address, testData.Accounts.account1.address); + should.equal(icpTransaction.operations[1].account.address, testData.Accounts.account2.address); + should.equal(icpTransaction.metadata.memo, DEFAULT_MEMO); + }); +});