rpc: Register calls where they are defined by laanwj · Pull Request #7766 · bitcoin/bitcoin

laanwj · 2016-03-29T18:05:30Z

Split out registration of 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.
Removes most of the bitcoin-specific code from rpcserver.cpp and .h.

Continues #7307 for the non-wallet.

jonasschnelli · 2016-03-29T18:15:26Z

Nice!
Concept ACK.

maflcko · 2016-03-29T18:18:32Z

Concept ACK. Would it make sense to get rid of the duplicate code in void Register*()?

laanwj · 2016-03-30T07:10:57Z

Would it make sense to get rid of the duplicate code in void Register*()?

Maybe. I thought about it, but it's only a simple loop of four lines, sometimes factoring something so simple out to a function makes things harder to understand, not easier.
Another option would be to export a 'descriptor structure' with a name, pointer to the array, and size of the array. E.g. like Linux modules.
But simply having Register* function leaves it up to the module as to how to determine what to register (e.g. there may be optional features enabled on the command line).

jonasschnelli · 2016-03-30T09:30:14Z

IMO we should keep the array loop as it is (and not refactor with some hard-to-understand macro-like optimization).

laanwj · 2016-03-30T10:07:18Z

Could make it two lines, even, without losing any clarity:

   for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
        tableRPC.appendCommand(commands[vcidx].name, &commands[vcidx]);

(will update the one in rpcwallet as well)

CodeShark · 2016-03-30T10:51:53Z

Concept ACK

sipa · 2016-03-30T11:47:04Z

Concept ACK, though I wonder if it wouldn't be cleaner to pass a pointer to tableRPC to the registration commands, so the individual RPC implementations don't even need to depend on access to the global?

laanwj · 2016-03-30T11:58:11Z

pass a pointer to tableRPC to the registration commands

Awesome idea, will do that.

sipa · 2016-03-30T15:56:15Z

utACK 3be52e1

dcousens · 2016-03-31T01:00:14Z

utACK 3be52e1

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. - Removes most of the bitcoin-specific code from rpcserver.cpp and .h. Continues bitcoin#7307 for the non-wallet.

laanwj · 2016-03-31T08:48:58Z

Squashed a20fca9 0b18725 dcb0a76 3be52e1 into fb8a8cf

fb8a8cf rpc: Register calls where they are defined (Wladimir J. van der Laan)

RPC code refactor Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7307 - bitcoin/bitcoin#7348 - bitcoin/bitcoin#7766

9f4868a [move] move listunspent to wallet/rpcwallet.cpp (furszy) e85e4f1 [RPC] Move RPC dispatch table registration to rpcwallet code (furszy) c0bc348 [RPC] push down reqWallet [first step] (furszy) cdb80a6 [RPC] Remove not used threadsafe in CRPCCommand (furszy) Pull request description: Initial work over the RPC dispatch table registration update. Includes the following changes: 1) Remove the unused threadSafe member from every CPRCCommand. (bitcoin#5711 , second commit) 2) Remove reqWallet from every CPRCCommand (bitcoin#5992 , second commit partially). 3) Move most of the wallet RPC commands registration to the rpcwallet file (bitcoin#7307 without the `EnsureWalletIsAvailable` on every RPC call --> mainly because the wallet RPC commands will only be registered if the wallet is available). to do: Move the remaining commands that were not moved from server to rpcwallet. ---------- Next step would be bitcoin#7766 (If anyone want to tackle it, feel more than welcome to tackle it). ACKs for top commit: Fuzzbawls: ACK 9f4868a random-zebra: All good. ACK 9f4868a and merging... Tree-SHA512: cd83cdc2de81efb5cb84d39753a160b19d8ca2967a1b303d85c3279559b478352c5a4316942c7a654e9c667b5a8f5bfa18020c1b6c9e78f4c408028840d0fc86

903b618 Mark findserial and getserials as zerocoin commands (Fuzzbawls) 83dfeaf Refactor: Split up the "pivx" RPC command category (Fuzzbawls) 577414b Refactor: Hide testing-only RPC commands (Fuzzbawls) a876068 Refactor: Move getstakingstatus to rpcwallet.cpp (Fuzzbawls) d25ab83 Refactor: Extract zpiv RPC commands out of rpcwallet (Fuzzbawls) 94eb34b [RPC] Register calls where they are defined (Fuzzbawls) Pull request description: Revives/replaces #1720, backporting bitcoin#7766. 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. - Move zPIV RPC commands to their own file - Move `getstakingstatus` to the more appropriate `rpcwallet.cpp` - Hide commands intended for testing environments only - Split up the ambiguous "pivx" command category ACKs for top commit: furszy: looking good, travis is still running but utACK 903b618. random-zebra: utACK 903b618 and merging... Tree-SHA512: 84f3340efb6081501277883d3ca10f0c9aff8dbe68859a05bd082471b9a311e834670ab6e773a04e7ecfa665d75f3632e9f66062e65b5a5440b098fef7da2016

laanwj added the RPC/REST/ZMQ label Mar 29, 2016

laanwj force-pushed the 2016_03_separate_rpc_registration branch from e1cf847 to a20fca9 Compare March 29, 2016 18:07

laanwj added the Refactoring label Mar 29, 2016

laanwj force-pushed the 2016_03_separate_rpc_registration branch from 3be52e1 to fb8a8cf Compare March 31, 2016 08:49

laanwj merged commit fb8a8cf into bitcoin:master Mar 31, 2016

laanwj added a commit that referenced this pull request Mar 31, 2016

Merge #7766: rpc: Register calls where they are defined

16555b6

fb8a8cf rpc: Register calls where they are defined (Wladimir J. van der Laan)

laanwj mentioned this pull request Apr 6, 2016

Add cursor to iterate over utxo set, use this in gettxoutsetinfo #7756

Merged

sickpig mentioned this pull request Feb 22, 2017

Bundle together deadalnix's PR 213 to 218 BitcoinUnlimited/BitcoinUnlimited#313

Merged

codablock pushed a commit to codablock/dash that referenced this pull request Dec 19, 2017

Merge bitcoin#7766: rpc: Register calls where they are defined

6442401

fb8a8cf rpc: Register calls where they are defined (Wladimir J. van der Laan)

dagurval mentioned this pull request Feb 12, 2018

rpc: Register calls where they are defined bitcoinxt/bitcoinxt#330

Merged

str4d mentioned this pull request Jun 20, 2018

RPC code refactor zcash/zcash#3350

Merged

zkbot added a commit to zcash/zcash that referenced this pull request Jun 20, 2018

Auto merge of #3350 - str4d:rpc-reorg, r=<try>

52f4abc

RPC code refactor Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7307 - bitcoin/bitcoin#7348 - bitcoin/bitcoin#7766

zkbot added a commit to zcash/zcash that referenced this pull request Jul 18, 2018

Auto merge of #3350 - str4d:rpc-reorg, r=<try>

5978dfe

RPC code refactor Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7307 - bitcoin/bitcoin#7348 - bitcoin/bitcoin#7766

zkbot added a commit to zcash/zcash that referenced this pull request Jul 18, 2018

Auto merge of #3350 - str4d:rpc-reorg, r=bitcartel

cd0484e

RPC code refactor Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7307 - bitcoin/bitcoin#7348 - bitcoin/bitcoin#7766

furszy mentioned this pull request Jun 23, 2020

[RPC] Table registration update and wallet table decoupled. PIVX-Project/PIVX#1702

Merged

This was referenced Jun 28, 2020

[RPC] Register calls where they are defined PIVX-Project/PIVX#1714

Closed

[RPC] Register calls where they are defined PIVX-Project/PIVX#1720

Closed

Fuzzbawls mentioned this pull request Oct 17, 2020

[RPC][Refactor] Register calls where they are defined PIVX-Project/PIVX#1926

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rpc: Register calls where they are defined#7766

rpc: Register calls where they are defined#7766
laanwj merged 1 commit intobitcoin:masterfrom
laanwj:2016_03_separate_rpc_registration

laanwj commented Mar 29, 2016

Uh oh!

jonasschnelli commented Mar 29, 2016

Uh oh!

maflcko commented Mar 29, 2016

Uh oh!

laanwj commented Mar 30, 2016

Uh oh!

jonasschnelli commented Mar 30, 2016

Uh oh!

laanwj commented Mar 30, 2016

Uh oh!

CodeShark commented Mar 30, 2016

Uh oh!

sipa commented Mar 30, 2016

Uh oh!

laanwj commented Mar 30, 2016

Uh oh!

sipa commented Mar 30, 2016

Uh oh!

dcousens commented Mar 31, 2016

Uh oh!

laanwj commented Mar 31, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

laanwj commented Mar 29, 2016

Uh oh!

jonasschnelli commented Mar 29, 2016

Uh oh!

maflcko commented Mar 29, 2016

Uh oh!

laanwj commented Mar 30, 2016

Uh oh!

jonasschnelli commented Mar 30, 2016

Uh oh!

laanwj commented Mar 30, 2016

Uh oh!

CodeShark commented Mar 30, 2016

Uh oh!

sipa commented Mar 30, 2016

Uh oh!

laanwj commented Mar 30, 2016

Uh oh!

sipa commented Mar 30, 2016

Uh oh!

dcousens commented Mar 31, 2016

Uh oh!

laanwj commented Mar 31, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants