#36489 wallet: rpc: Add snake_case aliases for wallet funding options
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #36489 wallet: rpc: Add snake_case aliases for wallet funding options | Ready | 1 | Mild | 43 + 11 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
Adds snake_case aliases for options in the `fundrawtransaction` and `walletcreatefundedpsbt` RPCs, updating the RPC schema, client conversions, and documentation. Introduces explicit validation that throws an RPC error if both camelCase and snake_case variants of an option are provided simultaneously, and fixes a bug where `subtract_fee_from_outputs` was silently ignored by the server.
Problem: Wallet funding RPCs handle option names inconsistently across interfaces. The RPC server accepted some snake_case options that `bitcoin-cli` rejected, `subtract_fee_from_outputs` was ignored entirely in favor of only the camelCase variant, and supplying both variants caused one to be silently ignored without warning.
Evidence of importance:
- Commit message fac8ef10f4 and PR description note that 'subtract_fee_from_outputs' was silently ignored by the server, reported by stutxo in PR #35317.
- Diff in src/wallet/rpc/spend.cpp confirms `options["subtractFeeFromOutputs"]` was previously accessed directly without checking the snake_case option, whereas `change_address` and `change_position` had ad-hoc checks.
Risk of a mistake: medium (rpc_api, wallet). Mistakes in parsing or validating wallet funding RPC parameters could cause transaction creation to fail or select the wrong change outputs or fee subtraction rules.
Changed since the previous assessment: Diff and tests are now present; the change implements a helper `GetOptionWithAlias` to reject conflicting aliases and exercises snake_case options in functional tests.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- reviewer: fametrano to follow up on author reply regarding schema rejection of deprecated aliases 2026-10-26
- author: response to fametrano's suggestion on leftover camelCase names in user text 2026-10-25
Agreement
Mild
objection that needs a change was answered, no reply from the objector (fametrano: The schema now rejects camelCase options that the server still accepts, breaking clients that validate against the schem)
Thread notes: The author responded to fametrano's objection regarding the schema rejecting camelCase aliases, explaining it is intentional pre-existing behavior that should be handled in a separate PR if changed. fametrano's suggestion regarding remaining camelCase strings in help/error text remains unaddressed.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| kevkevinpal (contributor) | question, interface | Why is there no snake_case alias for includeWatching? | accepted · withdrawn by reviewer after noticing includeWatching is deprecated | answered | no | 2026-10-10: 'is there any reason why we don't have a snake_case for `includeWatching`?' Status from: 2026-10-10: kevkevinpal: 'Never mind I see `includeWatching` is marked deprecated' |
| kevkevinpal (contributor) | suggestion, maintenance | Mention in the release notes that providing both camelCase and snake_case options will now throw an error | answered · author declined, saying release notes should not be bloated with expected behavior | answered | yes | 2026-10-10: 'Should we add in the release note that sending both camelCase and snake_case will now throw an error?' Status from: 2026-10-12: maflcko: 'I think that should be the obvious and expected behavior. No need to bloat the release notes with stuff that no user sho' |
| fametrano (none) | objection, interface | The schema now rejects camelCase options that the server still accepts, breaking clients that validate against the schema | answered · author replied that schema rejection of deprecated aliases is intentional and existing behavior | changed | yes | 2026-10-25: 'The schema now rejects `changeAddress`, `changePosition`, `lockUnspents` and `subtractFeeFromOutputs`, which the server still accepts.' Status from: 2026-10-26: maflcko: 'Yes, I presume the schema rejecting the deprecated alias is intentional. This is the behavior before and after this pull request.' |
| fametrano (none) | suggestion, interface | Update remaining camelCase names in help text and error messages | open | answered | no | 2026-10-25: 'Two camelCase names remain in text a user reads. The `change_type` help still says "Only valid if changeAddress is not specified", and an out-of-range position still fails with "changePosition out of ' |
Support:
- psychemist (none) (Concept ACK, 2026-10-09): (no reason given) [not substantive]
- kevkevinpal (contributor) (Concept ACK, 2026-10-10): (no reason given) [not substantive]
- Conradgabe (none) (Tested ACK, 2026-10-12) [tested]: reproduced all 3 issues on master and verified the fixes on regtest
Participants:
- psychemist (none, support, 1 statements 2026-10-09 to 2026-10-09): Concept ACK
- kevkevinpal (contributor, support, 6 statements 2026-10-10 to 2026-10-10): Concept ACK; asked about includeWatching and suggested a release note update
- Conradgabe (none, support, 1 statements 2026-10-12 to 2026-10-12): Tested ACK verifying fixes on regtest after reproducing issues on master
- fametrano (none, objection, 1 statements 2026-10-25 to 2026-10-25): objected that the schema rejects camelCase options that the server still accepts, and pointed out leftover camelCase strings in help and error text
Files
15 lines under test/bench/ci.
- src/wallet/rpc/spend.cpp +30/-14
- doc/release-notes-36489.md +7/-0
- src/rpc/client.cpp +6/-0
- test/functional/wallet_fundrawtransaction.py +6/-0
- test/functional/wallet_keypool.py +3/-2
- test/functional/wallet_basic.py +1/-1
- test/functional/wallet_musig.py +1/-1
Card
The short text the ranking pass compares across PRs.
This PR adds missing snake_case aliases (`change_address`, `change_position`, `lock_unspents`, and `subtract_fee_from_outputs`) to the RPC schema, `bitcoin-cli` client table, and documentation for `fundrawtransaction` and `walletcreatefundedpsbt`. It fixes a bug where `subtract_fee_from_outputs` was previously ignored by the server, and errors out when callers pass both camelCase and snake_case versions of an option instead of silently ignoring one.
Data
thread read through 2026-10-26 (12 statements), head faeb073ca6
labels: Wallet
Categories
Wallet
P3 · bug fix
Fixes a bug where subtract_fee_from_outputs was silently ignored by the server and adds snake_case alias consistency across wallet funding commands. The improvements prevent unexpected fee deduction behavior and mutual option collisions, but are reasonably deferrable.
Membership: Labeled Wallet and modifies wallet funding RPC option parsing in src/wallet/rpc/spend.cpp.
Factors: security/stability 0, bug 1, performance 0, user value 1, leverage 0