Paths

Table of Contentst

Differential D5239

Merge #14530: Use RPCHelpMan to generate RPC doc strings
ClosedPublic
Actions

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

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rSTAGING7a59e0987888: Merge #14530: Use RPCHelpMan to generate RPC doc strings
rABC7a59e0987888: Merge #14530: Use RPCHelpMan to generate RPC doc strings

Summary

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

Repository

rABC Bitcoin ABC

Branch

pr14530

Lint

Lint Passed

Severity	Location	Code	Message
Auto-Fix	src/rpc/util.h:41	DOXYGEN1	Misplaced inline doxygen comment.
Auto-Fix	src/rpc/util.h:42	DOXYGEN1	Misplaced inline doxygen comment.
Auto-Fix	src/rpc/util.h:44	DOXYGEN1	Misplaced inline doxygen comment.

Unit

No Test Coverage

Build Status

Buildable 9387
Build 16705: Default Diff Build & Tests
Build 16704: arc lint + arc unit

Event Timeline

markblundeberg created this revision.Feb 10 2020, 14:40

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 10 2020, 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.Feb 10 2020, 14:42

src/rpc/util.h
41	note: these lint advices are wrong (appeared due to wrapping) ... will fix.

a few examples of the changes:

src/rpc/rawtransaction.cpp
248	changes to: gettxoutproof ["txid",...] ( "blockhash" )
1019	changes to: signrawtransactionwithkey "hexstring" ["privatekey",...] ( [{"txid":"hex","vout":n,"scriptPubKey":"hex","redeemScript":"hex","amount":amount},...] "sighashtype" )
src/wallet/rpcwallet.cpp
4024	changes to listunspent ( minconf maxconf ["address",...] include_unsafe {"minimumAmount":amount,"maximumAmount":amount,"maximumCount":n,"minimumSumAmount":amount} )
5334	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 )

Build Bitcoin-ABC / Diffs / Diff Testing started.

fix //!< comments to be in front

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9388: Diff 16219.Feb 10 2020, 15:14

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9387: Diff 16218.Feb 10 2020, 15:14

Fabien mentioned this in D5240: [LINTER] Fix the doxygen linter when inline comments are multilined.Feb 10 2020, 15:50

Fabien requested changes to this revision.Feb 11 2020, 09:54

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/rpc/util.cpp
126 ↗	(On Diff #16219)	You should enable clang-tidy :) Missing braces here
137 ↗	(On Diff #16219)	Dito.
188 ↗	(On Diff #16219)	Dito
src/wallet/rpcwallet.cpp
4515 ↗	(On Diff #16219)	`STR_HEX` ?
5398 ↗	(On Diff #16219)	`sequence` is optional.

This revision now requires changes to proceed.Feb 11 2020, 09:54

Fabien added inline comments.Feb 11 2020, 10:08

src/wallet/rpcwallet.cpp
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.

Fabien mentioned this in D5255: Fix nits in RPC help messages.Feb 11 2020, 10:40

markblundeberg added inline comments.Feb 11 2020, 11:12

src/wallet/rpcwallet.cpp
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.

Fabien mentioned this in rABC7f6d7071b816: Fix nits in RPC help messages.Feb 11 2020, 11:16

braces; drop nonexistent param

Build Bitcoin-ABC / Diffs / Diff Testing started.

markblundeberg added a child revision: D5257: RPCHelpMan fixups.Feb 11 2020, 11:46

actually various wrong params -- moved to D5257

src/rpc/rawtransaction.cpp
1043 ↗	(On Diff #16219)	this should just be STR (it's base58 format not base16)
src/wallet/rpcwallet.cpp
4047 ↗	(On Diff #16219)	should just be STR
5424 ↗	(On Diff #16219)	this should just be STR