node/cli: register --rpc.logs.maxresults in DefaultFlags so it takes effect via CLI#21426
Merged
Merged
Conversation
…effect via CLI Forward-port of #21389 to main. `RpcGetLogsMaxResults` was defined in `cmd/utils/flags.go` and consumed in `node/cli/flags.go` (`ApplyFlagsForHttpCfg`), but never added to the `DefaultFlags` slice, so the `erigon` binary rejected the flag at startup: Error: flag provided but not defined: -rpc.logs.maxresults Same bug that #21372 reported against `release/3.4`; the fix landed there as #21389 but main was not updated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Registers the existing --rpc.logs.maxresults CLI flag in the Erigon node binary’s DefaultFlags, ensuring the flag is actually recognized at startup and correctly wires through to HTTP RPC configuration (consistent with how rpc.blockrange.limit is handled).
Changes:
- Add
utils.RpcGetLogsMaxResultstonode/cli’sDefaultFlagsso theerigonbinary registers--rpc.logs.maxresults.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lupin012
approved these changes
May 26, 2026
pull Bot
pushed a commit
to Dustin4444/erigon
that referenced
this pull request
May 27, 2026
…igontech#21445) ## Problem The merge-queue fail-fast optimization (erigontech#20789) cancels the entire CI Gate run as soon as one leaf job fails, so the gate doesn't stall ~30 min waiting for the heavy jobs (hive/kurtosis/eest) to finish. The downside: `gh run cancel` cancels the *whole* run including the leaf that called it, so the real culprit's conclusion flips `failure → cancelled` — visually identical to every innocent sibling. The `ci-gate` aggregator then lumps `failure` and `cancelled` together and prints only job names, so finding the actual failure means drilling into step-level conclusions by hand. This surfaced when erigontech#21426 (a green, approved one-line PR) was silently evicted from the merge queue: a flaky data race in `TestHistoryVerification_SimpleBlocks` tripped the fast-cancel, but the run showed a sea of "cancelled" jobs with no indication which one failed. ## Fix Keep the latency win; make the root cause prominent. - **Each leaf** emits a GitHub `::error` annotation right before `gh run cancel`. Only the *true trigger* reaches this step — collateral jobs have it `skipped` by the in-progress cancellation — so the annotation is attributed to the actual failing job and shows at the top of the run + in the PR Checks tab. - **The `ci-gate` aggregator** now names the root cause instead of dumping ambiguous job names. It identifies the trigger precisely — the job whose "Cancel workflow run on failure" step actually ran (`success`) — and annotates it plus its failing step. Falls back to listing all genuinely-failed jobs on `pull_request` runs (where no fast-cancel fires). ## Validation - `actionlint` clean on all changed files (no new findings; the pre-existing shellcheck infos are untouched). - Replayed the aggregator's query against the real failed run that evicted erigontech#21426: it now outputs exactly `::error … race-tests … (execution-other, serial) — failed step: Run execution-other tests`, isolating the real culprit and excluding the 4 collateral `hive-eest` failures. No behavior change to the fast-cancel itself — only added visibility. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: info@weblogix.biz <admin@10gbps.weblogix.it>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Forward-port of #21389 to
main.RpcGetLogsMaxResultswas defined incmd/utils/flags.goand consumed innode/cli/flags.go(ApplyFlagsForHttpCfgpopulatesGetLogsMaxResults), but it was never added to theDefaultFlagsslice innode/cli/default_flags.go. As a result theerigonbinary does not register the flag and rejects it at startup:Same bug that #21372 reported against
release/3.4; the fix landed there as #21389.mainwas missed.One-line addition, placing the entry next to the already-registered
--rpc.blockrange.limit:&utils.RpcBlockRangeLimit, +&utils.RpcGetLogsMaxResults, &utils.RpcBatchLimit,Test plan
make lintcleango build ./node/cli/...cleanerigon --rpc.logs.maxresults=10000 ...starts (no "flag provided but not defined")🤖 Generated with Claude Code