refactor: strengthen a nonce check in performGenericWalletChecks()

[vasild]

refactor: strengthen a nonce check in performGenericWalletChecks()

In TransactionHandler.performGenericWalletChecks() we would only check
if the transaction's nonce is greater than the sender wallet nonce. The
check should be not just greater, but exactly sender wallet nonce plus
one.

Also, move multiplicated code into two methods and use those methods.

[spkjp]

LGTM. Just need to sort out the failing tests.

[codecov]

Codecov Report

Merging #2949 into develop will increase coverage by 0.09%.
The diff coverage is 85.71%.

@@             Coverage Diff             @@
##           develop    #2949      +/-   ##
===========================================
+ Coverage    68.31%   68.41%   +0.09%     
===========================================
  Files          405      405              
  Lines         9826     9818       -8     
  Branches       464      462       -2     
===========================================
+ Hits          6713     6717       +4     
+ Misses        3077     3065      -12     
  Partials        36       36

Impacted Files	Coverage Δ
...ages/core-transactions/src/handlers/htlc-refund.ts	80.51% <100%> (+2.31%)	⬆️
...kages/core-transactions/src/handlers/htlc-claim.ts	81.39% <100%> (+2.32%)	⬆️
packages/core-transaction-pool/src/connection.ts	92.22% <100%> (ø)	⬆️
packages/core-transaction-pool/src/processor.ts	81.33% <100%> (+2.38%)	⬆️
...ages/core-transactions/src/handlers/transaction.ts	68.33% <100%> (+4.39%)	⬆️
packages/core-state/src/wallets/wallet.ts	85.71% <50%> (-1.52%)	⬇️
packages/crypto/src/validation/keywords.ts	90.74% <0%> (-0.79%)	⬇️
packages/crypto/src/transactions/types/schemas.ts	100% <0%> (ø)	⬆️
packages/core-p2p/src/peer-communicator.ts	61.45% <0%> (+1.04%)	⬆️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebee960...26f980a. Read the comment docs.

[vasild]

This change revealed a flaw in the code: if we receive more than one transaction for addition in the pool, in Processor.validate() / filterAndTransformTransactions() we would call throwIfCannotBeApplied() on each one, without applying any of them:
https://github.com/ArkEcosystem/core/blob/7bfba2710/packages/core-transaction-pool/src/processor.ts#L124
this, obviously, will not work wrt nonce - receiving two transactions from the same sender, we have to apply the first one so that the nonce is incremented before testing if the second can be applied.

There are a few solutions:

• Remove the throwIfCannotBeApplied() call from filterAndTransformTransactions(). It looks unnecessary because later throwIfCannotBeApplied() is called again from the pool's Connection.addTransaction().

• If the call to throwIfCannotBeApplied() from filterAndTransformTransactions() is absolutely unnecessary, then a bigger refactor is due, changing https://github.com/ArkEcosystem/core/blob/7bfba2710/packages/core-transaction-pool/src/processor.ts#L28 to process transactions one by one: calling filterAndTransformTransactions(), removeForgedTransactions() and addTransactionsToPool() for one transaction before proceeding with the next.

• Workaround the flaw from the test, processing the transactions one by one:

             // reset processor, if not the 1st transaction will still be in this.accept and mess up
             processor = transactionPool.makeProcessor();

-            await processor.validate([votes[0].data, delegateRegs[0].data, signatures[0].data]);
+            await processor.validate([votes[0].data]);
+            processor = transactionPool.makeProcessor();
+            await processor.validate([delegateRegs[0].data]);
+            processor = transactionPool.makeProcessor();
+            await processor.validate([signatures[0].data]);

[spkjp]

We could also apply the transactions on a temporary wallet manager, but given that we already call apply (on the pool wallet manager) at the end of the processor, removing the check completely should also be fine.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (develop@1e37cdd). Click here to learn what that means.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##             develop    #2949   +/-   ##
==========================================
  Coverage           ?   68.39%           
==========================================
  Files              ?      405           
  Lines              ?     9827           
  Branches           ?      465           
==========================================
  Hits               ?     6721           
  Misses             ?     3070           
  Partials           ?       36

Impacted Files	Coverage Δ
...ages/core-transactions/src/handlers/htlc-refund.ts	80.51% <100%> (ø)
...kages/core-transactions/src/handlers/htlc-claim.ts	81.39% <100%> (ø)
packages/core-transaction-pool/src/connection.ts	92.22% <100%> (ø)
packages/core-transaction-pool/src/processor.ts	81.33% <100%> (ø)
...ages/core-transactions/src/handlers/transaction.ts	68.33% <100%> (ø)
packages/core-state/src/wallets/wallet.ts	85.71% <50%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e37cdd...e5bc0b9. Read the comment docs.