Skip to content

Conversation

@cskiraly
Copy link
Contributor

@cskiraly cskiraly commented Nov 7, 2025

The fetcher should not fetch transactions that are already on chain. Until now we were only checking in the txpool, but that does not have the old transaction. This was leading to extra fetches of transactions that were annoounced by a peer but are already on chain.

Here we extend the check to the chain as well. Still WIP, as this check might be expensive, and there are other options to do the same check.

The fetcher should not fetch transactions that are already on chain.
Until now we were only checking in the txpool, but that does not
have the old transaction.
Here we extend the check to the chain as well. Still WIP, as this
check might be expensive, and there are other options to do the
same check.

Signed-off-by: Csaba Kiraly <[email protected]>
@cskiraly
Copy link
Contributor Author

cskiraly commented Nov 7, 2025

@rjl493456442 , I've committed this fix for now, but there are other options. Curious which one you would prefer:

  • I think the check above is expensive as GetCanonicalTransaction pulls the tx. We can implement a cheaper HasCanonicalTransaction to do the check.
  • I also have a variant that is not looking at the chain, but has a "seen" cache directly in the tx_fetcher.

I can also separate the hasTx callback from a chainTx callback, and pass both to the handler. Seems cleaner.
What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant