Skip to content

node/cli: register --rpc.logs.maxresults in DefaultFlags so it takes effect via CLI#21426

Merged
lupin012 merged 1 commit into
mainfrom
yperbasis/rpc-logs-maxresults-default-flags
May 26, 2026
Merged

node/cli: register --rpc.logs.maxresults in DefaultFlags so it takes effect via CLI#21426
lupin012 merged 1 commit into
mainfrom
yperbasis/rpc-logs-maxresults-default-flags

Conversation

@yperbasis

Copy link
Copy Markdown
Member

Summary

Forward-port of #21389 to main.

RpcGetLogsMaxResults was defined in cmd/utils/flags.go and consumed in node/cli/flags.go (ApplyFlagsForHttpCfg populates GetLogsMaxResults), but it was never added to the DefaultFlags slice in node/cli/default_flags.go. As a result the erigon binary does not register the flag and rejects it at startup:

Error: flag provided but not defined: -rpc.logs.maxresults
Run 'erigon --help' for usage.

Same bug that #21372 reported against release/3.4; the fix landed there as #21389. main was missed.

One-line addition, placing the entry next to the already-registered --rpc.blockrange.limit:

 &utils.RpcBlockRangeLimit,
+&utils.RpcGetLogsMaxResults,
 &utils.RpcBatchLimit,

Test plan

  • make lint clean
  • go build ./node/cli/... clean
  • CI passes
  • After merge: confirm erigon --rpc.logs.maxresults=10000 ... starts (no "flag provided but not defined")

🤖 Generated with Claude Code

…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>
@yperbasis yperbasis requested a review from AskAlexSharov as a code owner May 26, 2026 15:36
@yperbasis yperbasis enabled auto-merge May 26, 2026 15:40
@yperbasis yperbasis added this to the 3.5.0 milestone May 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.RpcGetLogsMaxResults to node/cli’s DefaultFlags so the erigon binary registers --rpc.logs.maxresults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@yperbasis yperbasis added this pull request to the merge queue May 26, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 26, 2026
@lupin012 lupin012 added this pull request to the merge queue May 26, 2026
Merged via the queue into main with commit ce94063 May 26, 2026
95 checks passed
@lupin012 lupin012 deleted the yperbasis/rpc-logs-maxresults-default-flags branch May 26, 2026 20:43
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants