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

ReviewabilityReviewsAgreementSize
#34950 defenwycke rpc: migrate generatetoaddress params to self.Arg for consistencyWaiting on author0Mild3 + 3 tests
  • Align generatetoaddress parameter parsing with generatetodescriptor
  • Explicitly reject negative maxtries inputs instead of silently overflowing
  • Waiting on the author to answer an open objection regarding release notes
  • No review verdicts recorded
  • Support for consistency cleanup with generatetodescriptor (maflcko, GerardoTaboada)
  • Unaddressed objection: behavioral change on negative maxtries needs a release note (GerardoTaboada)
  • Resolved compilation and linker failure in bitcoin-qt target (maflcko)
  • 3 lines added or modified outside tests
  • 3 lines added or modified in tests
  • 3 lines removed in total
  • 2 files, 1 commits
RPC / REST / ZMQ#53 of 55
P4 · cleanup
  • P4 because it is a minor cleanup aligning generatetoaddress argument parsing with modern RPC helpers
  • Fixes negative maxtries wrapping to UINT64_MAX with minimal external impact
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

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.

Reviewability

Paused · Waiting on author

Open objection from GerardoTaboada with no author reply for 188 days; review would repeat what has been asked.

Agreement

Mild

objection that needs an answer has none yet (GerardoTaboada)

Thread notes: Confirmed the resolved Qt build failure prompted by maflcko's query. Kept GerardoTaboada's note on the maxtries behavior change as an open non-blocking claim regarding release notes, while dropping dergoegge's statement from claims because concern over reviewer attention and LLM churn is not a concrete code harm under project definitions.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
maflcko (member)objection, correctnesscompilation and linker failure in bitcoin-qt targetfixed · fix pushed · v1, not yet re-checkedansweredyes2026-03-30: 'Did you even try to compile it locally?'
Status from: 2026-03-30: defenwycke: 'I have switched to self.Arg<std::string_view> to match generatetodescriptor.'
GerardoTaboada (none)objection, interfaceuser-visible change rejecting negative maxtries could surprise integrators without a release noteopen · v1, not yet re-checkedansweredno2026-04-25: 'it might still be worth a release-notes line under RPC so integrators don't get surprised'

Support:

  • maflcko (member) (2026-03-30): consistency with generatetodescriptor makes sense as a cleanup
  • GerardoTaboada (none) (2026-04-25): aligning with generatetodescriptor is the obvious cleanup

Participants:

  • maflcko (member, objection, 4 statements 2026-03-30 to 2026-03-30): flagged build failure by asking if author compiled locally (resolved); supported consistency with generatetodescriptor
  • GerardoTaboada (none, objection, 1 statements 2026-04-25 to 2026-04-25): supported aligning with generatetodescriptor but flagged that negative maxtries behavior change may warrant a release note
  • dergoegge (member, neutral, 1 statements 2026-04-27 to 2026-04-27): urged author to reconsider LLM-generated changes given scarce reviewer attention, without asking to close

Files

3 lines under test/bench/ci.

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

Card

The short text the ranking pass compares across PRs.

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 ledger, generated 2026-09-18T03:28, confidence high, input hash ee41409c791f5a8e
thread read through 2026-04-27 (8 statements), head 2586ed683e
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: RPC/REST/ZMQ

Categories

RPC / REST / ZMQ

P4 · cleanup

Migrates argument parsing in a test generation RPC to the modern self.Arg helper for consistency. While it prevents negative maxtries wrapping to UINT64_MAX, reviewers characterize the change as straightforward cleanup of legacy parsing code with minimal external impact.

Membership: Carries the RPC/REST/ZMQ label and updates argument handling via self.Arg for generatetoaddress in src/rpc/mining.cpp.

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