refactor(pool): remove unnecessary check from validateTransactions() (#…

…2951)

* refactor(pool): remove unnecessary check from validateTransactions()

We would call

  handler.throwIfCannotBeApplied(transaction, sender, databaseWalletManager);

followed by

  handler.applyToSender(transaction, localWalletManager);

but applyToSender() calls throwIfCannotBeApplied() internally, so we
don't need to call it ourselves. Also, it was wrong to use the
databaseWalletManager in that call because we are applying the
transactions to localWalletManager, so the databaseWalletManager will be
outdated (e.g. sender nonce and balance will be stale in
databaseWalletManager).

So, remove the unnecessary call to throwIfCannotBeApplied().

* test: adjust "should handle getTransaction() not finding transaction"

throwIfCannotBeApplied() is not called directly anymore. It is only
called from applyToSender(), but that method is mocked.

* test: remove irrelevant test

We don't call throwIfCannotBeApplied() before applyToSender() anymore,
so the test becomes irrelevant - it fakes an exception from
throwIfCannotBeApplied() and checks that applyToSender() is not called.

* test(core-transaction-pool): fix test to use correct nonces

The test was trying to use transaction mockData.dummyLarge1 which has
nonce=2, but without having prior transactions from that sender applied.

So, use mockData.dummy10 which is from that sender and has nonce=1.

* test: fix to not break due to leftovers from previous tests

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

@@ -779,30 +779,8 @@ describe("Connection", () => {
 
             expect(getTransaction).toHaveBeenCalled();
             expect(findByPublicKey).not.toHaveBeenCalled();
-            expect(throwIfCannotBeApplied).toHaveBeenCalled();
             expect(applyToSender).toHaveBeenCalled();
         });
-
-        it("should not apply transaction to wallet if throwIfCannotBeApplied() failed", async () => {
-            const transactionHandler = await Handlers.Registry.get(TransactionType.Transfer);
-            throwIfCannotBeApplied = jest.spyOn(transactionHandler, "throwIfCannotBeApplied").mockImplementation(() => {
-                throw new Error("throw from test");
-            });
-            const purgeByPublicKey = jest.spyOn(connection, "purgeByPublicKey").mockReturnValue();
-
-            updateSenderNonce(mockData.dummy1);
-            addTransactions([mockData.dummy1]);
-
-            await connection.buildWallets();
-
-            expect(applyToSender).not.toHaveBeenCalled();
-            expect(throwIfCannotBeApplied).toHaveBeenCalledWith(
-                mockData.dummy1,
-                findByPublicKeyWallet,
-                (connection as any).databaseService.walletManager,
-            );
-            expect(purgeByPublicKey).not.toHaveBeenCalledWith(mockData.dummy1.data.senderPublicKey);
-        });
     });
 
     describe("senderHasTransactionsOfType", () => {
@@ -845,14 +823,20 @@ describe("Connection", () => {
         it("save and restore transactions", async () => {
             await expect(connection.getPoolSize()).resolves.toBe(0);
 
+            // Reset the senders' nonces to cleanup leftovers from previous tests.
+            updateSenderNonce(mockData.dummy1);
+            updateSenderNonce(mockData.dummy10);
+
+            // Be sure to use transactions with appropriate nonce - can't fire a transaction
+            // with nonce 5 if the sender wallet has nonce 1, for example.
+            const transactions = [mockData.dummy1, mockData.dummy10, mockData.dummyLarge1];
+
             indexWalletWithSufficientBalance(mockData.dummy1);
             indexWalletWithSufficientBalance(mockData.dummyLarge1);
 
-            const transactions = [mockData.dummy1, mockData.dummyLarge1];
-
             addTransactions(transactions);
 
-            await expect(connection.getPoolSize()).resolves.toBe(2);
+            await expect(connection.getPoolSize()).resolves.toBe(transactions.length);
 
             connection.disconnect();
 
@@ -862,7 +846,7 @@ describe("Connection", () => {
 
             await delay(200);
 
-            await expect(connection.getPoolSize()).resolves.toBe(2);
+            await expect(connection.getPoolSize()).resolves.toBe(transactions.length);
 
             for (const t of transactions) {
                 expect((await connection.getTransaction(t.id)).serialized).toEqual(t.serialized);

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

@@ -500,7 +500,6 @@ export class Connection implements TransactionPool.IConnection {
                     transaction.type,
                     transaction.typeGroup,
                 );
-                await handler.throwIfCannotBeApplied(transaction, sender, databaseWalletManager);
 
                 await handler.applyToSender(transaction, localWalletManager);