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

Lint

Lint Not Applicable

Unit

Tests Not Applicable

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