fix(core-transaction-pool): sort by fee, nonce (#2937)

The merge 3923c37 wrongly removed the method sortAll() from memory.ts.
This method was sorting transactions in the pool by fee, nonce (sorted
by highest fee first, but transactions from the same sender must be
sorted lowest nonce first).

Restore the sortAll() method as of before the merge and adapt it to use
the calculateTransactionExpiration() method for expiration sorting.

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

@@ -1044,8 +1044,7 @@ describe("Connection", () => {
             expect(topFeesReceived).toEqual(topFeesExpected);
         });
 
-        // TODO: @vd connection.getTransactions(...) doesn't return sorted by nonce
-        it.skip("sort by fee, nonce", async () => {
+        it("sort by fee, nonce", async () => {
             const nTransactions = 1000;
             const nDifferentSenders = 100;
 

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

@@ -31,35 +31,7 @@ export class Memory {
 
     public allSortedByFee(): Interfaces.ITransaction[] {
         if (!this.allIsSorted) {
-            const currentHeight: number = this.currentHeight();
-            const expirationContext = {
-                blockTime: Managers.configManager.getMilestone(currentHeight).blocktime,
-                currentHeight,
-                now: Crypto.Slots.getTime(),
-            };
-
-            this.all.sort((a, b) => {
-                const feeA: Utils.BigNumber = a.data.fee;
-                const feeB: Utils.BigNumber = b.data.fee;
-
-                if (feeA.isGreaterThan(feeB)) {
-                    return -1;
-                }
-
-                if (feeA.isLessThan(feeB)) {
-                    return 1;
-                }
-
-                const expirationA: number = this.calculateTransactionExpiration(a, expirationContext);
-                const expirationB: number = this.calculateTransactionExpiration(b, expirationContext);
-
-                if (expirationA !== null && expirationB !== null) {
-                    return expirationA - expirationB;
-                }
-
-                return 0;
-            });
-
+            this.sortAll();
             this.allIsSorted = true;
         }
 
@@ -286,6 +258,79 @@ export class Memory {
         return removed;
     }
 
+    /**
+     * Sort `this.all` by fee (highest fee first) with the exception that transactions
+     * from the same sender must be ordered lowest `nonce` first.
+     */
+    private sortAll(): void {
+        const currentHeight: number = this.currentHeight();
+        const expirationContext = {
+            blockTime: Managers.configManager.getMilestone(currentHeight).blocktime,
+            currentHeight,
+            now: Crypto.Slots.getTime(),
+        };
+
+        this.all.sort((a, b) => {
+            const feeA: Utils.BigNumber = a.data.fee;
+            const feeB: Utils.BigNumber = b.data.fee;
+
+            if (feeA.isGreaterThan(feeB)) {
+                return -1;
+            }
+
+            if (feeA.isLessThan(feeB)) {
+                return 1;
+            }
+
+            const expirationA: number = this.calculateTransactionExpiration(a, expirationContext);
+            const expirationB: number = this.calculateTransactionExpiration(b, expirationContext);
+
+            if (expirationA !== null && expirationB !== null) {
+                return expirationA - expirationB;
+            }
+
+            return 0;
+        });
+
+        const indexBySender = {};
+        for (let i = 0; i < this.all.length; i++) {
+            const transaction: Interfaces.ITransaction = this.all[i];
+
+            if (transaction.data.version < 2) {
+                continue;
+            }
+
+            const sender: string = transaction.data.senderPublicKey;
+            if (indexBySender[sender] === undefined) {
+                indexBySender[sender] = [];
+            }
+            indexBySender[sender].push(i);
+
+            let nMoved = 0;
+
+            for (let j = 0; j < indexBySender[sender].length - 1; j++) {
+                const prevIndex: number = indexBySender[sender][j];
+                if (this.all[i].data.nonce.isLessThan(this.all[prevIndex].data.nonce)) {
+                    const newIndex = i + 1 + nMoved;
+                    this.all.splice(newIndex, 0, this.all[prevIndex]);
+                    this.all[prevIndex] = undefined;
+
+                    indexBySender[sender][j] = newIndex;
+
+                    nMoved++;
+                }
+            }
+
+            if (nMoved > 0) {
+                indexBySender[sender].sort((a, b) => a - b);
+            }
+
+            i += nMoved;
+        }
+
+        this.all = this.all.filter(t => t !== undefined);
+    }
+
     private currentHeight(): number {
         return app
             .resolvePlugin<State.IStateService>("state")