diff --git a/modules/sdk-coin-canton/src/lib/allocationAllocateBuilder.ts b/modules/sdk-coin-canton/src/lib/allocationAllocateBuilder.ts index efe4691081..045be87ee6 100644 --- a/modules/sdk-coin-canton/src/lib/allocationAllocateBuilder.ts +++ b/modules/sdk-coin-canton/src/lib/allocationAllocateBuilder.ts @@ -11,6 +11,13 @@ import { import utils from './utils'; import { normalizeCantonAmount } from './amount'; +/** + * Error for an amount that cannot fund an allocation. Zero only becomes valid for a receive-only + * allocation, so the message names the escape hatch that unlocks it. + */ +const AMOUNT_ERROR_MESSAGE = + "amount must be a positive number; zero is only allowed for a receive-only allocation, so call allocationRole('receive')"; + export class AllocationAllocateBuilder extends TransactionBuilder { private _commandId: string; private _amount: CantonAmount; @@ -77,7 +84,7 @@ export class AllocationAllocateBuilder extends TransactionBuilder { /** * Sets the role of this allocation. A 'receive' allocation is an unfunded, receive-only V2 allocation - * (cross-issuer trades): it locks nothing, so its amount is 0. Call before amount(). + * (cross-issuer trades): it locks nothing, so its amount is 0. May be called before or after amount(). * @param role - 'send' (default) or 'receive' * @returns The current builder instance for chaining. */ @@ -91,17 +98,17 @@ export class AllocationAllocateBuilder extends TransactionBuilder { /** * Sets the allocation amount. + * + * Zero is accepted here regardless of the role so that amount() and allocationRole() may be called in + * either order; a zero amount is only valid for a receive-only allocation, which is enforced when the + * request object is built. Negative, empty and non-numeric amounts are rejected immediately. + * * @param amount - allocation amount. Use a string or bigint above Number.MAX_SAFE_INTEGER. - * Zero is accepted only for a 'receive' allocation (set allocationRole first). * @returns The current builder instance for chaining. - * @throws Error if amount is not positive (or negative, for a receive allocation) + * @throws Error if amount is negative, empty or not a number. */ amount(amount: CantonAmountInput): this { - this._amount = normalizeCantonAmount( - amount, - 'amount must be a positive number', - this._allocationRole === 'receive' - ); + this._amount = normalizeCantonAmount(amount, AMOUNT_ERROR_MESSAGE, true); return this; } @@ -280,7 +287,7 @@ export class AllocationAllocateBuilder extends TransactionBuilder { private validate(): void { if (!this._commandId) throw new Error('commandId is missing'); if (this._amount === undefined || this._amount === null) throw new Error('amount is missing'); - if (this._allocationRole === 'send' && !Number(this._amount)) throw new Error('amount is missing'); + if (this._allocationRole === 'send' && !Number(this._amount)) throw new Error(AMOUNT_ERROR_MESSAGE); if (!this._token) throw new Error('token is missing'); if (!this._operatorId) throw new Error('operatorId is missing'); if (this._contractId === undefined) throw new Error('contractId is missing'); diff --git a/modules/sdk-coin-canton/test/unit/builder/allocationAllocate/allocationAllocateBuilder.ts b/modules/sdk-coin-canton/test/unit/builder/allocationAllocate/allocationAllocateBuilder.ts index 543274d90f..a9d93fe1fc 100644 --- a/modules/sdk-coin-canton/test/unit/builder/allocationAllocate/allocationAllocateBuilder.ts +++ b/modules/sdk-coin-canton/test/unit/builder/allocationAllocate/allocationAllocateBuilder.ts @@ -40,6 +40,13 @@ function buildWithAllRequired(txBuilder: AllocationAllocateBuilder): AllocationA .senderPartyId(senderPartyId); } +// Helper to create an initialised builder with no fields set +function newAllocationBuilder(): AllocationAllocateBuilder { + const txBuilder = new AllocationAllocateBuilder(coins.get('tcanton')); + txBuilder.initBuilder(new Transaction(coins.get('tcanton'))); + return txBuilder; +} + describe('AllocationAllocate Builder', () => { it('should build the allocation allocate request object with all fields', function () { const txBuilder = new AllocationAllocateBuilder(coins.get('tcanton')); @@ -82,9 +89,7 @@ describe('AllocationAllocate Builder', () => { }); it('should build an unfunded receive-only allocation with amount 0', function () { - const txBuilder = new AllocationAllocateBuilder(coins.get('tcanton')); - const tx = new Transaction(coins.get('tcanton')); - txBuilder.initBuilder(tx); + const txBuilder = newAllocationBuilder(); buildWithAllRequired(txBuilder.allocationRole('receive')).amount(0); const requestObj = txBuilder.toRequestObject(); assert.equal(requestObj.amount, 0); @@ -92,26 +97,54 @@ describe('AllocationAllocate Builder', () => { }); it('should accept a zero string amount for a receive-only allocation', function () { - const txBuilder = new AllocationAllocateBuilder(coins.get('tcanton')); - const tx = new Transaction(coins.get('tcanton')); - txBuilder.initBuilder(tx); + const txBuilder = newAllocationBuilder(); buildWithAllRequired(txBuilder.allocationRole('receive')).amount('0'); assert.equal(txBuilder.toRequestObject().amount, '0'); }); - it('should still reject a zero amount for a send allocation', function () { - const txBuilder = new AllocationAllocateBuilder(coins.get('tcanton')); - const tx = new Transaction(coins.get('tcanton')); - txBuilder.initBuilder(tx); - assert.throws(() => txBuilder.amount(0), /amount must be a positive number/); - assert.throws(() => txBuilder.allocationRole('send').amount('0'), /amount must be a positive number/); + it('should build a receive-only allocation when amount() is called before allocationRole()', function () { + const txBuilder = newAllocationBuilder(); + buildWithAllRequired(txBuilder).amount(0).allocationRole('receive'); + const requestObj = txBuilder.toRequestObject(); + assert.equal(requestObj.amount, 0); + assert.equal(requestObj.allocationRole, 'receive'); + }); + + it('should build a receive-only allocation with a zero string amount set before the role', function () { + const txBuilder = newAllocationBuilder(); + buildWithAllRequired(txBuilder).amount('0').allocationRole('receive'); + assert.equal(txBuilder.toRequestObject().amount, '0'); + }); + + it('should still reject a zero amount for a send allocation, naming the receive role', function () { + const txBuilder = newAllocationBuilder(); + buildWithAllRequired(txBuilder).amount(0); + assert.throws(() => txBuilder.toRequestObject(), /allocationRole\('receive'\)/); + }); + + it('should still reject an explicit send allocation with a zero string amount, naming the receive role', function () { + const txBuilder = newAllocationBuilder(); + buildWithAllRequired(txBuilder).amount('0').allocationRole('send'); + assert.throws(() => txBuilder.toRequestObject(), /allocationRole\('receive'\)/); + }); + + it('should reject a negative or empty amount with no receive role, naming the receive role', function () { + for (const badAmount of [-1, '-1', '']) { + assert.throws( + () => newAllocationBuilder().amount(badAmount), + /allocationRole\('receive'\)/, + `amount ${JSON.stringify(badAmount)} should name allocationRole('receive')` + ); + } }); it('should reject a negative amount even for a receive-only allocation', function () { - const txBuilder = new AllocationAllocateBuilder(coins.get('tcanton')); - const tx = new Transaction(coins.get('tcanton')); - txBuilder.initBuilder(tx); - assert.throws(() => txBuilder.allocationRole('receive').amount(-1), /amount must be a positive number/); + for (const badAmount of [-1, '-1']) { + assert.throws( + () => newAllocationBuilder().allocationRole('receive').amount(badAmount), + /amount must be a positive number/ + ); + } }); it('should not set allocationRole on a send allocation request', function () {