From c27392747fb4d1bedd6669dd04ae92031c3b74b9 Mon Sep 17 00:00:00 2001 From: Doddanna17 Date: Thu, 14 May 2026 15:10:30 +0000 Subject: [PATCH] fix(sdk-coin-hbar): merge same-account entries in buildTransferData to fix self-transfer buildTransferData() produced two separate protobuf entries for the same accountID on self-transfer (sender == recipient), causing Hedera to reject with ACCOUNT_REPEATED_IN_ACCOUNT_AMOUNTS (status 74). Now merges entries by accountID so self-transfer yields a single [{acct, 0}] entry. Also updates transaction.ts getTransferData() fallback to handle zero-amount entries from the merged format (!isNegative instead of isPositive). Ticket: SI-539 --- .../src/lib/coinTransferBuilder.ts | 16 +++- modules/sdk-coin-hbar/src/lib/transaction.ts | 4 +- .../transactionBuilder/coinTransferBuilder.ts | 86 +++++++++++++------ 3 files changed, 80 insertions(+), 26 deletions(-) diff --git a/modules/sdk-coin-hbar/src/lib/coinTransferBuilder.ts b/modules/sdk-coin-hbar/src/lib/coinTransferBuilder.ts index 9491a4073a..11394d0929 100644 --- a/modules/sdk-coin-hbar/src/lib/coinTransferBuilder.ts +++ b/modules/sdk-coin-hbar/src/lib/coinTransferBuilder.ts @@ -41,8 +41,22 @@ export class CoinTransferBuilder extends TransferBuilder { }); accountAmounts[0].amount = Long.fromString(totalSend.toString()).negate(); // update sender send amount + // Merge entries with the same accountID to prevent ACCOUNT_REPEATED_IN_ACCOUNT_AMOUNTS. + // This handles self-transfer (sender == recipient) by collapsing [{acct, -N}, {acct, +N}] + // into a single [{acct, 0}] entry that Hedera accepts. + const merged = new Map(); + for (const entry of accountAmounts) { + const key = stringifyAccountId(entry.accountID!); + const existing = merged.get(key); + if (existing) { + existing.amount = Long.fromValue(existing.amount!).add(Long.fromValue(entry.amount!)); + } else { + merged.set(key, { accountID: entry.accountID, amount: entry.amount }); + } + } + return { - accountAmounts: accountAmounts, + accountAmounts: Array.from(merged.values()), }; } diff --git a/modules/sdk-coin-hbar/src/lib/transaction.ts b/modules/sdk-coin-hbar/src/lib/transaction.ts index 7527d28a6e..d05103d6fd 100644 --- a/modules/sdk-coin-hbar/src/lib/transaction.ts +++ b/modules/sdk-coin-hbar/src/lib/transaction.ts @@ -155,8 +155,10 @@ export class Transaction extends BaseTransaction { // Handle self-transfer: when sender == recipient, the positive entry is filtered out above // because its accountID matches the sender. Fall back to including it as the recipient. + // Use !isNegative() instead of isPositive() to also match zero-amount entries produced by + // merged self-transfers (e.g., [{accountID, amount: 0}] from buildTransferData merge). if (transferData.length === 0 && transfers.length > 0) { - const selfTransferEntry = transfers.find((t) => Long.fromValue(t.amount!).isPositive()); + const selfTransferEntry = transfers.find((t) => !Long.fromValue(t.amount!).isNegative()); if (selfTransferEntry) { transferData.push({ address: stringifyAccountId(selfTransferEntry.accountID!), diff --git a/modules/sdk-coin-hbar/test/unit/transactionBuilder/coinTransferBuilder.ts b/modules/sdk-coin-hbar/test/unit/transactionBuilder/coinTransferBuilder.ts index 93d4c34b71..f2b9bf148d 100644 --- a/modules/sdk-coin-hbar/test/unit/transactionBuilder/coinTransferBuilder.ts +++ b/modules/sdk-coin-hbar/test/unit/transactionBuilder/coinTransferBuilder.ts @@ -11,14 +11,12 @@ import * as testData from '../../resources/hbar'; * account. staking-service uses this as the CLAIM_REWARDS operation. * * The key behaviour under test: - * buildTransferData() must produce TWO separate accountAmounts entries: - * [{accountId, -1}, {accountId, +1}] - * The Hedera SDK TransferTransaction merges same-account entries (nets to 0), - * so CoinTransferBuilder.buildTransferData() bypasses that by building the - * proto list directly. + * buildTransferData() merges same-account entries to produce a SINGLE + * accountAmounts entry: [{accountId, 0}] (net of -1 + 1). + * This avoids Hedera's ACCOUNT_REPEATED_IN_ACCOUNT_AMOUNTS rejection. * - * initTransfers() must reconstruct the self-transfer from serialised bytes: - * it filters for positive amounts only, so recipients[0].address == source. + * getTransferData() (in transaction.ts) handles zero-amount entries via + * its self-transfer fallback, so toJson() correctly reports the recipient. */ describe('HBAR CoinTransferBuilder - self-transfer (stakeClaimRewards)', () => { const factory = getBuilderFactory('thbar'); @@ -43,37 +41,28 @@ describe('HBAR CoinTransferBuilder - self-transfer (stakeClaimRewards)', () => { // Source and recipient are the same account should.deepEqual(txJson.from, SOURCE); should.deepEqual(txJson.to, SOURCE); - should.deepEqual(txJson.amount, '1'); // inputs and outputs both reference the same address tx.inputs.length.should.equal(1); tx.inputs[0].address.should.equal(SOURCE); - tx.inputs[0].value.should.equal('1'); tx.outputs.length.should.equal(1); tx.outputs[0].address.should.equal(SOURCE); - tx.outputs[0].value.should.equal('1'); }); - it('should produce two separate accountAmounts entries in the protobuf', async () => { + it('should produce a single merged accountAmounts entry in the protobuf', async () => { const tx = await initSelfTransferBuilder().build(); // Access the raw protobuf transfer list const transfers = (tx as any).txBody.cryptoTransfer.transfers.accountAmounts as any[]; should.exist(transfers); - transfers.length.should.equal(2, 'expected exactly two entries: [{source,-1},{source,+1}]'); + transfers.length.should.equal(1, 'expected single merged entry: [{source, 0}]'); - // Both entries reference the same account - const accountNums = transfers.map((a: any) => { - const id = a.accountID; - return `${id.shardNum || 0}.${id.realmNum || 0}.${id.accountNum}`; - }); - accountNums.every((id: string) => id === SOURCE || id.endsWith('.81320')).should.be.true(); - - // One entry is -1 (debit), one is +1 (credit) - const amounts = transfers.map((a: any) => Number(a.amount.toString())); - amounts.should.containEql(-1); - amounts.should.containEql(1); + // The single entry references the source account with net-zero amount + const id = transfers[0].accountID; + const accountId = `${id.shardNum || 0}.${id.realmNum || 0}.${id.accountNum}`; + (accountId === SOURCE || accountId.endsWith('.81320')).should.be.true(); + Number(transfers[0].amount.toString()).should.equal(0); }); it('should round-trip through serialisation: deserialized tx has source == recipient', async () => { @@ -91,7 +80,6 @@ describe('HBAR CoinTransferBuilder - self-transfer (stakeClaimRewards)', () => { // After round-trip, source and recipient must still be the same account should.deepEqual(rebuiltJson.from, SOURCE); should.deepEqual(rebuiltJson.to, SOURCE); - should.deepEqual(rebuiltJson.amount, '1'); }); it('should sign a self-transfer transaction successfully', async () => { @@ -104,5 +92,55 @@ describe('HBAR CoinTransferBuilder - self-transfer (stakeClaimRewards)', () => { should.deepEqual(txJson.from, SOURCE); should.deepEqual(txJson.to, SOURCE); }); + + it('should not merge entries for normal transfers (sender != recipient)', async () => { + const RECIPIENT = testData.ACCOUNT_2.accountId; // '0.0.75861' + const txBuilder = factory.getTransferBuilder(); + txBuilder.fee({ fee: testData.FEE }); + txBuilder.source({ address: SOURCE }); + txBuilder.send({ address: RECIPIENT, amount: '100' }); + txBuilder.node({ nodeId: '0.0.3' }); + txBuilder.startTime('1596110493.372646570'); + const tx = await txBuilder.build(); + + // Normal transfer: two distinct entries (sender and recipient) + const transfers = (tx as any).txBody.cryptoTransfer.transfers.accountAmounts as any[]; + should.exist(transfers); + transfers.length.should.equal(2, 'expected two entries for normal transfer'); + + const amounts = transfers.map((a: any) => Number(a.amount.toString())); + amounts.should.containEql(-100); + amounts.should.containEql(100); + }); + + it('should merge sender entry when sender is also a recipient in multi-recipient transfer', async () => { + const OTHER = testData.ACCOUNT_3.accountId; // '0.0.78963' + const txBuilder = factory.getTransferBuilder(); + txBuilder.fee({ fee: testData.FEE }); + txBuilder.source({ address: SOURCE }); + txBuilder.send({ address: OTHER, amount: '100' }); + txBuilder.send({ address: SOURCE, amount: '50' }); // sender is also a recipient + txBuilder.node({ nodeId: '0.0.3' }); + txBuilder.startTime('1596110493.372646570'); + const tx = await txBuilder.build(); + + // Sender 0.0.81320 appears as both sender (-150) and recipient (+50). + // Merge collapses to net -100. Without merge, Hedera rejects with + // ACCOUNT_REPEATED_IN_ACCOUNT_AMOUNTS. + const transfers = (tx as any).txBody.cryptoTransfer.transfers.accountAmounts as any[]; + should.exist(transfers); + transfers.length.should.equal(2, 'expected two entries after merging sender with self-recipient'); + + const entryByAccount: Record = {}; + transfers.forEach((a: any) => { + const id = `${a.accountID.shardNum || 0}.${a.accountID.realmNum || 0}.${a.accountID.accountNum}`; + entryByAccount[id] = Number(a.amount.toString()); + }); + + // SOURCE: -150 (total send) + 50 (self-recipient) = net -100 + entryByAccount[SOURCE].should.equal(-100); + // OTHER: +100 (unchanged) + entryByAccount[OTHER].should.equal(100); + }); }); });