if checkTxnPool true, try to fetch from pool first

[mralj]

Changes

• When fetching a tx, if checkTxnPool is set to true, firstly try to fetch the tx from the pool

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

Documentation

Requires documentation update

• Yes
• No

Requires explanation in Release Notes

• Yes
• No

Remarks

This heuristic should be better than the previous one, if the caller of the method expects the tx to be in the pool, we should check it first.

[LukaszRozmej]

Why is that important? Theoretically we remove canonical transactions from pool in background thread, so we could have canonical transaction with a receipt still in the pool.

[mralj]

I wouldn't say it's important, but it seemed to make more sense to check the pool first, as it would bring small perf boost in scenarios where e.g. user subscribes to the pending transactions and then fetches each of these pending txs by hash (all of them would be in the pool).
Given there is the edgewise you mentioned, feel free to close the PR if you think the scenario above isn't worth optimizng for :)

[LukaszRozmej]

@marcindsobczak WDYT?

If we want we could potentially wait till channel becomes empty. We can add manual counter and reset event.

@marcindsobczak WDYT?

Made an attempt in #8348

[marcindsobczak]

IMO we can go with it as it is.
It is used in two places: eth_getRawTransactionByHash and eth_getTransactionByHash. For the first one, there is no issue, as it doesn't require the receipt. For the second, after the changes, we might occasionally return a recently processed transaction without the receipt. However, in my opinion, this is fine, and this method doesn’t need to be 100% accurate. If someone needs the receipt, they can always request it again.

[LukaszRozmej]

IMO we can go with it as it is. It is used in two places: eth_getRawTransactionByHash and eth_getTransactionByHash. For the first one, there is no issue, as it doesn't require the receipt. For the second, after the changes, we might occasionally return a recently processed transaction without the receipt. However, in my opinion, this is fine, and this method doesn’t need to be 100% accurate. If someone needs the receipt, they can always request it again.

I disagree, if someone is notified about the head change, we can't give him inconsistent view of transaction

[marcindsobczak]

Right. So we need either both this PR and #8348, or neither. We should consider if such changes are justified just for a micro-optimization.

[LukaszRozmej]

Right. So we need either both this PR and #8348, or neither. We should consider if such changes are justified just for a micro-optimization.

So for suggested use - builder wants to get transactions from mempol ASAP this is probably not that micro as can skip db call. If you take into account that we can have hundreds of concurrent calls for transactions. But it is a concreate scenario.

[mralj]

Right. So we need either both this PR and #8348, or neither. We should consider if such changes are justified just for a micro-optimization.

So for suggested use - builder wants to get transactions from mempol ASAP this is probably not that micro as can skip db call. If you take into account that we can have hundreds of concurrent calls for transactions. But it is a concreate scenario.

Not only builder, but searcher as well, basically anyone who is subscribed to the mempool tx stream.
IMO we should optimise for this, because, it's highly likely (in these scenarios) that TX will be in the pool, and in that case I think we should deliver it with the least possible latency.
If the TX isn't in the pool then, overhead of firstly checking the pool doesn't matter.

[Copilot]

Pull Request Overview

This PR optimizes transaction retrieval by checking the transaction pool first when fetching a transaction if the checkTxnPool flag is true.

• Prioritizes pending transactions from the pool if available
• Falls back to fetching canonical transactions when no pool transaction is found

[benaadams]

Added "Don not merge" as dependent on other PR