-
Notifications
You must be signed in to change notification settings - Fork 725
[RPC][Refactor] Register calls where they are defined #1926
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RPC][Refactor] Register calls where they are defined #1926
Conversation
random-zebra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebase needed (new getcachedblockhashes introduced in #1904 can be added to the hidden category), otherwise looking good. Left just a couple notes.
fb81bb3 to
03c518a
Compare
Split out methods to every module, apart from 'help' and 'stop' which are implemented in rpcserver.cpp itself. - This makes it easier to add or remove RPC commands - no longer everything that includes rpcserver.h has to be rebuilt when there's a change there. - Cleans up `rpc/server.h` by getting rid of the huge cluttered list of function definitions.
All of these commands are set to be deprecated/removed post-5.0, might as well extract them out of rpcwallet to a dedicated file to reduce bloat in rpcwallet.cpp
This is a better place to have this command, especially since it is already categorized as a "wallet" command.
These commands are primarily only used during regtest functional testing, or are otherwise not relevant to our current mainnet/testnet chains.
Split into two primary categories ("masternode" and "budget") to better
reflect the purpose. `mnsync` and `spork` are now in the "control"
category.
Keeping them in `rpc/blockchain.cpp` because they aren't strictly wallet commands, and `findserials` relies on the `validaterange` function here, no sense in duplicating that code elsewhere. Both are set to be deleted after 5.0 is locked in.
03c518a to
903b618
Compare
|
rebased |
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good, travis is still running but utACK 903b618.
Left a tiny nit that can be tackled in another work.
| void RegisterBlockchainRPCCommands(CRPCTable &tableRPC) | ||
| { | ||
| for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) | ||
| tableRPC.appendCommand(commands[vcidx].name, &commands[vcidx]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiny nit, could be written:
for (const auto& command : commands)
tableRPC.appendCommand(command.name, &command);
(And same for the others RegisterxxxRPCCommands)
random-zebra
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 903b618 and merging...
Revives/replaces #1720, backporting bitcoin#7766.
Split out methods to every module, apart from 'help' and 'stop' which
are implemented in rpcserver.cpp itself.
everything that includes rpcserver.h has to be rebuilt when there's a
change there.
rpc/server.hby getting rid of the huge cluttered list offunction definitions.
getstakingstatusto the more appropriaterpcwallet.cpp