feat: make trace_filter block range configurable #14939
Conversation
stevencartavia
requested review from
mattsse,
Rjected,
onbjerg,
klkvr and
gakonst
as code owners
crates/rpc/rpc/src/trace.rs
Outdated
| @@ -257,7 +258,7 @@ where | |||
|
|
|||
| // ensure that the range is not too large, since we need to fetch all blocks in the range | |||
| let distance = end.saturating_sub(start); | |||
| if distance > 100 { | |||
| if distance > DEFAULT_MAX_TRACE_FILTER_BLOCKS { | |||
There was a problem hiding this comment.
this should use the configured max blocks right? instead of the const
crates/rpc/rpc/src/trace.rs
Outdated
| let eth_config = EthConfig::default(); | ||
| let max_blocks_per_filter = eth_config.max_trace_filter_blocks; |
There was a problem hiding this comment.
we can't use default() here, we need to actually pass down the config into the trace api
see how we do it similarly in EthFilterInner here
reth/crates/rpc/rpc/src/eth/filter.rs
Lines 370 to 371 in 57bb9ba
and make it accessible in the rpc method here
reth/crates/rpc/rpc/src/eth/filter.rs
Line 495 in 57bb9ba
|
🔥 |
crates/rpc/rpc/src/trace.rs
Outdated
| pub fn new(eth_api: Eth, blocking_task_guard: BlockingTaskGuard) -> Self { | ||
| let inner = Arc::new(TraceApiInner { eth_api, blocking_task_guard }); | ||
| Self { inner } | ||
| Self { inner, eth_config: EthConfig::default() } |
There was a problem hiding this comment.
I don't think this works just yet, but nothing is being configured here and this will always use the default
There was a problem hiding this comment.
so what we need to do here is provide the ethconfig as arg, or just the max_trace_filter_blocks
crates/rpc/rpc/src/trace.rs
Outdated
| @@ -38,6 +38,7 @@ use tokio::sync::{AcquireError, OwnedSemaphorePermit}; | |||
| /// This type provides the functionality for handling `trace` related requests. | |||
| pub struct TraceApi<Eth> { | |||
| inner: Arc<TraceApiInner<Eth>>, | |||
| eth_config: EthConfig, | |||
There was a problem hiding this comment.
we also want the settings in the inner type, that's what the inner type is for that that cloning TraceApi per request is as cheap as possible
should close #14930