fix: fixed precision issueswith small amounts in getRecipientAmount (#55

)

* fix: fixed precision issueswith small amounts in getRecipientAmount

* chore: unit tests

* chore: review cleanup

packages/core/src/transaction/__tests__/getRecipientAmountsFromMultiOutPayment.spec.ts

@@ -1,17 +1,31 @@
 import {getRecipientAmountsFromMultiOutPayment} from '../getRecipientAmountsFromMultiOutPayment';
 import {TransactionType} from '../../constants/transactionType';
 import {TransactionPaymentSubtype} from '../../constants';
-import {convertNumberToNQTString} from '@signumjs/util';
-
-
-const nqt = convertNumberToNQTString;
+import {Amount} from '@signumjs/util';
 
 describe('getRecipientAmountsFromMultiOutPayment', () => {
 
     it('returns recipients for Multi Out Same Amount Payment', () => {
         const transaction = {
             transaction: '123',
-            amountNQT: nqt(100),
+            amountNQT: Amount.fromSigna(100).getPlanck(),
+            type: TransactionType.Payment,
+            subtype: TransactionPaymentSubtype.MultiOutSameAmount,
+            attachment: {'version.MultiOutCreation': 1, recipients: ['123', '456']}
+        };
+
+        const recipientAmounts = getRecipientAmountsFromMultiOutPayment(transaction);
+        expect(recipientAmounts).toHaveLength(2);
+        expect(recipientAmounts[0].recipient).toBe('123');
+        expect(recipientAmounts[0].amountNQT).toBe(Amount.fromSigna(50).getPlanck());
+        expect(recipientAmounts[1].recipient).toBe('456');
+        expect(recipientAmounts[1].amountNQT).toBe(Amount.fromSigna(50).getPlanck());
+    });
+
+    it('returns recipients for Multi Out Same Amount Payment for with very small value', () => {
+        const transaction = {
+            transaction: '123',
+            amountNQT: 100,
             type: TransactionType.Payment,
             subtype: TransactionPaymentSubtype.MultiOutSameAmount,
             attachment: {'version.MultiOutCreation': 1, recipients: ['123', '456']}
@@ -20,9 +34,9 @@ describe('getRecipientAmountsFromMultiOutPayment', () => {
         const recipientAmounts = getRecipientAmountsFromMultiOutPayment(transaction);
         expect(recipientAmounts).toHaveLength(2);
         expect(recipientAmounts[0].recipient).toBe('123');
-        expect(recipientAmounts[0].amountNQT).toBe(nqt(50));
+        expect(recipientAmounts[0].amountNQT).toBe("50");
         expect(recipientAmounts[1].recipient).toBe('456');
-        expect(recipientAmounts[1].amountNQT).toBe(nqt(50));
+        expect(recipientAmounts[1].amountNQT).toBe("50");
     });
 
     it('returns recipients for Multi Out Different Amount Payment', () => {

packages/core/src/transaction/__tests__/getRecipientsAmount.spec.ts

@@ -1,14 +1,11 @@
 import {
-    TransactionArbitrarySubtype,
     TransactionEscrowSubtype,
     TransactionPaymentSubtype,
     TransactionType
 } from '../../constants';
-import {convertNumberToNQTString} from '@signumjs/util';
+import {Amount} from '@signumjs/util';
 import {getRecipientsAmount} from '../getRecipientsAmount';
 
-const nqt = convertNumberToNQTString;
-
 describe('getRecipientsAmount', () => {
 
     const recipientId = `123`;
@@ -17,35 +14,53 @@ describe('getRecipientsAmount', () => {
 
         const transaction = {
             transaction: '1',
-            amountNQT: nqt(100),
+            amountNQT: Amount.fromSigna(100).getPlanck(),
             type: TransactionType.Payment,
             subtype: TransactionPaymentSubtype.Ordinary,
         };
 
         const amount = getRecipientsAmount(recipientId, transaction);
-        expect(amount).toBe(100);
+        expect(amount.getSigna()).toBe('100');
 
     });
 
-    it('receives correct amount from any kind transaction, but multi ou', () => {
+    it('receives correct amount from any kind transaction, but multi out', () => {
 
         const transaction = {
             transaction: '1',
-            amountNQT: nqt(100),
+            amountNQT: Amount.fromSigna(100).getPlanck(),
             type: TransactionType.Escrow,
             subtype: TransactionEscrowSubtype.EscrowCreation,
         };
 
         const amount = getRecipientsAmount(recipientId, transaction);
-        expect(amount).toBe(100);
+        expect(amount.getSigna()).toBe('100');
 
     });
 
     it('receives correct amount from multi out same transaction', () => {
 
         const transaction = {
             transaction: '1',
-            amountNQT: nqt(150),
+            amountNQT: Amount.fromSigna(150).getPlanck(),
+            type: TransactionType.Payment,
+            subtype: TransactionPaymentSubtype.MultiOutSameAmount,
+            attachment: {
+                'version.MultiSameOutCreation': 1,
+                recipients: [recipientId, '456', recipientId] // tests also multiple mentions
+            }
+        };
+
+        const amount = getRecipientsAmount(recipientId, transaction);
+        expect(amount.getSigna()).toBe('100');
+
+    });
+
+    it('receives correct amount from multi out same transaction with very low value', () => {
+
+        const transaction = {
+            transaction: '1',
+            amountNQT: 10,
             type: TransactionType.Payment,
             subtype: TransactionPaymentSubtype.MultiOutSameAmount,
             attachment: {
@@ -55,51 +70,51 @@ describe('getRecipientsAmount', () => {
         };
 
         const amount = getRecipientsAmount(recipientId, transaction);
-        expect(amount).toBe((2 / 3) * 150);
+        expect(amount.getSigna()).toBe('0.00000006');
 
     });
 
     it('receives correct amount from multi out diff transaction', () => {
 
         const transaction = {
             transaction: '1',
-            amountNQT: nqt(160),
+            amountNQT: Amount.fromSigna(160).getPlanck(),
             type: TransactionType.Payment,
             subtype: TransactionPaymentSubtype.MultiOut,
             attachment: {
                 'version.MultiOutCreation': 1,
                 // tests also multiple mentions
                 recipients: [
-                    [recipientId, nqt(100)],
-                    ['456', nqt(10)],
-                    [recipientId, nqt(50)],
+                    [recipientId, Amount.fromSigna(100).getPlanck()],
+                    ['456', Amount.fromSigna(10).getPlanck()],
+                    [recipientId, Amount.fromSigna(50).getPlanck()],
                 ]
             }
         };
 
         const amount = getRecipientsAmount(recipientId, transaction);
-        expect(amount).toBe(150);
+        expect(amount.getSigna()).toBe('150');
 
     });
 
     it('receives zero amount from multi out as not being recipient', () => {
 
         const transaction = {
             transaction: '1',
-            amountNQT: nqt(110),
+            amountNQT: Amount.fromSigna(110).getPlanck(),
             type: TransactionType.Payment,
             subtype: TransactionPaymentSubtype.MultiOut,
             attachment: {
                 'version.MultiOutCreation': 1,
                 recipients: [
-                    ['789', nqt(100)],
-                    ['456', nqt(10)],
+                    ['789', Amount.fromSigna(100).getPlanck()],
+                    ['456', Amount.fromSigna(10).getPlanck()],
                 ]
             }
         };
 
         const amount = getRecipientsAmount(recipientId, transaction);
-        expect(amount).toBe(0);
+        expect(amount.getSigna()).toBe('0');
 
     });
 

packages/core/src/transaction/getRecipientAmountsFromMultiOutPayment.ts

@@ -1,7 +1,7 @@
 import {Transaction, MultioutRecipientAmount} from '..';
 import {isMultiOutSameTransaction} from './isMultiOutSameTransaction';
 import {isMultiOutTransaction} from './isMultiOutTransaction';
-import {convertNQTStringToNumber, convertNumberToNQTString} from '@signumjs/util';
+import {Amount} from '@signumjs/util';
 
 /**
  * Tries to extract recipients and its amounts for multi out payments (different and same amount)
@@ -16,11 +16,11 @@ export function getRecipientAmountsFromMultiOutPayment(transaction: Transaction)
 
         const recipients = transaction.attachment.recipients;
 
-        const amount = recipients.length ? convertNQTStringToNumber(transaction.amountNQT) / recipients.length : 0;
-        const amountNQT = convertNumberToNQTString(amount);
+        const amount = recipients.length ? Amount.fromPlanck(transaction.amountNQT).divide(recipients.length) : Amount.Zero();
+
         return transaction.attachment.recipients.map(recipient => ({
             recipient,
-            amountNQT
+            amountNQT: amount.getPlanck()
         }));
     }
 

packages/core/src/transaction/getRecipientsAmount.ts

@@ -1,4 +1,4 @@
-import {convertNQTStringToNumber} from '@signumjs/util';
+import {Amount} from '@signumjs/util';
 import {Transaction} from '../typings/transaction';
 import {getRecipientAmountsFromMultiOutPayment} from './getRecipientAmountsFromMultiOutPayment';
 import {isMultiOutTransaction} from './isMultiOutTransaction';
@@ -8,18 +8,18 @@ import {isMultiOutSameTransaction} from './isMultiOutSameTransaction';
  * Gets the amount from a transaction, considering ordinary and multi out transactions (with same and different payments)
  * @param recipientId The numeric id of the recipient
  * @param transaction The payment transaction
- * @return the amount in BURST (not NQT)
+ * @return the amount value object
  * @module core
  */
-export function getRecipientsAmount(recipientId: string, transaction: Transaction): number {
+export function getRecipientsAmount(recipientId: string, transaction: Transaction): Amount {
 
     if (isMultiOutTransaction(transaction) || isMultiOutSameTransaction(transaction)) {
 
         const recipientAmounts = getRecipientAmountsFromMultiOutPayment(transaction);
         return recipientAmounts
             .filter(ra => ra.recipient === recipientId)
-            .reduce((amount, ra) => amount + convertNQTStringToNumber(ra.amountNQT), 0);
+            .reduce((amount, ra) => amount.add(Amount.fromPlanck(ra.amountNQT)), Amount.Zero());
     }
 
-    return convertNQTStringToNumber(transaction.amountNQT);
+    return Amount.fromPlanck(transaction.amountNQT);
 }