RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest #8775

luke-jr · 2016-09-21T09:56:50Z

Part of the refactorings needed for basic multiwallet (#8694)

jonasschnelli · 2016-09-21T13:07:21Z

I don't like the coupling and the #ifdef ENABLE_WALLET in rpc/server.cpp|.h.
I'd recommend to keep the CRPCRequestInfo wallet-free.

I think we should pack the request path (URI) into the CRPCRequestInfo and or informations about the authentication (in case we want to distinct wallets based on authentication).
Then I think there should be a method in wallet.cpp (or in rpcwallet.cpp) that maps a CWallet * pointer from a given URI, Auth-Info or the complete CRPCRequestInfo instance.

Instead of the CWallet *& pwallet = reqinfo.wallet; there could be then something like CWallet *pwallet = CWallets::getWalletFromRequest(reqinfo)

jonasschnelli · 2016-09-21T13:09:42Z

General ConceptACK on a CRPCRequestInfo for the RPC table commands.
Maybe it could also include the UniValue params and fHelp?

paveljanik · 2016-09-21T16:17:01Z

src/rpc/misc.cpp

Yes, pwallet is an alias to reqinfo.wallet which is of type CWallet*.

luke-jr · 2016-09-21T20:40:26Z

I think we should pack the request path (URI) into the CRPCRequestInfo and or informations about the authentication (in case we want to distinct wallets based on authentication).
Then I think there should be a method in wallet.cpp (or in rpcwallet.cpp) that maps a CWallet * pointer from a given URI, Auth-Info or the complete CRPCRequestInfo instance.

That sounds nice, but greatly complicates the implementation. :(

maflcko · 2016-10-19T15:16:41Z

I think this can be closed after #8788?

jonasschnelli · 2016-10-19T15:28:54Z

Closing in favor of merged #8788

Wrapped in CRPCRequestInfo to avoid gratuitous #ifdef ENABLE_WALLET everywhere Github-Pull: bitcoin#8775 Rebased-From: 80f4ab7b5f404d20bed41ec70182e6fc0990205b

Github-Pull: bitcoin#8775 Rebased-From: 9279b515815e58df5bd796b0570e388ff42fb84b

…nique per CWallet Github-Pull: bitcoin#8775 Rebased-From: 041f4e06a664b7e31ba77bbfbb2f7bc8d04174ca

luke-jr · 2016-10-25T09:14:49Z

Rebased and refactored based on @jonasschnelli 's idea.

laanwj · 2016-10-25T11:18:26Z

Makes sense, utACK ab5ce98

laanwj · 2016-10-25T11:19:35Z

src/rpc/misc.cpp


 using namespace std;

+CWallet *GetWalletForJSONRPCRequest(const JSONRPCRequest&);


Not sure how this can pass the build w/ --disable-wallet, but this should be bracketed with #ifdef ENABLE_WALLET

luke-jr · 2016-11-12T01:44:44Z

Rebased and addressed nit

Github-Pull: bitcoin#8775 Similar-To: 7de55733c5690350c48ec94660b5be56fbb5eb39

instagibbs · 2017-01-04T18:25:55Z

utACK 7de55733c5690350c48ec94660b5be56fbb5eb39

…tForJSONRPCRequest d678771 Wallet: Sanitise -wallet parameter (Luke Dashjr) 9756be3 Wallet/RPC: Use filename rather than CWallet pointer, for lockwallet RPCRunLater job name (Luke Dashjr) 86be48a More tightly couple EnsureWalletIsAvailable with GetWalletForJSONRPCRequest where appropriate (Luke Dashjr) a435632 Move wallet RPC declarations to rpcwallet.h (Luke Dashjr) ad15734 RPC: Pass on JSONRPCRequest metadata (URI/user/etc) for "help" method (Luke Dashjr) bf8a04a Reformat touched lines with C++11 (Luke Dashjr) 2e518e3 Move nWalletUnlockTime to CWallet::nRelockTime, and name timed task unique per CWallet (Luke Dashjr) d77ad6d RPC: Do all wallet access through new GetWalletForJSONRPCRequest (Luke Dashjr) eca550f RPC/Wallet: Pass CWallet as pointer to helper functions (Luke Dashjr) Tree-SHA512: bfd592da841693390e16f83b451503eb5cedb71208089aa32b3fc45e973555584a3ed7696dd239f6409324464d565dacf0f3d0e36e8e13ae6a7843848465f960

…port bitcoin#9908

adneerav · 2019-02-05T07:39:27Z

Hello @luke-jr ,

Is there any way where we can increase the number of wallets to be in loaded state at same time.As rightnow its like if I load wallet more than 100 or 150 daemon is getting crashed/stopped. I am trying to load/unload wallet dynamically using RPC.

What i tried is incraesing the set_lk_max_lockers to 400000 and also added
set_lg_regionmax 1048576 But still same things I am facing.

Reflink for above setup : https://docs.oracle.com/cd/E17276_01/html/api_reference/C/set_lk_max_lockers_parameter.html
https://bitcoin.stackexchange.com/a/70737

So is there any way or configuration or any changes which can increase and allow more number of wallets to be loaded ?

4734a84 [Cleanup][Tests] Fix chainparams-change in librust tests (random-zebra) ff4cee0 [Trivial] Add wallet filename to backup errors/warning (random-zebra) dc596f3 [Doc] Add multiwallet section to release notes (random-zebra) 36796f2 [Cleanup] Fix formatting in wallet and walletdb (random-zebra) b6dbbf3 wallet: Forbid -salvagewallet, -zapwallettxes, and -upgradewallet with multiple wallets (random-zebra) 60f9b4b wallet: Base backup filenames on original wallet filename (Luke Dashjr) e6efa6b wallet: Include actual backup filename in recovery warning message (Luke Dashjr) 647fbc9 Wallet: Move multiwallet sanity checks to CWallet::Verify, and do other checks on all wallets (Luke Dashjr) b27dcfe Wallet: Support loading multiple wallets if -wallet used more than once (Luke Dashjr) d10acd5 [Tests] move pwalletMain to wallet test fixture + use smart pointer (random-zebra) d6cf608 [Refactor] Remove CWalletDBWrapper::GetUpdateCounter() (random-zebra) 4bbad5c [Wallet] Replace pwalletMain with a vector of wallet pointers (random-zebra) 100d67c Wallet: Sanitise -wallet parameter (Luke Dashjr) 3bfa7d8 Wallet/RPC: Use filename rather than CWallet pointer, for lockwallet RPCRunLater job name (Luke Dashjr) ca6a62d [MOVE-ONLY] Move wallet RPC declarations to rpcwallet.h (random-zebra) 687c2fd RPC: Pass on JSONRPCRequest metadata (URI/user/etc) for "help" method (Luke Dashjr) cc965fe Move nWalletUnlockTime to CWallet::nRelockTime, and name timed task unique per CWallet (Luke Dashjr) 22f8507 [Trivial] Rename pwalletMain --> pwallet for local variables in RPC (random-zebra) 325baaa RPC: Do all wallet access through new GetWalletForJSONRPCRequest (random-zebra) 0e21e09 [Cleanup] Remove un-used printAddresses() function in rpcwallet (random-zebra) a8dd236 RPC/Wallet: Pass CWallet as pointer to helper functions (random-zebra) fb64bbc [Doc] Remove ThreadFlushWalletDB from developer notes (random-zebra) dc2e022 [Refactor] fix WalletTestingSetup fixture (random-zebra) f9d7fe1 refactor: move bdb (bitdb) interaction from init.cpp to wallet.cpp (random-zebra) 9cfb711 CWalletDB: Store the update counter per wallet (Luke Dashjr) 8edb74f Bug: increment counter when writing minversion (random-zebra) 8aa93b9 Bugfix: wallet: Increment "update counter" only after actually making the applicable db changes to avoid potential races (Luke Dashjr) Pull request description: Quite a bit of refactoring to bring us a little closer to upstream in the wallet/walletdb area, introducing basic support for multiple wallets. The PIVX client can now be started with more than one `-wallet` argument (either as startup flags, or as multiple lines in pivx.conf). The wallets will be all loaded and kept separated, with individual balances, keys and received transactions. Even though only the first wallet will be used in the GUI/RPC for the moment (selectable wallets will be added later), all other loaded wallets will remain synchronized to the node's current tip and update their internal data. Bulk of changes coming from: - bitcoin#8775 RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest - bitcoin#8694 Basic multiwallet support - bitcoin#11713 Fix for mismatched extern definition in wallet tests - bitcoin#11781 tests: move pwalletMain to wallet test fixture ACKs for top commit: furszy: Code ACK 4734a84 after rebase and inclusion of 4734a84. Tree-SHA512: 483fc34310c86070ffde08487605b25da957e55fbc8ef1b9cc1f682c6af0789619bb40839e915618d603097a1bdcf6b2110c7fabf017f8351b3a426976ca77a9

maflcko added RPC/REST/ZMQ Wallet labels Sep 21, 2016

paveljanik reviewed Sep 21, 2016

View reviewed changes

jonasschnelli mentioned this pull request Sep 22, 2016

[RPC] Give RPC commands more information about the RPC request #8788

Merged

luke-jr force-pushed the multiwallet_prefactor_rpc branch 2 times, most recently from 7697a0e to df2c0c5 Compare October 18, 2016 12:20

jonasschnelli closed this Oct 19, 2016

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Oct 20, 2016

RPC/Wallet: Access wallet via CRPCRequestInfo only

cb370e3

Github-Pull: bitcoin#8775 Rebased-From: 9279b515815e58df5bd796b0570e388ff42fb84b

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Oct 20, 2016

Move nWalletUnlockTime to CWallet::nRelockTime, and name timed task u…

06e4c90

…nique per CWallet Github-Pull: bitcoin#8775 Rebased-From: 041f4e06a664b7e31ba77bbfbb2f7bc8d04174ca

laanwj reopened this Oct 25, 2016

luke-jr force-pushed the multiwallet_prefactor_rpc branch 2 times, most recently from 06bd5d0 to ab5ce98 Compare October 25, 2016 08:50

laanwj reviewed Oct 25, 2016

View reviewed changes

luke-jr force-pushed the multiwallet_prefactor_rpc branch 2 times, most recently from 5cce71a to d6bc295 Compare November 12, 2016 01:44

luke-jr changed the title ~~RPC refactoring: Never access wallet directly, only via new CRPCRequestInfo~~ RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest Nov 12, 2016

luke-jr force-pushed the multiwallet_prefactor_rpc branch from d6bc295 to 7de5573 Compare December 29, 2016 13:20

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Dec 31, 2016

RPC: Pass on CRPCRequestInfo metadata (wallet) for "help" method

41383de

Github-Pull: bitcoin#8775 Similar-To: 7de55733c5690350c48ec94660b5be56fbb5eb39

luke-jr mentioned this pull request Jan 3, 2017

Basic multiwallet support #8694

Merged

luke-jr force-pushed the multiwallet_prefactor_rpc branch from 7de5573 to 7c65786 Compare January 5, 2017 22:52

PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 25, 2019

fix bitcoin#8775 backport

c094d4b

PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 25, 2019

change bitcoin#8775 to keep dash codebase improvement, but still back…

c70aa60

…port bitcoin#9908

PastaPastaPasta mentioned this pull request Jan 25, 2019

Backports 0.15 pr4 dashpay/dash#2646

Merged

random-zebra mentioned this pull request Apr 23, 2021

[Wallet] Basic multiwallet support PIVX-Project/PIVX#2337

Merged

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021


		using namespace std;

		CWallet *GetWalletForJSONRPCRequest(const JSONRPCRequest&);

RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest #8775

RPC refactoring: Access wallet using new GetWalletForJSONRPCRequest #8775

Uh oh!

Conversation

luke-jr commented Sep 21, 2016

Uh oh!

jonasschnelli commented Sep 21, 2016

Uh oh!

jonasschnelli commented Sep 21, 2016

Uh oh!

paveljanik Sep 21, 2016

Choose a reason for hiding this comment

Uh oh!

luke-jr Sep 21, 2016

Choose a reason for hiding this comment

Uh oh!

luke-jr commented Sep 21, 2016

Uh oh!

maflcko commented Oct 19, 2016

Uh oh!

jonasschnelli commented Oct 19, 2016

Uh oh!

luke-jr commented Oct 25, 2016

Uh oh!

laanwj commented Oct 25, 2016

Uh oh!

laanwj Oct 25, 2016

Choose a reason for hiding this comment

Uh oh!

luke-jr commented Nov 12, 2016

Uh oh!

instagibbs commented Jan 4, 2017

Uh oh!

adneerav commented Feb 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants