Fix issue 5824 - Duplicate key errors in EthScheduler-Transactions

[ahamlat]

PR description

Fix Duplicate key errors in EthScheduler-Transactions like this one :

2023-08-29 19:31:22.624+00:00 | EthScheduler-Transactions-0 | ERROR | Besu | Uncaught exception in thread "EthScheduler-Transactions-0"
java.lang.IllegalStateException: Duplicate key 0xf526c3350c599fa381f0a9fd4412105d6e3b84cef407973520a4a536a602806d (attempted merging values ValidationResult{invalidReason=Optional.empty, errorMessage=Optional.empty} and ValidationResult{invalidReason=Optional[TRANSACTION_ALREADY_KNOWN], errorMessage=Optional.empty})
        at java.base/java.util.stream.Collectors.duplicateKeyException(Collectors.java:135)
        at java.base/java.util.stream.Collectors.lambda$uniqKeysMapAccumulator$1(Collectors.java:182)
        at java.base/java.util.stream.ReduceOps$3ReducingSink.accept(ReduceOps.java:169)
        at java.base/java.util.stream.SortedOps$SizedRefSortingSink.end(SortedOps.java:357)
        at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:510)
        at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
        at java.base/java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:921)
        at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
        at java.base/java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:682)
        at org.hyperledger.besu.ethereum.eth.transactions.TransactionPool.addRemoteTransactions(TransactionPool.java:212)
        at org.hyperledger.besu.ethereum.eth.transactions.TransactionsMessageProcessor.processTransactionsMessage(TransactionsMessageProcessor.java:87)
        at org.hyperledger.besu.ethereum.eth.transactions.TransactionsMessageProcessor.processTransactionsMessage(TransactionsMessageProcessor.java:60)
        at org.hyperledger.besu.ethereum.eth.transactions.TransactionsMessageHandler.lambda$exec$0(TransactionsMessageHandler.java:51)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
        at java.base/java.lang.Thread.run(Thread.java:833)

Unit Test without the modification in TransactionPool

Unit Test with the modification in TransactionPool

Fixed Issue(s)

#5824

[github-actions]

• I thought about documentation and added the doc-change-required label to this PR if updates are required.
• I thought about the changelog and included a changelog update if required.
• If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

[siladu]

I understand that the toMap merge function picks one of two transactions when there's a key (tx hash) collision, but I have couple of questions:

1. How do we end up with duplicate transactions? Wondering if this might be hiding another root cause. Or maybe it's just data from the network we can't control so this is fine.

2. How do we know picking tx1 is correct over tx2? Do matches hashes imply the contents are identical too?

[ahamlat]

My understanding is that we cannot have two transactions with the same hash and different content, it is a fundamental property of hash functions.
I think we have duplicate transactions because we receive them from different peers.

[siladu]

Thanks for answering my questions. Have added a test name suggestion but it's not a blocker for me.