Skip to content

Commit

Permalink
refactor: transaction fees (#2842)
Browse files Browse the repository at this point in the history 
* refactor: static fees

* refactor: dynamic fees

* chore: drop lodash.camelcase
  • Loading branch information
@spkjp
spkjp committed Jul 25, 2019
1 parent 016ab15 commit aeff8b1
Show file tree
Hide file tree
Showing 47 changed files with 298 additions and 353 deletions.
85 changes: 41 additions & 44 deletions __tests__/unit/core-transaction-pool/dynamic-fee.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { container } from "./mocks/core-container";

import { Utils } from "@arkecosystem/crypto";
import { Handlers } from "@arkecosystem/core-transactions/src";
import { defaults } from "../../../packages/core-transaction-pool/src/defaults";
import { calculateMinimumFee, dynamicFeeMatcher } from "../../../packages/core-transaction-pool/src/dynamic-fee";
import { dynamicFeeMatcher } from "../../../packages/core-transaction-pool/src/dynamic-fee";
import { transactions } from "./__fixtures__/transactions";

describe("static fees", () => {
Expand Down Expand Up @@ -46,32 +46,59 @@ describe("dynamic fees", () => {
expect(dynamicFeeMatcher(transactions.dummy1).broadcast).toBeTrue();
expect(dynamicFeeMatcher(transactions.dummy2).broadcast).toBeTrue();

transactions.dynamicFeeNormalDummy1.data.fee = calculateMinimumFee(
dynamicFeeConfig.minFeeBroadcast,
transactions.dynamicFeeNormalDummy1,
).plus(100);
const addonBytes: number = (container.app.resolveOptions() as any).dynamicFees.addonBytes[
transactions.dynamicFeeNormalDummy1.key
];

const handler = Handlers.Registry.get(transactions.dummy1.type);
transactions.dynamicFeeNormalDummy1.data.fee = handler
.dynamicFee(transactions.dynamicFeeNormalDummy1, addonBytes, dynamicFeeConfig.minFeeBroadcast)
.plus(100);

expect(dynamicFeeMatcher(transactions.dynamicFeeNormalDummy1).broadcast).toBeTrue();

// testing with transaction fee === min fee for transaction broadcast
transactions.dummy3.data.fee = calculateMinimumFee(dynamicFeeConfig.minFeeBroadcast, transactions.dummy3);
transactions.dummy4.data.fee = calculateMinimumFee(dynamicFeeConfig.minFeeBroadcast, transactions.dummy4);
transactions.dummy3.data.fee = handler.dynamicFee(
transactions.dummy3,
addonBytes,
dynamicFeeConfig.minFeeBroadcast,
);
transactions.dummy4.data.fee = handler.dynamicFee(
transactions.dummy4,
addonBytes,
dynamicFeeConfig.minFeeBroadcast,
);
expect(dynamicFeeMatcher(transactions.dummy3).broadcast).toBeTrue();
expect(dynamicFeeMatcher(transactions.dummy4).broadcast).toBeTrue();
});

it("should accept transactions with high enough fee to enter the pool", () => {
const addonBytes: number = (container.app.resolveOptions() as any).dynamicFees.addonBytes[
transactions.dynamicFeeNormalDummy1.key
];

const handler = Handlers.Registry.get(transactions.dummy1.type);

expect(dynamicFeeMatcher(transactions.dummy1).enterPool).toBeTrue();
expect(dynamicFeeMatcher(transactions.dummy2).enterPool).toBeTrue();

transactions.dynamicFeeNormalDummy1.data.fee = calculateMinimumFee(
dynamicFeeConfig.minFeePool,
transactions.dynamicFeeNormalDummy1,
).plus(100);
transactions.dynamicFeeNormalDummy1.data.fee = handler
.dynamicFee(transactions.dynamicFeeNormalDummy1, addonBytes, dynamicFeeConfig.minFeePool)
.plus(100);

expect(dynamicFeeMatcher(transactions.dynamicFeeNormalDummy1).enterPool).toBeTrue();

// testing with transaction fee === min fee for transaction enter pool
transactions.dummy3.data.fee = calculateMinimumFee(dynamicFeeConfig.minFeePool, transactions.dummy3);
transactions.dummy4.data.fee = calculateMinimumFee(dynamicFeeConfig.minFeePool, transactions.dummy4);
transactions.dummy3.data.fee = handler.dynamicFee(
transactions.dummy3,
addonBytes,
dynamicFeeConfig.minFeeBroadcast,
);
transactions.dummy4.data.fee = handler.dynamicFee(
transactions.dummy4,
addonBytes,
dynamicFeeConfig.minFeeBroadcast,
);
expect(dynamicFeeMatcher(transactions.dummy3).enterPool).toBeTrue();
expect(dynamicFeeMatcher(transactions.dummy4).enterPool).toBeTrue();
});
Expand All @@ -84,33 +111,3 @@ describe("dynamic fees", () => {
expect(dynamicFeeMatcher(transactions.dynamicFeeLowDummy2).enterPool).toBeFalse();
});
});

describe("calculateMinimumFee", () => {
it("should correctly calculate the transaction fee based on transaction size and addonBytes", () => {
jest.spyOn(container.app, "resolveOptions").mockReturnValue({
...defaults,
...{ dynamicFees: { addonBytes: { transfer: 137 } } },
});

expect(calculateMinimumFee(3, transactions.dummy1)).toEqual(
Utils.BigNumber.make(137 + transactions.dummy1.serialized.length / 2).times(3),
);
expect(calculateMinimumFee(6, transactions.dummy1)).toEqual(
Utils.BigNumber.make(137 + transactions.dummy1.serialized.length / 2).times(6),
);

jest.spyOn(container.app, "resolveOptions").mockReturnValue({
...defaults,
...{ dynamicFees: { addonBytes: { transfer: 0 } } },
});

expect(calculateMinimumFee(9, transactions.dummy1)).toEqual(
Utils.BigNumber.make(transactions.dummy1.serialized.length / 2).times(9),
);
});

it("should default satoshiPerByte to 1 if value provided is <= 0", () => {
expect(calculateMinimumFee(-50, transactions.dummy1)).toEqual(calculateMinimumFee(1, transactions.dummy1));
expect(calculateMinimumFee(0, transactions.dummy1)).toEqual(calculateMinimumFee(1, transactions.dummy1));
});
});
32 changes: 32 additions & 0 deletions __tests__/unit/core-transactions/handler.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,38 @@ describe("General Tests", () => {
expect(recipientWallet.balance).toEqual(Utils.BigNumber.make(recipientBalance).minus(instance.data.amount));
});
});

describe("dynamicFees", () => {
const transaction = TransactionFactory.transfer("AFzQCx5YpGg5vKMBg4xbuYbqkhvMkKfKe5")
.withNonce(Utils.BigNumber.make(0))
.withNetwork("testnet")
.withPassphrase("secret")
.build()[0];

it("should correctly calculate the transaction fee based on transaction size and addonBytes", () => {
const addonBytes = 137;
const handler = Handlers.Registry.get(transaction.type);

expect(handler.dynamicFee(transaction, addonBytes, 3)).toEqual(
Utils.BigNumber.make(137 + transaction.serialized.length / 2).times(3),
);

expect(handler.dynamicFee(transaction, addonBytes, 6)).toEqual(
Utils.BigNumber.make(137 + transaction.serialized.length / 2).times(6),
);

expect(handler.dynamicFee(transaction, 0, 9)).toEqual(
Utils.BigNumber.make(transaction.serialized.length / 2).times(9),
);
});

it("should default satoshiPerByte to 1 if value provided is <= 0", () => {
const handler = Handlers.Registry.get(transaction.type);

expect(handler.dynamicFee(transaction, 0, -50)).toEqual(handler.dynamicFee(transaction, 0, 1));
expect(handler.dynamicFee(transaction, 0, 0)).toEqual(handler.dynamicFee(transaction, 0, 1));
});
});
});

describe("TransferTransaction", () => {
Expand Down
64 changes: 1 addition & 63 deletions __tests__/unit/crypto/managers/config.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import "jest-extended";

import { TransactionTypes } from "../../../../packages/crypto/src/enums";
import { configManager, feeManager } from "../../../../packages/crypto/src/managers";
import { configManager } from "../../../../packages/crypto/src/managers";
import { devnet, mainnet } from "../../../../packages/crypto/src/networks";
import { BigNumber } from "../../../../packages/crypto/src/utils";

beforeEach(() => configManager.setConfig(devnet));

Expand Down Expand Up @@ -34,66 +32,6 @@ describe("Configuration", () => {
expect(configManager.getMilestones()).toEqual(devnet.milestones);
});

it("should build fees", () => {
const feesStatic = devnet.milestones[0].fees.staticFees;

expect(feeManager.get(TransactionTypes.Transfer)).toEqual(BigNumber.make(feesStatic.transfer));
expect(feeManager.get(TransactionTypes.SecondSignature)).toEqual(BigNumber.make(feesStatic.secondSignature));
expect(feeManager.get(TransactionTypes.DelegateRegistration)).toEqual(
BigNumber.make(feesStatic.delegateRegistration),
);
expect(feeManager.get(TransactionTypes.Vote)).toEqual(BigNumber.make(feesStatic.vote));
expect(feeManager.get(TransactionTypes.MultiSignature)).toEqual(BigNumber.make(feesStatic.multiSignature));
expect(feeManager.get(TransactionTypes.Ipfs)).toEqual(BigNumber.make(feesStatic.ipfs));
expect(feeManager.get(TransactionTypes.TimelockTransfer)).toEqual(BigNumber.make(feesStatic.timelockTransfer));
expect(feeManager.get(TransactionTypes.MultiPayment)).toEqual(BigNumber.make(feesStatic.multiPayment));
expect(feeManager.get(TransactionTypes.DelegateResignation)).toEqual(
BigNumber.make(feesStatic.delegateResignation),
);
});

it("should update fees on milestone change", () => {
devnet.milestones.push({
height: 100000000,
fees: { staticFees: { transfer: 1234 } },
} as any);

configManager.setHeight(100000000);

let { staticFees } = configManager.getMilestone().fees;
expect(feeManager.get(TransactionTypes.Transfer)).toEqual(BigNumber.make(1234));
expect(feeManager.get(TransactionTypes.SecondSignature)).toEqual(BigNumber.make(staticFees.secondSignature));
expect(feeManager.get(TransactionTypes.DelegateRegistration)).toEqual(
BigNumber.make(staticFees.delegateRegistration),
);
expect(feeManager.get(TransactionTypes.Vote)).toEqual(BigNumber.make(staticFees.vote));
expect(feeManager.get(TransactionTypes.MultiSignature)).toEqual(BigNumber.make(staticFees.multiSignature));
expect(feeManager.get(TransactionTypes.Ipfs)).toEqual(BigNumber.make(staticFees.ipfs));
expect(feeManager.get(TransactionTypes.TimelockTransfer)).toEqual(BigNumber.make(staticFees.timelockTransfer));
expect(feeManager.get(TransactionTypes.MultiPayment)).toEqual(BigNumber.make(staticFees.multiPayment));
expect(feeManager.get(TransactionTypes.DelegateResignation)).toEqual(
BigNumber.make(staticFees.delegateResignation),
);

configManager.setHeight(1);
staticFees = configManager.getMilestone().fees.staticFees;
expect(feeManager.get(TransactionTypes.Transfer)).toEqual(BigNumber.make(staticFees.transfer));
expect(feeManager.get(TransactionTypes.SecondSignature)).toEqual(BigNumber.make(staticFees.secondSignature));
expect(feeManager.get(TransactionTypes.DelegateRegistration)).toEqual(
BigNumber.make(staticFees.delegateRegistration),
);
expect(feeManager.get(TransactionTypes.Vote)).toEqual(BigNumber.make(staticFees.vote));
expect(feeManager.get(TransactionTypes.MultiSignature)).toEqual(BigNumber.make(staticFees.multiSignature));
expect(feeManager.get(TransactionTypes.Ipfs)).toEqual(BigNumber.make(staticFees.ipfs));
expect(feeManager.get(TransactionTypes.TimelockTransfer)).toEqual(BigNumber.make(staticFees.timelockTransfer));
expect(feeManager.get(TransactionTypes.MultiPayment)).toEqual(BigNumber.make(staticFees.multiPayment));
expect(feeManager.get(TransactionTypes.DelegateResignation)).toEqual(
BigNumber.make(staticFees.delegateResignation),
);

devnet.milestones.pop();
});

it("should get milestone for height", () => {
expect(configManager.getMilestone(21600)).toEqual(devnet.milestones[2]);
});
Expand Down
60 changes: 0 additions & 60 deletions __tests__/unit/crypto/managers/fee.test.ts
View file

This file was deleted.

5 changes: 2 additions & 3 deletions __tests__/unit/crypto/transactions/builders/transactions/delegate-registration.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import "jest-extended";

import { TransactionTypes } from "../../../../../../packages/crypto/src/enums";
import { feeManager } from "../../../../../../packages/crypto/src/managers/fee";
import { Utils } from "../../../../../../packages/crypto/src/transactions";
import { DelegateRegistrationTransaction, Utils } from "../../../../../../packages/crypto/src/transactions";
import { BuilderFactory } from "../../../../../../packages/crypto/src/transactions/builders";
import { DelegateRegistrationBuilder } from "../../../../../../packages/crypto/src/transactions/builders/transactions/delegate-registration";
import { BigNumber } from "../../../../../../packages/crypto/src/utils";
Expand Down Expand Up @@ -41,7 +40,7 @@ describe("Delegate Registration Transaction", () => {
it("should have its specific properties", () => {
expect(builder).toHaveProperty("data.type", TransactionTypes.DelegateRegistration);
expect(builder).toHaveProperty("data.amount", BigNumber.ZERO);
expect(builder).toHaveProperty("data.fee", feeManager.get(TransactionTypes.DelegateRegistration));
expect(builder).toHaveProperty("data.fee", DelegateRegistrationTransaction.staticFee());
expect(builder).toHaveProperty("data.recipientId", undefined);
expect(builder).toHaveProperty("data.senderPublicKey", undefined);
expect(builder).toHaveProperty("data.asset", { delegate: {} });
Expand Down
5 changes: 3 additions & 2 deletions __tests__/unit/crypto/transactions/builders/transactions/delegate-resignation.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import "jest-extended";

import { TransactionTypes } from "../../../../../../packages/crypto/src/enums";
import { configManager, feeManager } from "../../../../../../packages/crypto/src/managers";
import { configManager } from "../../../../../../packages/crypto/src/managers";
import { DelegateResignationTransaction } from "../../../../../../packages/crypto/src/transactions/";
import { BuilderFactory } from "../../../../../../packages/crypto/src/transactions/builders";
import { DelegateResignationBuilder } from "../../../../../../packages/crypto/src/transactions/builders/transactions/delegate-resignation";
import { BigNumber } from "../../../../../../packages/crypto/src/utils";
Expand Down Expand Up @@ -35,7 +36,7 @@ describe("Delegate Resignation Transaction", () => {
it("should have its specific properties", () => {
expect(builder).toHaveProperty("data.type", TransactionTypes.DelegateResignation);
expect(builder).toHaveProperty("data.amount", BigNumber.ZERO);
expect(builder).toHaveProperty("data.fee", feeManager.get(TransactionTypes.DelegateResignation));
expect(builder).toHaveProperty("data.fee", DelegateResignationTransaction.staticFee());
expect(builder).toHaveProperty("data.senderPublicKey", undefined);
});

Expand Down
5 changes: 2 additions & 3 deletions __tests__/unit/crypto/transactions/builders/transactions/ipfs.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import "jest-extended";

import { Utils } from "@arkecosystem/crypto";
import { TransactionTypes } from "../../../../../../packages/crypto/src/enums";
import { feeManager } from "../../../../../../packages/crypto/src/managers/fee";
import { BuilderFactory } from "../../../../../../packages/crypto/src/transactions";
import { BuilderFactory, IpfsTransaction } from "../../../../../../packages/crypto/src/transactions";
import { IPFSBuilder } from "../../../../../../packages/crypto/src/transactions/builders/transactions/ipfs";
import { transactionBuilder } from "./__shared__/transaction-builder";

Expand All @@ -18,7 +17,7 @@ describe("IPFS Transaction", () => {

it("should have its specific properties", () => {
expect(builder).toHaveProperty("data.type", TransactionTypes.Ipfs);
expect(builder).toHaveProperty("data.fee", feeManager.get(TransactionTypes.Ipfs));
expect(builder).toHaveProperty("data.fee", IpfsTransaction.staticFee());
expect(builder).toHaveProperty("data.amount", Utils.BigNumber.make(0));
expect(builder).toHaveProperty("data.asset", {});
});
Expand Down
5 changes: 2 additions & 3 deletions __tests__/unit/crypto/transactions/builders/transactions/multi-payment.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ import "jest-extended";

import { TransactionTypes } from "../../../../../../packages/crypto/src/enums";
import { MaximumPaymentCountExceededError } from "../../../../../../packages/crypto/src/errors";
import { feeManager } from "../../../../../../packages/crypto/src/managers/fee";
import { BuilderFactory } from "../../../../../../packages/crypto/src/transactions";
import { BuilderFactory, MultiPaymentTransaction } from "../../../../../../packages/crypto/src/transactions";
import { MultiPaymentBuilder } from "../../../../../../packages/crypto/src/transactions/builders/transactions/multi-payment";
import { BigNumber } from "../../../../../../packages/crypto/src/utils";
import { transactionBuilder } from "./__shared__/transaction-builder";
Expand All @@ -19,7 +18,7 @@ describe("Multi Payment Transaction", () => {

it("should have its specific properties", () => {
expect(builder).toHaveProperty("data.type", TransactionTypes.MultiPayment);
expect(builder).toHaveProperty("data.fee", feeManager.get(TransactionTypes.MultiPayment));
expect(builder).toHaveProperty("data.fee", MultiPaymentTransaction.staticFee());
expect(builder).toHaveProperty("data.asset.payments", []);
expect(builder).toHaveProperty("data.vendorFieldHex", undefined);
});
Expand Down
5 changes: 2 additions & 3 deletions __tests__/unit/crypto/transactions/builders/transactions/multi-signature.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ configManager.setFromPreset("testnet");

import { TransactionTypes } from "../../../../../../packages/crypto/src/enums";
import { TransactionVersionError } from "../../../../../../packages/crypto/src/errors";
import { feeManager } from "../../../../../../packages/crypto/src/managers/fee";
import { BuilderFactory } from "../../../../../../packages/crypto/src/transactions";
import { BuilderFactory, MultiSignatureRegistrationTransaction } from "../../../../../../packages/crypto/src/transactions";
import { MultiSignatureBuilder } from "../../../../../../packages/crypto/src/transactions/builders/transactions/multi-signature";
import * as Utils from "../../../../../../packages/crypto/src/utils";
import { transactionBuilder } from "./__shared__/transaction-builder";
Expand Down Expand Up @@ -72,7 +71,7 @@ describe("Multi Signature Transaction", () => {
});

describe("multiSignatureAsset", () => {
const multiSignatureFee = feeManager.get(TransactionTypes.MultiSignature);
const multiSignatureFee = MultiSignatureRegistrationTransaction.staticFee();
const multiSignature = {
publicKeys: ["key a", "key b", "key c"],
min: 1,
Expand Down

0 comments on commit aeff8b1

Please sign in to comment.