[RPC, Wallet] Move RPC dispatch table registration to wallet/ code by jonasschnelli · Pull Request #7307 · bitcoin/bitcoin

jonasschnelli · 2016-01-07T07:39:38Z

Another try...

Move all wallet RPC commands definitions and dispatch entries to wallet/wallet_rpcdispatch.h.
Allow extending the RPC dispatch table by appending commands when server is not running.

Motivation:

wallet/core code seperation
better interface for integrating another wallet

laanwj · 2016-01-07T11:30:57Z

Concept ACK. It's good that 'modules' can register their own RPC calls. The wallet is a prime example of this, but there may be more in the future.

Some nits:

walletRegisterRPCCommands should not be on CWallet. CWallet is the class that implements the wallet not its RPC interface. A better place is in rpcwallet.cpp.
walletRegisterRPCCommands implementation should not be in the header file, but in an implementation file
The contents (initialization) of const CRPCCommand vWalletRPCCommands[] belong in an implementation file, not a header file. Having a data structure initialization in a header file means it can only be included in one place, otherwise there will be duplicate definition errors.

jonasschnelli · 2016-01-07T12:20:31Z

Agreed with and fixed @laanwj points/nits.

Having a data structure initialization in a header file means it can only be included in one place, otherwise there will be duplicate definition errors.

Just curios: wouldn't this be avoided because of the headers preprocessor marcos (like #ifndef BITCOIN_WALLET_RPCWALLET_H)

paveljanik · 2016-01-07T12:31:34Z

@jonasschnelli No. Think about including this file into two separately compiled source files. Compile works. Link phase fails.

jgarzik · 2016-01-07T14:32:58Z

src/wallet/rpcwallet.cpp

Use ARRAYLEN() macro.

jgarzik · 2016-01-07T14:34:56Z

ut ACK

Note - The need to define functions in rpcwallet.h is greatly diminished. The rpcwallet.cpp externs could be removed from rpcwallet.h right now. The only reference is the in-rpcwallet call table.

jonasschnelli · 2016-01-07T19:55:52Z

@jgarzik: Your totally right. Removed the function definitions in rpcwallet.h, kept some (functions of rpcdump.cpp) in rpcwallet.cpp.

jonasschnelli · 2016-01-15T10:06:37Z

Rebased.
Would be nice to get some ACKs or NACKs because this PR tend to get merge conflicts very quick.

maflcko · 2016-01-15T16:51:55Z

Concept ACK

Allow extending the rpc dispatch table by appending commands when server is not running.

…llet/ code dd2dc40 [RPC, Wallet] Move RPC dispatch table registration to wallet/ code (Jonas Schnelli)

laanwj · 2016-01-20T14:31:57Z

utACK

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.

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 #7307 for the non-wallet.

…n to wallet/ code dd2dc40 [RPC, Wallet] Move RPC dispatch table registration to wallet/ code (Jonas Schnelli)

…was backported Allows to compile with no wallet again.

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

jonasschnelli added Wallet RPC/REST/ZMQ labels Jan 7, 2016

jonasschnelli force-pushed the 2016/01/corewallet branch from 9d56914 to e1409e8 Compare January 7, 2016 08:15

jonasschnelli force-pushed the 2016/01/corewallet branch from ee34681 to 7aafd60 Compare January 7, 2016 12:18

jonasschnelli force-pushed the 2016/01/corewallet branch from 7aafd60 to b87edf9 Compare January 7, 2016 13:16

jgarzik reviewed Jan 7, 2016
View reviewed changes

src/wallet/rpcwallet.cpp Outdated

Copy link

Contributor

jgarzik Jan 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ARRAYLEN() macro.

jonasschnelli force-pushed the 2016/01/corewallet branch from 5e6c28e to 160b237 Compare January 7, 2016 19:54

jonasschnelli force-pushed the 2016/01/corewallet branch from 160b237 to 23b583a Compare January 15, 2016 09:24

jonasschnelli mentioned this pull request Jan 15, 2016

MOVE ONLY: move rpc* to rpc/ #7348

Merged

[RPC, Wallet] Move RPC dispatch table registration to wallet/ code

dd2dc40

Allow extending the rpc dispatch table by appending commands when server is not running.

jonasschnelli force-pushed the 2016/01/corewallet branch from 23b583a to dd2dc40 Compare January 20, 2016 14:03

laanwj merged commit dd2dc40 into master Jan 20, 2016

laanwj added a commit that referenced this pull request Jan 20, 2016

Merge #7307: [RPC, Wallet] Move RPC dispatch table registration to wa…

9982710

…llet/ code dd2dc40 [RPC, Wallet] Move RPC dispatch table registration to wallet/ code (Jonas Schnelli)

jonasschnelli deleted the 2016/01/corewallet branch January 21, 2016 08:02

jonasschnelli mentioned this pull request Mar 8, 2016

Add autocomplete to bitcoin-qt's console window. #7613

Merged

laanwj mentioned this pull request Mar 29, 2016

Make it possible to get zmq notification URIs through RPC #7760

Closed

laanwj mentioned this pull request Mar 29, 2016

rpc: Register calls where they are defined #7766

Merged

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

Merge bitcoin#7307: [RPC, Wallet] Move RPC dispatch table registratio…

78ad32d

…n to wallet/ code dd2dc40 [RPC, Wallet] Move RPC dispatch table registration to wallet/ code (Jonas Schnelli)

codablock added a commit to codablock/dash that referenced this pull request Dec 11, 2017

Move privatesend to rpcwallet.cpp as well after Bitcoin bitcoin#7307 …

51ca0d5

…was backported Allows to compile with no wallet again.

dagurval mentioned this pull request Jan 17, 2018

[RPC, Wallet] Move RPC dispatch table registration to wallet/ code bitcoinxt/bitcoinxt#299

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

Liquid369 mentioned this pull request Jun 28, 2020

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

Closed

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, Wallet] Move RPC dispatch table registration to wallet/ code#7307

[RPC, Wallet] Move RPC dispatch table registration to wallet/ code#7307
laanwj merged 1 commit intomasterfrom
2016/01/corewallet

jonasschnelli commented Jan 7, 2016

Uh oh!

laanwj commented Jan 7, 2016

Uh oh!

jonasschnelli commented Jan 7, 2016

Uh oh!

paveljanik commented Jan 7, 2016

Uh oh!

jgarzik Jan 7, 2016

Uh oh!

jgarzik commented Jan 7, 2016

Uh oh!

jonasschnelli commented Jan 7, 2016

Uh oh!

jonasschnelli commented Jan 15, 2016

Uh oh!

maflcko commented Jan 15, 2016

Uh oh!

laanwj commented Jan 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

jonasschnelli commented Jan 7, 2016

Uh oh!

laanwj commented Jan 7, 2016

Uh oh!

jonasschnelli commented Jan 7, 2016

Uh oh!

paveljanik commented Jan 7, 2016

Uh oh!

jgarzik Jan 7, 2016

Choose a reason for hiding this comment

Uh oh!

jgarzik commented Jan 7, 2016

Uh oh!

jonasschnelli commented Jan 7, 2016

Uh oh!

jonasschnelli commented Jan 15, 2016

Uh oh!

maflcko commented Jan 15, 2016

Uh oh!

laanwj commented Jan 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants