rpc: Remove duplicate name and argNames from CRPCCommand #20012
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cdf900b to
4f2ed2f
Compare
|
Concept ACK |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
This was referenced Sep 25, 2020
Merged
4f2ed2f to
fa6aaef
Compare
This was referenced Nov 19, 2020
Merged
fa247fc to
fafd58e
Compare
|
Rebased |
This was referenced Jan 12, 2021
ryanofsky
approved these changes
Jan 21, 2021
There was a problem hiding this comment.
Code review ACK fafd58e. Nice cleanup which gets rid of some parameter repetition dealing with RPCs. It's also nice to drop some c++ parsing code. The previous asserts and new test updates seem to make this pretty foolproof and ensure there's no change in behavior.
|
Much tidier! |
fafd58e to
fab5a77
Compare
fab5a77 to
fa04f9b
Compare
|
Rebased, should be trivial to re-ACK |
|
@laanwj As the author of |
|
ACK fa04f9b |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jan 29, 2021
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Jan 29, 2021
ryanofsky
reviewed
Jan 29, 2021
| UniValue ret{UniValue::VARR}; | ||
| for (const auto& cmd : mapCommands) { | ||
| for (const auto& c : cmd.second) { | ||
| const auto help = RpcMethodFnType(c->unique_id)(); |
There was a problem hiding this comment.
In commit "rpc: Use RPCHelpMan for check-rpc-mappings linter" (fa92912)
I missed this in initial review, but this cast from unique_id to function pointer isn't really safe. Or at least it results in segfaults with #10102 (https://cirrus-ci.com/task/5469433013469184?command=ci#L2275). The unique_id field was really only supposed to be used to detect RPC aliases, not be converted to a function pointer and called. #21035 updates this to take a different approach making arg retrieval work more like help retrieval.
domob1812
added a commit
to domob1812/namecoin-core
that referenced
this pull request
Feb 1, 2021
Updated auxpow RPCs for bitcoin/bitcoin#20012.
domob1812
added a commit
to domob1812/namecoin-core
that referenced
this pull request
Feb 1, 2021
Updated name RPCs for bitcoin/bitcoin#20012. The rpc_help.py regtest now verifies that arguments with a certain name (independent of the underlying method) are either converted or not by the RPC client. This causes a mismatch between the name RPCs' "value" (which is a string and not converted) and the "setwalletflag" RPC (whose value argument is a bool). To fix this and keep compatibility with name clients, we change the name of setwalletflag's argument to "newvalue".
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 5, 2022
Summary: This is a backport of [[bitcoin/bitcoin#20012 | core#20012]] [1/3] bitcoin/bitcoin@faf8356 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10759
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 5, 2022
Summary: This is a backport of [[bitcoin/bitcoin#20012 | core#20012]] [2/3] bitcoin/bitcoin@fa92912 Test Plan: Update linters: `sudo arc liberate` Add errors in RPCHelpMan and make sure they are detected: ``` diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 8bb08769ec..770e8272e9 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -170,6 +170,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { {"buildavalancheproof", 0, "sequence"}, {"buildavalancheproof", 1, "expiration"}, {"buildavalancheproof", 3, "stakes"}, + {"buildavalancheproof", 4, "staks"}, }; class CRPCConvertTable { ``` ``` diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 8bb08769ec..f883869060 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -168,7 +168,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { // Avalanche {"addavalanchenode", 0, "nodeid"}, {"buildavalancheproof", 0, "sequence"}, - {"buildavalancheproof", 1, "expiration"}, + {"buildavalancheproof", 8, "expiration"}, {"buildavalancheproof", 3, "stakes"}, }; ``` `ninja && test/functional/test_runner.py rpc_help` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D10760
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 5, 2022
Summary: This is a backport of [[bitcoin/bitcoin#20012 | core#20012]] [3/3] bitcoin/bitcoin@fa04f9b Depends on D10760 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D10761
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the RPC argument names are specified twice to simplify consistency linting. To avoid having to specify the argnames twice when adding new arguments, remove the linter and add an equivalent test based on RPCHelpMan.