Details

Reviewers

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING3a84ae8b0c04: Partial Merge #14726: Use RPCHelpMan for all RPCs
rABC3a84ae8b0c04: Partial Merge #14726: Use RPCHelpMan for all RPCs

Summary

fa5e0452e875a7ca6bf6fe61fdd652d341eece40 rpc: Documentation fixups (MarcoFalke)
fa91e8eda541acdb78ca481b74605639f319c108 Use RPCHelpMan for all RPCs (MarcoFalke)

Pull request description:

The resulting documentation should not change unless the type in the oneline-summary was previously incorrect. (E.g. string vs bool)

Tree-SHA512: 4ff355b6a53178f02781e97a7aca7ee1d0d97ff348b6bf5a01caa1c96904ee33c704465fae54c2cd7445097427fd04c71ad3779bb7a7ed886055ef36c1b5a1d0

Partial backport of Core PR14726
Excluded the linter commit.

Reviewer note: the following RPCs have their help changed in another
related PR:

testmempoolaccept
createrawtransaction
createmultisig

Depends on D5567

Test Plan

ninja
ninja check
./bitcoind
./bitcoin-cli help <RPCs>

Verify help is printed without error.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

PR14726

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 9948
Build 17750: Default Diff Build & Tests
Build 17749: arc lint + arc unit

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Owners added a reviewer: Restricted Owners Package.Mar 23 2020, 19:30

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 23 2020, 19:30

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

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

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9941: Diff 17124.Mar 23 2020, 19:40

Need to re-layout some strings.

src/rpc/server.cpp
341–345 ↗	(On Diff #17124)	This function was added out-of-order in D5173.

Formatting and re-layout strings.

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

Missed an argument in unparkblock.

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9947: Diff 17130.Mar 24 2020, 00:31

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Harbormaster completed remote builds in B9948: Diff 17131.Mar 24 2020, 00:35

You may need to rebase on top of D5537 to take the new RPC into account.
Also you're missing the rpc/abc.cpp RPCs.

src/rpc/blockchain.cpp
288	Should be `STR_HEX` ?
1232	`STR_HEX`
1712	Fix layout
src/rpc/mining.cpp
290	`STR_HEX`
863	Not that smart
src/rpc/net.cpp
660	`BOOL`
src/rpc/rawtransaction.cpp
383	`STR_HEX`
1683	`STR`
1951	Remove
src/rpc/util.h
57	Move comment above (and remove the doxygen `<`, but the linter should tell you).
src/wallet/rpcdump.cpp
107	Fix layout
303	Fix layout
305	Dito
416	Fix layout
502	Ditp
504	Dito
556	Dito
558	Dito
651	Dito
910	Dito
913	Dito
916	Dito
1434	If you keep the `clang-format off`, you need to keep this one as well.
1450	Please check if this is still required (core removes it). The long line below should really be split.
1464	Remove. `pubkeys` and `keys` arrays are missing (also on core, idk why ?).
src/wallet/rpcwallet.cpp
176	Remove
870	`BOOL`
2890	Should be `AMOUNT` ?
3106	Fix layout
3179	Missing `blank`
3381	Missing `"query_options"`
3743	Remove. Missing `changePosition`
3756	Remove
4663	Not from this PR, but the `\n` is wrong here.
4801	`AMOUNT`
4807	Missing `"options"`

This revision now requires changes to proceed.Mar 24 2020, 09:17

Fixed critiques and made changes to rpc/abc.cpp. Also Made estimatefee RPC less smart.

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) started.

Harbormaster completed remote builds in B9958: Diff 17156.Mar 24 2020, 19:55

Build Bitcoin-ABC / Diffs / Diff Testing (build-diff) passed.

Fabien requested changes to this revision.Mar 25 2020, 08:43

Fabien added inline comments.

src/rpc/abc.cpp
42 ↗	(On Diff #17156)	Not sure if this is the right place to fix the RPC doc, but at least you can add the parameter to `RPCHelpMan`
src/wallet/rpcdump.cpp
1458 ↗	(On Diff #17156)	You missed that comment: Remove. pubkeys and keys arrays are missing (also on core, idk why ?).
1481 ↗	(On Diff #17156)	This is barely readable, probably worth leaving a clang-off/clang-on for this section. I'll leave that up to you.

This revision now requires changes to proceed.Mar 25 2020, 08:43

nakihito added inline comments.Mar 25 2020, 19:40

src/rpc/abc.cpp
42 ↗	(On Diff #17156)	Rebased off D5567.
src/wallet/rpcdump.cpp
1458 ↗	(On Diff #17156)	Looks like they added it back in the next patch. I'll restore it here since it doesn't make any sense for it to be missing.
1481 ↗	(On Diff #17156)	I think its fine for now. The next PR fixes the readability issue.