Disable NonceGap check for internal transactions

[deffrian]

Fixes | Closes | Resolves #

• Fixes Nonce increment bugs #4845

Changes:

• Disables NonceGap check for internal transactions
• Removes ReuseOwnTxNonce filter
• All nonce management has been moved outside tx pool
• Transaction pool can't process multiple transactions from the same address in parallel

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation Update
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Other (please describe):

Testing

Tested manually with sepolia node.
Hive tests: rpc, rpc-compat

Requires testing

• Yes
• No

In case you checked yes, did you write tests??

• Yes
• No

[marcindsobczak]

We have a bug in incrementing nonces. When receiving eth_sendTransaction:
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.JsonRpc/Modules/Eth/EthRpcModule.cs#L325
we firstly reserve nonce:
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/TxPoolSender.cs#L24
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/TxNonceTxPoolReserveSealer.cs#L37
and then go to the filters (filters are in SubmitTx):
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/TxPoolSender.cs#L25
If tx is rejected in filters, nonce remains incremented and for next transactions send from this endpoint nonce will be incorrect. In this form (not rejecting nonce-gap transactions) we will accept this new tx with higher nonce, but it will never be processed, as lower nonce doesn't exist, so it will be some kind of invalid in a hidden way, without notice to the sender

[gituser]

Any update on this? @marcindsobczak @deffrian
Thanks.

[deffrian]

@gituser I'm working on it right now:)

[marcindsobczak]

There is a conflict to be resolved @deffrian

[deffrian]

There is a conflict to be resolved @deffrian

@marcindsobczak merged

[marcindsobczak]

There is a regression in POSDAO tests.

Post-merge POSDAO - I tried 2 times and have 2 fails:
https://github.com/NethermindEth/nethermind/actions/runs/3647856241/jobs/6160583037
https://github.com/NethermindEth/nethermind/actions/runs/3647926505/jobs/6160732425
From master we are passing it:
https://github.com/NethermindEth/nethermind/actions/runs/3647922863/jobs/6160723733

Pre-merge POSDAO - fails as well:
https://github.com/NethermindEth/nethermind/actions/runs/3647857359/jobs/6160585145
And pass from master:
https://github.com/NethermindEth/nethermind/actions/runs/3647923297/jobs/6160724754

[marcindsobczak]

One more regression - in hive transition tests.

This branch fails:
https://github.com/NethermindEth/nethermind/actions/runs/3648552791/jobs/6162106175
Master pass:
https://github.com/NethermindEth/nethermind/actions/runs/3648553594/jobs/6162107696

[LukaszRozmej]

I think this design has its own problems as highlighted in the review. Do we need Set of nonces at all? Maybe we should follow what Geth is doing and always store local transactions in the pool?

Other than that it has bugs and missed things that could be improved.