Use already recovered transaction during tracing

[mattsse]

Describe the feature

with #5302

we're caching the senders of transactions, so we no longer need to recover them during tracing

TODO

use SealedBlockWithSenders during block level tracing

Additional context

No response

[yash-atreya]

Can take this !

[mattsse]

sweet!, lmk if you need pointers, this affects trace_ debug_ rpc calls that fetch the full blocks

[gakonst]

@yash-atreya do let us know when you intend to take it on ^^ happy to guide in any way. thank you!!!

[yash-atreya]

Hey @gakonst @mattsse

I was going to start working on this on Thursday.

Do you intend to include this in alpha.11? Will try to do it sooner then.

[mattsse]

thursday's fine :)

thanks in advance 🙏

[yash-atreya]

Hey @mattsse

Extremely sorry for the delay.

Needed some guidance to implement this. I have identified the block level tracing methods where the cached senders should be used instead of recovering the signers.

1. trace_block
   Under the hood, this function uses fn trace_block_until to generate the traces.
   

   reth/crates/rpc/rpc/src/eth/api/transactions.rs

   Line 762 in 025511b

   async fn trace_block_until<F, R>(

   
   trace_block_until calls self.block_by_id to get the SealedBlock
   

   reth/crates/rpc/rpc/src/eth/api/transactions.rs

   Lines 781 to 782 in 025511b

   	let ((cfg, block_env, _), block) =
   	futures::try_join!(self.evm_env_at(block_id), self.block_by_id(block_id),)?;

   
   Enhancement:
   Use self.block_with_senders(blockId) which returns SealedBlockWithSenders. This removes the need to do the following
   

   reth/crates/rpc/rpc/src/eth/api/transactions.rs

   Line 808 in 025511b

   let tx = tx.into_ecrecovered().ok_or(BlockError::InvalidSignature)?;

   instead we can do:

let tx = TransactionSignedEcRecovered::from_signed_transaction(tx, signer); // Signer is from block.senders[idx]

2. trace_filter
   The above enhancement will apply here as this function uses trace_block_until.
   Moreover, this function fetches blocks in the specified range:
   

   reth/crates/rpc/rpc/src/trace.rs

   Line 260 in 025511b

   let blocks = self.provider().block_range(start..=end)?;

   
   And the signers of the fetched blocks are recovered and used in the match filter like so:
   

   reth/crates/rpc/rpc/src/trace.rs

   Lines 267 to 275 in 025511b

   	for (tx_idx, tx) in block.body.iter().enumerate() {
   	let from = tx.recover_signer().ok_or(BlockError::InvalidSignature)?;
   	let to = tx.to();
   	if matcher.matches(from, to) {
   	let idx = tx_idx as u64;
   	transaction_indices.insert(idx);
   	highest_matching_index = idx;
   	}
   	}

   
   Enhancement:
   Add new method fn block_range_with_senders which fetches the range of block and returns ProviderResult<Vec<BlockWithSenders>>. We do not need to recover the signers now. However, this uses the db and not the cache.

3. replay_block_transactions
   This method also uses trace_block_until under the hood, and the initial enhancement also applies here.

I hope I'm on the right track, would appreciate your input on this.

[mattsse]

@yash-atreya sorry I forgot to respond to this one here.
your PR was great, so now we can the same thing for:

https://github.com/paradigmxyz/reth/blob/main/crates/rpc/rpc/src/eth/api/call.rs#L74-L74

and all the others that fetch blocks for tracing

[yash-atreya]

@yash-atreya sorry I forgot to respond to this one here. your PR was great, so now we can the same thing for:

https://github.com/paradigmxyz/reth/blob/main/crates/rpc/rpc/src/eth/api/call.rs#L74-L74

and all the others that fetch blocks for tracing

@mattsse Thanks. I'll get on it.

Just wanted to confirm whether I should move forward with the following enhancement I suggested for trace_filter, which uses the db instead of the cache to retrieve the block range with senders. I'll create a separate PR for this.

Enhancement:
Add new method fn block_range_with_senders which fetches the range of block and returns ProviderResult<Vec>. We do not need to recover the signers now. However, this uses the db and not the cache.

[mattsse]

Just wanted to confirm whether I should move forward with the following enhancement I suggested for trace_filter, which uses the db instead of the cache to retrieve the block range with senders. I'll create a separate PR for this.

need to think about it, but I believe we want this yes

[yash-atreya]

Just wanted to confirm whether I should move forward with the following enhancement I suggested for trace_filter, which uses the db instead of the cache to retrieve the block range with senders. I'll create a separate PR for this.

need to think about it, but I believe we want this yes

Have created the PR in case you need it. #5647

[yash-atreya]

Hey @mattsse, the last few PRs have been extremely fun, thank you for your guidance.
Feel free to assign me more issues, I'll be on the lookout as well.

[mattsse]

ty!
if you don't find any interesting issue please ping dm on tg or twitter and I'll come up if something