Conversation
…ass-through (#2147) * Include range limit filter flag * update docs * apply more range checks
kamuikatsurgi
left a comment
There was a problem hiding this comment.
Opening a minor point for discussion.
| } | ||
| if ctx.IsSet(RPCGlobalLogQueryLimit.Name) { | ||
| cfg.LogQueryLimit = ctx.Int(RPCGlobalLogQueryLimit.Name) | ||
| cfg.RPCLogQueryLimit = ctx.Int(RPCGlobalLogQueryLimit.Name) |
There was a problem hiding this comment.
Can we remove one of the fields? LogQueryLimit is the upstream geth field, while RPCLogQueryLimit was introduced by @marcello33 recently when porting some RPCs from erigon. If we are setting both to the same value, one of them is redundant. Ideally, we should get rid of one of them, preferably RPCLogQueryLimit, to match upstream and use LogQueryLimit everywhere.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
|
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (86.95%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #2150 +/- ##
===========================================
+ Coverage 51.51% 51.59% +0.07%
===========================================
Files 882 882
Lines 154081 154185 +104
===========================================
+ Hits 79377 79550 +173
+ Misses 69526 69460 -66
+ Partials 5178 5175 -3
... and 16 files with indirect coverage changes
🚀 New features to boost your workflow:
|
Description
Cherry pick
2.6.5intodevelopChanges
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it
Nodes audience
In case this PR includes changes that must be applied only to a subset of nodes, please specify how you handled it (e.g. by adding a flag with a default value...)
Checklist
Cross repository changes
Testing
Manual tests
Please complete this section with the steps you performed if you ran manual tests for this functionality, otherwise delete it
Additional comments
Please post additional comments in this section if you have them, otherwise delete it