fix(core-transaction-pool): use sorted array (instead of tree) for storing transactions by fee and nonce

__tests__/unit/core-transaction-pool/connection.test.ts

@@ -1087,16 +1087,19 @@ describe("Connection", () => {
                     expect(prevSender).toEqual(curSender);
                 }
 
-                if (prevSender !== curSender) {
-                    let j;
-                    for (j = i - 2; j >= 0 && sortedTransactions[j].data.senderPublicKey === prevSender; j--) {
-                        // Find the leftmost transaction in a sequence of transactions from the same
-                        // sender, ending at prevTransaction. That leftmost transaction's fee must
-                        // be greater or equal to the fee of curTransaction.
-                    }
-                    j++;
-                    expect(sortedTransactions[j].data.fee.isGreaterThanEqual(curTransaction.data.fee)).toBeTrue();
-                }
+                // This is not true anymore with current implementation, which is simpler and more performant
+                // than the previous one, but it does not do this fee optimization (which is a very specific
+                // case and is not worth it currently)
+                // if (prevSender !== curSender) {
+                //    let j;
+                //    for (j = i - 2; j >= 0 && sortedTransactions[j].data.senderPublicKey === prevSender; j--) {
+                //        // Find the leftmost transaction in a sequence of transactions from the same
+                //        // sender, ending at prevTransaction. That leftmost transaction's fee must
+                //        // be greater or equal to the fee of curTransaction.
+                //    }
+                //    j++;
+                //    expect(sortedTransactions[j].data.fee.isGreaterThanEqual(curTransaction.data.fee)).toBeTrue();
+                // }
 
                 if (lastNonceBySender[curSender] !== undefined) {
                     expect(lastNonceBySender[curSender].isLessThan(curTransaction.data.nonce)).toBeTrue();

__tests__/unit/core-transaction-pool/memory.test.ts

@@ -0,0 +1,120 @@
+import "./mocks/core-container";
+
+import { Managers, Transactions, Utils } from "@arkecosystem/crypto";
+import { Memory } from "../../../packages/core-transaction-pool/src/memory";
+
+Managers.configManager.setFromPreset("testnet");
+Managers.configManager.getMilestone().aip11 = true;
+
+describe("Memory", () => {
+    describe("getLowestFeeLastNonce", () => {
+        it("should get the lowest fee which also is the last nonce from the sender - when a 'last nonce tx' is among the 100 lowest fee txs", () => {
+            const txs = [];
+            let lastNonceLowestFeeTx;
+            for (let i = 0; i < 100; i++) {
+                for (let nonce = 1; nonce <= 10; nonce++) {
+                    const passphrase = `random ${i}`;
+                    const address = "AWLzbT4z7KntQ3D9LTz7ukPHyXy6mNy2YQ";
+                    const transaction = Transactions.BuilderFactory.transfer()
+                        .nonce(nonce.toString())
+                        .recipientId(address)
+                        .amount("1")
+                        .fee((1e6 - 1000 * nonce - i).toString()) // the last nonce tx will be the lowest fee
+                        .sign(passphrase)
+                        .build();
+
+                    txs.push(transaction);
+                }
+                const lastNonceTx = txs[txs.length - 1];
+                lastNonceLowestFeeTx = lastNonceLowestFeeTx
+                    ? lastNonceTx.data.fee.isLessThan(lastNonceLowestFeeTx.data.fee)
+                        ? lastNonceTx
+                        : lastNonceLowestFeeTx
+                    : lastNonceTx;
+            }
+
+            const memory = new Memory(120);
+            for (const tx of txs) {
+                memory.remember(tx);
+            }
+
+            expect(memory.getLowestFeeLastNonce()).toEqual(lastNonceLowestFeeTx);
+        });
+
+        it("should get the lowest fee which also is the last nonce from the sender - when the 100 lowest fees are not a 'last nonce tx'", () => {
+            const txs = [];
+            let lastNonceLowestFeeTx;
+            for (let i = 0; i < 100; i++) {
+                for (let nonce = 1; nonce <= 10; nonce++) {
+                    const passphrase = `random ${i}`;
+                    const address = "AWLzbT4z7KntQ3D9LTz7ukPHyXy6mNy2YQ";
+                    const transaction = Transactions.BuilderFactory.transfer()
+                        .nonce(nonce.toString())
+                        .recipientId(address)
+                        .amount("1")
+                        .fee((1000 * nonce + i).toString()) // the last nonce tx will be the biggest fee
+                        .sign(passphrase)
+                        .build();
+
+                    txs.push(transaction);
+                }
+                const lastNonceTx = txs[txs.length - 1];
+                lastNonceLowestFeeTx = lastNonceLowestFeeTx
+                    ? lastNonceTx.data.fee.isLessThan(lastNonceLowestFeeTx.data.fee)
+                        ? lastNonceTx
+                        : lastNonceLowestFeeTx
+                    : lastNonceTx;
+            }
+
+            const memory = new Memory(120);
+            for (const tx of txs) {
+                memory.remember(tx);
+            }
+
+            expect(memory.getLowestFeeLastNonce()).toEqual(lastNonceLowestFeeTx);
+        });
+    });
+
+    describe("sortedByFee", () => {
+        it.each([[undefined], [100], [190], [565], [1550]])(
+            "should keep nonce order when sorting by fee and nonce - with limit %s",
+            limit => {
+                const txs = [];
+                for (let i = 0; i < 50; i++) {
+                    for (let nonce = 1; nonce <= 50; nonce++) {
+                        const passphrase = `random ${i}`;
+                        const address = "AWLzbT4z7KntQ3D9LTz7ukPHyXy6mNy2YQ";
+                        const transaction = Transactions.BuilderFactory.transfer()
+                            .nonce(nonce.toString())
+                            .recipientId(address)
+                            .amount("1")
+                            .fee(Math.floor(Math.random() * 100000).toString()) //
+                            .sign(passphrase)
+                            .build();
+
+                        txs.push(transaction);
+                    }
+                }
+
+                const memory = new Memory(120);
+                for (const tx of txs) {
+                    memory.remember(tx);
+                }
+
+                const byFee = [...memory.sortedByFee(limit)];
+
+                // checking that nonces are in order
+                const lastNonceBySender: { [id: string]: Utils.BigNumber } = {};
+                for (const tx of byFee) {
+                    const sender = tx.data.senderPublicKey;
+                    if (lastNonceBySender[sender]) {
+                        expect(tx.data.nonce).toEqual(lastNonceBySender[sender].plus(1));
+                    } else {
+                        expect(tx.data.nonce).toEqual(Utils.BigNumber.ONE);
+                    }
+                    lastNonceBySender[sender] = tx.data.nonce;
+                }
+            },
+        );
+    });
+});

__tests__/unit/core-utils/sorted-array.test.ts

@@ -0,0 +1,170 @@
+import { SortedArray } from "@arkecosystem/core-utils/src/sorted-array";
+import { Utils } from "@arkecosystem/crypto";
+
+describe("SortedArray", () => {
+    // using Bignumbers for the tests
+    const compareFunction = (a: Utils.BigNumber, b: Utils.BigNumber) => {
+        if (a.isGreaterThan(b)) {
+            return 1;
+        }
+        if (a.isLessThan(b)) {
+            return -1;
+        }
+        return 0;
+    };
+
+    describe("insert", () => {
+        it("should insert sorted", () => {
+            const sortedArray = new SortedArray(compareFunction);
+
+            const bignums: Utils.BigNumber[] = [];
+            for (let i = 0; i < 1000; i++) {
+                const randomBignum = Utils.BigNumber.make(Math.floor(Math.random() * 1000000));
+                if (bignums.find(b => b.isEqualTo(randomBignum))) {
+                    continue;
+                }
+
+                sortedArray.insert(randomBignum);
+                bignums.push(randomBignum);
+            }
+
+            expect(sortedArray.getAll()).toEqual(bignums.sort(compareFunction));
+        });
+
+        it("should insert sorted - when some values are identical", () => {
+            const sortedArray = new SortedArray(compareFunction);
+
+            const bignums: Utils.BigNumber[] = [];
+            for (let i = 0; i < 1000; i++) {
+                const randomBignum = Utils.BigNumber.make(Math.floor(Math.random() * 100));
+                sortedArray.insert(randomBignum);
+                bignums.push(randomBignum);
+            }
+
+            expect(sortedArray.getAll()).toEqual(bignums.sort(compareFunction));
+        });
+    });
+
+    describe("findIndex", () => {
+        it("should find the index corresponding to the find function", () => {
+            const sortedArray = new SortedArray(compareFunction);
+
+            const bignums: Utils.BigNumber[] = [];
+            for (let i = 0; i < 1000; i++) {
+                const randomBignum = Utils.BigNumber.make(Math.floor(Math.random() * 1000000));
+                if (bignums.find(b => b.isEqualTo(randomBignum))) {
+                    continue;
+                }
+
+                sortedArray.insert(randomBignum);
+                bignums.push(randomBignum);
+            }
+
+            const findLastInserted = sortedArray.findIndex(bignum => bignum.isEqualTo(bignums[bignums.length - 1]));
+            const all = sortedArray.getAll();
+            expect(all[findLastInserted]).toEqual(bignums[bignums.length - 1]);
+        });
+    });
+
+    describe("removeAtIndex", () => {
+        it("should remove the item at the index specified", () => {
+            const sortedArray = new SortedArray(compareFunction);
+            const bignums = [
+                Utils.BigNumber.make(2),
+                Utils.BigNumber.make(5),
+                Utils.BigNumber.make(11),
+                Utils.BigNumber.make(34),
+                Utils.BigNumber.make(555),
+            ];
+            for (const bignum of bignums) {
+                sortedArray.insert(bignum);
+            }
+
+            sortedArray.removeAtIndex(0);
+            expect(sortedArray.getAll()).toEqual(bignums.slice(1));
+
+            sortedArray.removeAtIndex(2);
+            expect(sortedArray.getAll()).toEqual([
+                Utils.BigNumber.make(5),
+                Utils.BigNumber.make(11),
+                Utils.BigNumber.make(555),
+            ]);
+        });
+    });
+
+    describe("getStrictlyBelow", () => {
+        it("should get the items strictly below item specified", () => {
+            const sortedArray = new SortedArray(compareFunction);
+            const bignums = [
+                Utils.BigNumber.make(2),
+                Utils.BigNumber.make(5),
+                Utils.BigNumber.make(11),
+                Utils.BigNumber.make(34),
+                Utils.BigNumber.make(555),
+            ];
+            for (const bignum of bignums) {
+                sortedArray.insert(bignum);
+            }
+
+            expect(sortedArray.getStrictlyBelow(Utils.BigNumber.make(12))).toEqual([
+                Utils.BigNumber.make(2),
+                Utils.BigNumber.make(5),
+                Utils.BigNumber.make(11),
+            ]);
+
+            expect(sortedArray.getStrictlyBelow(Utils.BigNumber.make(11))).toEqual([
+                Utils.BigNumber.make(2),
+                Utils.BigNumber.make(5),
+            ]);
+
+            expect(sortedArray.getStrictlyBelow(Utils.BigNumber.make(2))).toEqual([]);
+
+            expect(sortedArray.getStrictlyBelow(Utils.BigNumber.make(1))).toEqual([]);
+
+            expect(sortedArray.getStrictlyBelow(Utils.BigNumber.make(556))).toEqual(bignums);
+        });
+    });
+
+    describe("getStrictlyBetween", () => {
+        it("should get the items strictly between items specified", () => {
+            const sortedArray = new SortedArray(compareFunction);
+            const bignums = [
+                Utils.BigNumber.make(2),
+                Utils.BigNumber.make(5),
+                Utils.BigNumber.make(11),
+                Utils.BigNumber.make(34),
+                Utils.BigNumber.make(555),
+            ];
+            for (const bignum of bignums) {
+                sortedArray.insert(bignum);
+            }
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(1), Utils.BigNumber.make(12))).toEqual([
+                Utils.BigNumber.make(2),
+                Utils.BigNumber.make(5),
+                Utils.BigNumber.make(11),
+            ]);
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(1), Utils.BigNumber.make(11))).toEqual([
+                Utils.BigNumber.make(2),
+                Utils.BigNumber.make(5),
+            ]);
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(2), Utils.BigNumber.make(11))).toEqual([
+                Utils.BigNumber.make(5),
+            ]);
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(2), Utils.BigNumber.make(4))).toEqual([]);
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(5), Utils.BigNumber.make(2))).toEqual([]);
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(35), Utils.BigNumber.make(556))).toEqual([
+                Utils.BigNumber.make(555),
+            ]);
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(555), Utils.BigNumber.make(1222))).toEqual([]);
+
+            expect(sortedArray.getStrictlyBetween(Utils.BigNumber.make(1), Utils.BigNumber.make(556))).toEqual(bignums);
+        });
+    });
+});

__tests__/unit/core-utils/tree.test.ts

@@ -175,4 +175,25 @@ describe("Tree", () => {
             expect(tree.find(treeId, toRemove)).toBeUndefined();
         }
     });
+
+    describe("getValuesLastToFirst", () => {
+        it("should get the values last to first", () => {
+            const tree = new Tree(compareFunction);
+
+            const bignums: Utils.BigNumber[] = [];
+            for (let i = 0; i < 1000; i++) {
+                const randomBignum = Utils.BigNumber.make(Math.floor(Math.random() * 1000000));
+                if (bignums.find(b => b.isEqualTo(randomBignum))) {
+                    continue;
+                }
+
+                const treeId = randomBignum.toString();
+                tree.insert(treeId, randomBignum);
+                bignums.push(randomBignum);
+            }
+
+            // checking that getAll() returns the bignums sorted just like the classic array sort() function
+            expect(tree.getValuesLastToFirst(1000)).toEqual([...bignums].sort(compareFunction).reverse());
+        });
+    });
 });

packages/core-transaction-pool/src/connection.ts

@@ -98,7 +98,7 @@ export class Connection implements TransactionPool.IConnection {
     public async getSenderSize(senderPublicKey: string): Promise<number> {
         await this.purgeExpired();
 
-        return this.memory.getBySender(senderPublicKey).size;
+        return this.memory.getBySender(senderPublicKey).length;
     }
 
     public async addTransactions(transactions: Interfaces.ITransaction[]): Promise<ITransactionsProcessed> {
@@ -187,7 +187,7 @@ export class Connection implements TransactionPool.IConnection {
             return false;
         }
 
-        return this.memory.getBySender(senderPublicKey).size >= this.options.maxTransactionsPerSender;
+        return this.memory.getBySender(senderPublicKey).length >= this.options.maxTransactionsPerSender;
     }
 
     public flush(): void {
@@ -574,20 +574,8 @@ export class Connection implements TransactionPool.IConnection {
 
         // Revert all transactions that have bigger or equal nonces than the ones in
         // lowestNonceBySender in order from bigger nonce to smaller nonce.
-
         for (const senderPublicKey of Object.keys(lowestNonceBySender)) {
-            const allTxFromSender = Array.from(this.memory.getBySender(senderPublicKey));
-            allTxFromSender.sort((a, b) => {
-                if (a.data.nonce.isGreaterThan(b.data.nonce)) {
-                    return -1;
-                }
-
-                if (a.data.nonce.isLessThan(b.data.nonce)) {
-                    return 1;
-                }
-
-                return 0;
-            });
+            const allTxFromSender = this.memory.getBySender(senderPublicKey).reverse(); // sorted by bigger to smaller nonce
 
             for (const transaction of allTxFromSender) {
                 await purge(transaction);

packages/core-transaction-pool/src/defaults.ts

@@ -6,8 +6,8 @@ export const defaults = {
     // only accepted if its fee is higher than the transaction with the lowest
     // fee in the pool. In this case the transaction with the lowest fee is removed
     // from the pool in order to accommodate the new one.
-    maxTransactionsInPool: process.env.CORE_MAX_TRANSACTIONS_IN_POOL || 100000,
-    maxTransactionsPerSender: process.env.CORE_TRANSACTION_POOL_MAX_PER_SENDER || 300,
+    maxTransactionsInPool: process.env.CORE_MAX_TRANSACTIONS_IN_POOL || 15000,
+    maxTransactionsPerSender: process.env.CORE_TRANSACTION_POOL_MAX_PER_SENDER || 150,
     allowedSenders: [],
     maxTransactionsPerRequest: process.env.CORE_TRANSACTION_POOL_MAX_PER_REQUEST || 40,
     // Max transaction age in number of blocks produced since the transaction was created.