#34950 rpc: migrate generatetoaddress params to self.Arg for consistency

full analysis

https://github.com/bitcoin/bitcoin/pull/34950 · defenwycke · +6/-3 in 2 files, 1 commits · labels: RPC/REST/ZMQ

Goal

  • Make parameter parsing consistent across testing RPCs
  • Prevent negative retry counts from silently overflowing to maximum integers

Migrates parameter extraction in the `generatetoaddress` RPC from `request.params[]` indexing to the `self.Arg<>()` helper. This aligns it with `generatetodescriptor` and adds an explicit check in the functional tests verifying that negative `maxtries` values are rejected rather than wrapping to `UINT64_MAX`.

Problem: Parameter parsing across testing RPCs is inconsistent, with `generatetoaddress` using legacy index-based parsing that silently overflows negative integers while `generatetodescriptor` uses the newer `self.Arg` API.

Category: RPC / REST / ZMQ (#47 of 52)

P4 · cleanup

  • P4 because this is a minor internal cleanup of RPC parameter parsing
  • Negative integer wrapping was harmless so the change is purely for code consistency

P4 because this is a minor internal cleanup of RPC parameter parsing. maflcko noted that the negative integer wrapping was harmless and that the change is purely for consistency across RPC helpers.

Membership: Changes parameter extraction in the generatetoaddress RPC and updates functional test test/functional/rpc_generate.py.

Factors: security/stability 0, bug 1, performance 0, user value 0, leverage 0

Reviewability: Stale: Author silent 170d

  • Review is likely not worthwhile because the author has been inactive for 170 days

The author has been inactive for 170 days, well exceeding the 60-day project staleness threshold, leaving dergoegge's prompt about the PR's utility unaddressed.

Author status: silent since 2026-03-30

Open concerns:

  • dergoegge questioned the value of LLM-generated PRs and asked the author to reconsider pursuing the change
  • GerardoTaboada noted that changing negative maxtries handling is user-visible behavior that might require release notes

Resolved concerns:

  • Author fixed Qt build failure caused by using `std::string` instead of `std::string_view` with `self.Arg`

Agreement: Positive

  • Support for aligning parameter parsing with other generate calls (maflcko, GerardoTaboada)
  • Noted that rejecting negative retry counts changes user-visible behavior (GerardoTaboada)
  • Thinks low-value LLM-assisted cleanups are not worth review bandwidth (dergoegge)

Positive on the code change, though dergoegge raised process concerns regarding LLM contributions.

maflcko and GerardoTaboada concurred that aligning with generatetodescriptor makes sense. dergoegge expressed skepticism regarding scarce review bandwidth being spent on LLM-assisted cleanups, but did not formally block the PR.

  • maflcko noted that consistency makes sense as a cleanup
  • GerardoTaboada commented 'Looks good to me. Aligning with generatetodescriptor is the obvious cleanup'
  • dergoegge urged author to reconsider whether the PR is worth reviewer attention given LLM churn

Review verdicts (DrahtBot): 0

Files

3 lines under test/bench/ci.

  • src/rpc/mining.cpp +3/-3
  • test/functional/rpc_generate.py +3/-0

Card

This PR migrates parameter parsing in `generatetoaddress` to `self.Arg<>()` to match `generatetodescriptor`, fixing silent integer wrapping on negative `maxtries`. The problem is minor internal inconsistency in RPC helper usage with almost no practical user impact. Reviewers agreed the cleanup is reasonable, but dergoegge raised broader concerns about reviewer attention spent on LLM-generated PRs, and the author has been silent for 170 days.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T15:51, confidence high, input hash 8a7374f6d8eb4cba