#36384 rpc: Use self.Arg<_>(name) helper over manual and duplicate parsing
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36384 rpc: Use self.Arg<_>(name) helper over manual and duplicate parsing | Ready | 0 | Positive | 73 + 2 tests | |||
|
|
|
|
| |||
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
Replaces manual parameter extraction, null checking, and fallback default handling across node and wallet RPC endpoints with the `self.Arg<T>(name)` helper. It updates `GetParamIndex` to match the primary argument name using `GetFirstName()` when parameter aliases exist, adds template instantiations for `double` and `int64_t`, and converts `verifychain` metadata to use `RPCArg::Default`.
Problem: RPC endpoints frequently duplicate parameter logic by checking `request.params` index validity and hardcoding fallback defaults that are already declared in the method definition metadata. This boilerplate creates redundant code and risks drift between documented defaults and actual execution.
Evidence of importance:
- PR description: 'The `Arg()` helper has many benefits: It does not require manual `isNull` checks, It does not require hard-coding the fallback/default value a second time, It does not require manually specifying the getter for the type. Specifying the type is enough.'
Scope: Touches 14 RPC and wallet RPC source files mechanically replacing `request.params` index access with named `self.Arg` lookups.
Risk of a mistake: medium (rpc_api, wallet). A typo in an argument name or a subtle difference in type conversion or default resolution could alter RPC behavior or cause crashes on unexpected inputs across multiple wallet and node RPC endpoints.
Changed since the previous assessment: Includes a minor documentation typo fix in a comment in walletcreatefundedpsbt.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- reviewer: review of the changes 2026-10-01
- reviewer: code review 2026-10-01
Agreement
Positive
support without stated reasons, no open objection (fanquake)
Thread notes: Initial record: fanquake gave Concept ACK; author noted a schema change in verifychain RPC. / Initial record. fanquake gave a Concept ACK with no further comments; maflcko clarified the RPC schema change in verifychain.
Objections, suggestions, and questions: none recorded.
Support:
- fanquake (member) (Concept ACK, 2026-10-01): (no reason given) [not substantive]
Participants:
- fanquake (member, support, 1 statements 2026-10-01 to 2026-10-01): Concept ACK
Files
4 lines under test/bench/ci.
- src/rpc/blockchain.cpp +18/-34
- src/wallet/rpc/transactions.cpp +11/-18
- src/rpc/mempool.cpp +6/-16
- src/wallet/rpc/spend.cpp +9/-12
- src/rpc/rawtransaction.cpp +6/-12
- src/wallet/rpc/coins.cpp +4/-14
- src/rpc/net.cpp +4/-7
- src/wallet/rpc/wallet.cpp +5/-5
- src/rpc/util.cpp +4/-4
- src/rpc/mining.cpp +3/-3
- src/rpc/fees.cpp +1/-4
- src/test/rpc_tests.cpp +2/-2
- src/wallet/rpc/addresses.cpp +1/-1
- src/wallet/rpc/backup.cpp +1/-1
Card
The short text the ranking pass compares across PRs.
Replaces manual RPC parameter null-checking and fallback logic across node and wallet RPC commands with the self.Arg<T>(name) helper. This removes boilerplate and prevents re-declaring parameter default values that are already defined in the RPC metadata. It also updates GetParamIndex to resolve parameter aliases via their primary name and adds missing type instantiations. The change is an internal refactoring with no stated change in user-facing behavior.
Data
thread read through 2026-10-01 (2 statements), head faf493feb4
labels: RPC/REST/ZMQ
Categories
RPC / REST / ZMQ
P4 · cleanup
Cleans up RPC parameter parsing boilerplate and reduces duplicate default definitions. The PR avoids hard-coding default fallbacks and manual null checks by leveraging `self.Arg<T>`, but it is an internal refactoring without user-facing interface changes or bug fixes.
Membership: Labeled RPC/REST/ZMQ and updates RPC parameter helper logic across core RPC methods in src/rpc/.
Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0