Page MenuHomePhabricator

Merge #14530: Use RPCHelpMan to generate RPC doc strings

Authored by markblundeberg on Mon, Feb 10, 14:40.


Group Reviewers
Restricted Project
rABC7a59e0987888: Merge #14530: Use RPCHelpMan to generate RPC doc strings

Backport PR14530

Pull request description:

This introduces a manager for the RPC help generation and demonstrates its use of it in some RPCs.

Backport note:
This change is nice when backporting additional RPC, so we can grab the
RPCHelpMan stuff and keep it in.

Some changes were made as we don't have replaceability or smart fees, otherwise
pretty much just lint differences.

Test Plan

ninja check-all ; read carefully

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markblundeberg created this revision.Mon, Feb 10, 14:40
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Feb 10, 14:40
teamcity edited the summary of this revision. (Show Details)Mon, Feb 10, 14:40

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those Bitcoin Core PRs have been inserted into the summary for reference.

markblundeberg added inline comments.Mon, Feb 10, 14:42
41 ↗(On Diff #16218)

note: these lint advices are wrong (appeared due to wrapping) ... will fix.

a few examples of the changes:

248 ↗(On Diff #16218)

changes to:

gettxoutproof ["txid",...] ( "blockhash" )
1019 ↗(On Diff #16218)

changes to:

signrawtransactionwithkey "hexstring" ["privatekey",...] ( [{"txid":"hex","vout":n,"scriptPubKey":"hex","redeemScript":"hex","amount":amount},...] "sighashtype" )
4024 ↗(On Diff #16218)

changes to

listunspent ( minconf maxconf ["address",...] include_unsafe {"minimumAmount":amount,"maximumAmount":amount,"maximumCount":n,"minimumSumAmount":amount} )
5334 ↗(On Diff #16218)

changes to

walletcreatefundedpsbt [{"txid":"hex","vout":n,"sequence":n},...] [{"address":amount},{"data":"hex"},...] ( locktime {"changeAddress":"hex","changePosition":n,"change_type":"str","includeWatching":bool,"lockUnspents":bool,"feeRate":n,"subtractFeeFromOutputs":[int,...]} bip32derivs )

fix //!< comments to be in front

Fabien requested changes to this revision.Tue, Feb 11, 09:54
Fabien added a subscriber: Fabien.
Fabien added inline comments.
126 ↗(On Diff #16219)

You should enable clang-tidy :) Missing braces here

137 ↗(On Diff #16219)


188 ↗(On Diff #16219)


4515 ↗(On Diff #16219)


5398 ↗(On Diff #16219)

sequence is optional.

This revision now requires changes to proceed.Tue, Feb 11, 09:54
Fabien added inline comments.Tue, Feb 11, 10:08
5426 ↗(On Diff #16219)

Note: this is unsupported, and would fail if passed (out of scope for this diff).
This will require a rebase/removal after the help is fixed.

markblundeberg added inline comments.Tue, Feb 11, 11:12
4515 ↗(On Diff #16219)

heh the name is kind of a giveaway isn't it :D

5398 ↗(On Diff #16219)

good catch, it's labelled wrong in Core even still

5426 ↗(On Diff #16219)

I already ripped out various 'replaceable' things so it's fine to remove this too.

braces; drop nonexistent param

actually various wrong params -- moved to D5257

1043 ↗(On Diff #16219)

this should just be STR (it's base58 format not base16)

4047 ↗(On Diff #16219)

should just be STR

5424 ↗(On Diff #16219)

this should just be STR

Fabien accepted this revision.Tue, Feb 11, 12:52
This revision is now accepted and ready to land.Tue, Feb 11, 12:52