feat(provider): add block_range_with_senders

[yash-atreya]

Mentioned in #5497 here

#5620 and #5630 address performance improvements in block-level tracing by retrieving senders of the txs from cache instead of recovering the signer from signed_transaction.
This PR attempts to make a similar enhancement, however, by using the db instead of the cache.

a2a6a05 adds a new method to BlockReader for reading a range of blocks (like block_range) along with senders. This method is named block_range_with_senders and returns ProviderResult<Vec<BlockWithSenders>>.

5f69696 replace block_range with block_range_with_senders in trace_filter

[mattsse]

ptal @joshieDo

[joshieDo]

i like the direction, especially because we were recovering for every tx individually. However... Senders table can actually be pruned, and this would mess up this call.

There's Block::with_senders which recovers all senders for a block in parallel if worth it. There should be some conditional here, if we werent able to read all from the DB, then calculate it using this.

A benchmark of Block::with_senders vs reading from the table would be interesting, i'm not sure it's always worth it to read from db instead of calculating.

[joshieDo]

fn block_with_senders actually calls with_senders and doesn't read from the DB. We probably want to harmonize here, but I don't know which one is faster (maybe doing some simple heuristic here). but there should always be the fallback of calculating it in case its pruned

[yash-atreya]

@joshieDo @mattsse
Just summarizing how to move forward from here:

• Check if the Senders table is pruned.

• Fetch from db using Block::with_senders if not pruned or recover_signers if pruned or if some senders are not found for the given range

• Benchmark recovering vs reading from db

[gakonst]

@joshieDo I think this is right?

@yash-atreya do you have a rough timeline for moving forward with this PR?

[yash-atreya]

@yash-atreya do you have a rough timeline for moving forward with this PR?

Will try to get this over the line by the end of the coming week.

[yash-atreya]

Hey @joshieDo @mattsse ptal.

I was able to test on my archive node.

Before i.e latest alpha.16:

After improvement:

However, this is the best case improvement without having to recover any signer as I'm an running an archive.

The margin of improvement will decrease with different pruning configuration of a node.

A benchmark of Block::with_senders vs reading from the table would be interesting, i'm not sure it's always worth it to read from db instead of calculating.

I'd like to test and benchmark this against different pruning configurations. But need some pointers about how to navigate that.

[mattsse]

I wonder if we can use the two existing functions here and zip the results?

block_range and get_take_block_transaction_range

[mattsse]

can we use the existing function here?

[yash-atreya]

1. block_range can be used here, but we still need the body_indices cursor to get the TxNumber's in order to get the tx_range to walk using the senders_cursor and also populate any missing senders after recovering them.
2. get_take_block_transaction_range cannot be used as it will require changing the trait bounds of the BlockReader implementation to add DbTxMut.

[yash-atreya]

Moreover, since this is a read-only method and TAKE would always be false in get_take_block_transaction_range. I don't think it makes sense to use it.

[mattsse]

ptal @joshieDo

I don't like how this effectively duplicates the other functions, this should be easier

[mattsse]

friendly bump @joshieDo

[yash-atreya]

@joshieDo friendly bump here.

[joshieDo]

Sorry for the delay, actually missed the numerous notifications. Unfortunately with #5191 , some of this code has to be transitioned to the new paradigm, where Headersand Transactions live both on database and static files.

There are some examples on the usage (eg. ProviderFactory::receipts_by_tx_range, DatabaseProvider::transactions_by_tx_range_with_cursor), to get data from both storage options under one api.

Once again I'm sorry, I can take this PR forward with the new changes starting next week or two if you wish, given the delay and overall changes

[gakonst]

Let's pause on this as not P0 and Yash's on Foundry stuff, and we can revisit if you have capacity to take over post beta @joshieDo ?