Skip to content

Commit d0a28b1

Browse files
Merge pull request #8627 from BitGo/CSHLD-687-fix
fix(sdk-coin-sui): sponsored balanceWithdrawal transactions fail with…
2 parents 05f7289 + 154a7ab commit d0a28b1

3 files changed

Lines changed: 133 additions & 1 deletion

File tree

modules/sdk-coin-sui/src/lib/transferBuilder.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,12 +257,31 @@ export class TransferBuilder extends TransactionBuilder<TransferProgrammableTran
257257
typeArguments: ['0x2::sui::SUI'],
258258
arguments: [programmableTxBuilder.withdrawal({ amount: BigInt(this._fundsInAddressBalance.toFixed()) })],
259259
});
260+
// addrCoin is a command result (from redeem_funds) and must be explicitly consumed.
261+
// Split for every recipient — keeps a 1:1 SplitCoins↔TransferObjects structure for the
262+
// transaction parser. After all splits, consume the source coin explicitly:
263+
// - If there is change (fundsInAddressBalance > total recipient amount): return addrCoin
264+
// to sender via TransferObjects.
265+
// - If send-all (no change): addrCoin has 0 balance; destroy it by merging into the
266+
// sponsor's gas coin (a 0-value merge is valid in Sui and deletes the source object).
267+
const totalRecipientAmount = this._recipients.reduce((sum, r) => sum.plus(r.amount), new BigNumber(0));
268+
const hasChange = this._fundsInAddressBalance.gt(totalRecipientAmount);
269+
260270
this._recipients.forEach((recipient) => {
261271
const splitObject = programmableTxBuilder.splitCoins(addrCoin, [
262272
programmableTxBuilder.pure(Number(recipient.amount)),
263273
]);
264274
programmableTxBuilder.transferObjects([splitObject], programmableTxBuilder.object(recipient.address));
265275
});
276+
277+
if (hasChange) {
278+
// Return the remaining balance (change) to the sender.
279+
programmableTxBuilder.transferObjects([addrCoin], programmableTxBuilder.object(this._sender));
280+
} else {
281+
// Send-all: addrCoin has 0 balance after all splits. Merge it into the sponsor's gas
282+
// coin to destroy the zero-balance object (coin::join accepts a 0-value source).
283+
programmableTxBuilder.mergeCoins(programmableTxBuilder.gas, [addrCoin]);
284+
}
266285
const txData1b = programmableTxBuilder.blockData;
267286
return {
268287
type: this._type,

modules/sdk-coin-sui/src/lib/utils.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,11 @@ export class Utils implements BaseUtils {
288288
destinations.push(this.getAddress(input));
289289
}
290290
});
291-
destinations.map((address, i) => {
291+
// In Path 1b (sponsored, address-balance-only), after all recipient SplitCoins/TransferObjects
292+
// there may be one extra TransferObjects that returns change to the sender. That transfer
293+
// has no corresponding SplitCoins entry, so destinations.length may be splitResults.length+1.
294+
// Limit the zip to splitResults.length so the change transfer is not counted as a recipient.
295+
destinations.slice(0, splitResults.length).map((address, i) => {
292296
receipts.push({
293297
address: address,
294298
amount: splitResults[i].toString(),

modules/sdk-coin-sui/test/unit/transactionBuilder/transferBuilder.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,115 @@ describe('Sui Transfer Builder', () => {
440440
rebuiltTx.toBroadcastFormat().should.equal(rawTx);
441441
});
442442

443+
it('should build Path 1b send-all: sponsored addr-balance-only, no change (MergeCoins consumes addrCoin)', async function () {
444+
// Reproduces the real on-chain failure: UnusedValueWithoutDrop { result_idx: 0 }
445+
// Occurs when redeem_funds returns addrCoin, SplitCoins drains it completely, and the
446+
// 0-balance source coin is never consumed. Fix: MergeCoins(gas, [addrCoin]) at the end.
447+
const SEND_AMOUNT = '1000000'; // 0.001 SUI — same as the failing tx
448+
const sponsoredGasData = {
449+
...testData.gasData,
450+
owner: testData.feePayer.address,
451+
};
452+
453+
const txBuilder = factory.getTransferBuilder();
454+
txBuilder.type(SuiTransactionType.Transfer);
455+
txBuilder.sender(testData.sender.address);
456+
txBuilder.send([{ address: testData.recipients[0].address, amount: SEND_AMOUNT }]);
457+
txBuilder.gasData(sponsoredGasData);
458+
txBuilder.fundsInAddressBalance(SEND_AMOUNT); // send-all: balance == recipient amount
459+
460+
const tx = await txBuilder.build();
461+
should.equal(tx.type, TransactionType.Send);
462+
463+
const suiTx = tx as SuiTransaction<TransferProgrammableTransaction>;
464+
const cmds = suiTx.suiTransaction.tx.transactions as any[];
465+
466+
// Expected command sequence for Path 1b send-all:
467+
// 0: MoveCall (redeem_funds) — withdraw addrCoin
468+
// 1: SplitCoins(addrCoin) — split recipient amount off addrCoin
469+
// 2: TransferObjects([split]) — send to recipient
470+
// 3: MergeCoins(gas, [addrCoin]) — consume the now-zero-balance addrCoin
471+
cmds[0].kind.should.equal('MoveCall');
472+
cmds[0].target.should.equal('0x2::coin::redeem_funds');
473+
cmds[1].kind.should.equal('SplitCoins');
474+
cmds[2].kind.should.equal('TransferObjects');
475+
cmds[3].kind.should.equal('MergeCoins', 'expected MergeCoins to consume 0-balance addrCoin after send-all');
476+
477+
// Recipient parsing must not be affected by the trailing MergeCoins
478+
const recipients = utils.getRecipients(suiTx.suiTransaction);
479+
recipients.length.should.equal(1);
480+
recipients[0].address.should.equal(testData.recipients[0].address);
481+
recipients[0].amount.should.equal(SEND_AMOUNT);
482+
483+
// Round-trip
484+
const rawTx = tx.toBroadcastFormat();
485+
should.equal(utils.isValidRawTransaction(rawTx), true);
486+
const rebuilder = factory.from(rawTx);
487+
rebuilder.addSignature({ pub: testData.sender.publicKey }, Buffer.from(testData.sender.signatureHex));
488+
const rebuiltTx = await rebuilder.build();
489+
rebuiltTx.toBroadcastFormat().should.equal(rawTx);
490+
});
491+
492+
it('should build Path 1b with change: sponsored addr-balance-only, excess returned to sender', async function () {
493+
// fundsInAddressBalance exceeds the total recipient amount → change must be returned to
494+
// the sender as an extra TransferObjects([addrCoin], sender). The transaction parser must
495+
// skip this change transfer and only report actual recipients.
496+
const SEND_AMOUNT = '100'; // each of the two testData.recipients gets 100
497+
const EXCESS = '9999900'; // fundsInAddressBalance = 10_000_000, total send = 200
498+
const FUNDS_BALANCE = (Number(SEND_AMOUNT) * testData.recipients.length + Number(EXCESS)).toString();
499+
500+
const sponsoredGasData = {
501+
...testData.gasData,
502+
owner: testData.feePayer.address,
503+
};
504+
505+
const txBuilder = factory.getTransferBuilder();
506+
txBuilder.type(SuiTransactionType.Transfer);
507+
txBuilder.sender(testData.sender.address);
508+
txBuilder.send(testData.recipients); // 2 recipients × 100 MIST = 200 MIST total
509+
txBuilder.gasData(sponsoredGasData);
510+
txBuilder.fundsInAddressBalance(FUNDS_BALANCE); // 10_000_000 > 200 → has change
511+
512+
const tx = await txBuilder.build();
513+
should.equal(tx.type, TransactionType.Send);
514+
515+
const suiTx = tx as SuiTransaction<TransferProgrammableTransaction>;
516+
const cmds = suiTx.suiTransaction.tx.transactions as any[];
517+
518+
// Expected sequence for Path 1b with change (2 recipients):
519+
// 0: MoveCall (redeem_funds)
520+
// 1: SplitCoins — recipient 0
521+
// 2: TransferObjects — recipient 0
522+
// 3: SplitCoins — recipient 1
523+
// 4: TransferObjects — recipient 1
524+
// 5: TransferObjects([addrCoin], sender) — change back to sender
525+
cmds[0].kind.should.equal('MoveCall');
526+
cmds[0].target.should.equal('0x2::coin::redeem_funds');
527+
const lastCmd = cmds[cmds.length - 1];
528+
lastCmd.kind.should.equal('TransferObjects', 'last command must be the change transfer');
529+
530+
// The last TransferObjects returns change to the *sender*, not a recipient
531+
const changeAddrInput = suiTx.suiTransaction.tx.inputs[lastCmd.address.index] as any;
532+
const changeAddr = utils.getAddress(changeAddrInput);
533+
changeAddr.should.equal(testData.sender.address, 'change must go back to sender');
534+
535+
// Parser must return only the actual recipients, not the change transfer
536+
const recipients = utils.getRecipients(suiTx.suiTransaction);
537+
recipients.length.should.equal(testData.recipients.length);
538+
recipients[0].address.should.equal(testData.recipients[0].address);
539+
recipients[0].amount.should.equal(SEND_AMOUNT);
540+
recipients[1].address.should.equal(testData.recipients[1].address);
541+
recipients[1].amount.should.equal(SEND_AMOUNT);
542+
543+
// Round-trip
544+
const rawTx = tx.toBroadcastFormat();
545+
should.equal(utils.isValidRawTransaction(rawTx), true);
546+
const rebuilder = factory.from(rawTx);
547+
rebuilder.addSignature({ pub: testData.sender.publicKey }, Buffer.from(testData.sender.signatureHex));
548+
const rebuiltTx = await rebuilder.build();
549+
rebuiltTx.toBroadcastFormat().should.equal(rawTx);
550+
});
551+
443552
it('should build a sponsored tx gas paid from sponsor address balance (empty payment)', async function () {
444553
const inputObjects = testData.generateObjects(1);
445554
const sponsoredGasDataNoPayment = {

0 commit comments

Comments
 (0)