Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update trace_filter, trace block only to highest matching index #4974

Merged
merged 22 commits into from
Oct 12, 2023
Merged
17 changes: 11 additions & 6 deletions crates/rpc/rpc/src/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,29 +276,34 @@ where
let mut target_blocks = Vec::new();
for block in blocks {
let mut transaction_indices = HashSet::new();
let mut highest_matching_index = 0;
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) {
transaction_indices.insert(tx_idx as u64);
let idx = tx_idx as u64;
transaction_indices.insert(idx);
if idx > highest_matching_index {
highest_matching_index = idx;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is redundant because all tx_idx are only increasing

}
}
if !transaction_indices.is_empty() {
target_blocks.push((block.number, transaction_indices));
target_blocks.push((block.number, transaction_indices, highest_matching_index));
}
}

// TODO: this could be optimized to only trace the block until the highest matching index in
// that block

// trace all relevant blocks
let mut block_traces = Vec::with_capacity(target_blocks.len());
for (num, indices) in target_blocks {
for (num, indices, highest_idx) in target_blocks {
let traces = self.trace_block_with(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's something else we need to do here:
skipping blocks with empty indices entirely

num.into(),
TracingInspectorConfig::default_parity(),
move |tx_info, inspector, res, _, _| {
if let Some(idx) = tx_info.index {
if idx > highest_idx {
return Ok(None);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a smol improvement, however this still traces the entire block,
https://github.com/paradigmxyz/reth/blob/main/crates/rpc/rpc/src/trace.rs#L353-L362

if !indices.contains(&idx) {
// only record traces for relevant transactions
return Ok(None)
Expand Down