Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fixed precision issueswith small amounts in getRecipientAmount #55

Merged
merged 3 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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']}
Expand All @@ -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', () => {
Expand Down
55 changes: 35 additions & 20 deletions packages/core/src/transaction/__tests__/getRecipientsAmount.spec.ts
Original file line number Diff line number Diff line change
@@ -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`;
Expand All @@ -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: {
Expand All @@ -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');

});

Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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()
}));
}

Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/transaction/getRecipientsAmount.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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);
}