feat(rpc): enhance eth_getLogs error handling with block range feedback
#12790
Conversation
| // Check if we have reached the max logs per response and return the last block if so | ||
| if let Some(max_logs_per_response) = max_logs_per_response { | ||
| if log_index >= max_logs_per_response as u64 { | ||
| return Ok(Some(block_num_hash.number.saturating_sub(1))); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
this doesn't look correct to me, this compares the index against the max logs per response. this is not what we want here.
There was a problem hiding this comment.
Oh mb, just need to compare against the logs array length?
There was a problem hiding this comment.
yes, only those should be enforced
crates/rpc/rpc/src/eth/filter.rs
Outdated
| /// Query result is too large. | ||
| #[error("query exceeds max results {0}")] | ||
| QueryExceedsMaxResults(usize), | ||
| #[error("query exceeds max results {0}, retry with the range {1}-{2}")] | ||
| QueryExceedsMaxResults(usize, u64, u64), |
There was a problem hiding this comment.
needs docs what these numbers represent, not obvious
| // Check if we have reached the max logs per response and return the last block if so | ||
| if let Some(max_logs_per_response) = max_logs_per_response { | ||
| if all_logs.len() > max_logs_per_response { | ||
| return Ok(Some(block_num_hash.number.saturating_sub(1))); | ||
| } | ||
| } |
There was a problem hiding this comment.
I find this check a bit out of place here because we now only need this to get the last block hash.
for the purpose of this check, we can move this to
https://github.com/paradigmxyz/reth/pull/12790/files#diff-d45b799806905149d455157b0d93125d80d302c75be6c5c0f84882a45ec9ed6bR522 entirely because we can get the same info from the last log in all_logs then we don't need to apply this check per log
There was a problem hiding this comment.
Moving the check outside would mean we'd always process the entire block range, even when we know early on that the result will exceed the response size limit?
There was a problem hiding this comment.
good point
then lets move this to block scope
Description
Improve the error handling for
eth_getLogsby providing the last successfully processed block number when the maximum logs limit is reached. This allows clients to automatically adjust their block range for subsequent queries.Changes
append_matching_block_logsto returnOption<u64>representing the last valid block number when max logs limit is reachedEthFilterError::QueryExceedsMaxResultsblock_num_hash.number - 1as the last valid block to ensure a safe retry pointMotivation
Currently, when querying logs over a large block range that exceeds the maximum logs limit, clients need to guess a smaller range through trial and error. This enhancement provides explicit feedback about the highest block number that can be safely queried, enabling more efficient log indexing strategies.