Use shared pointer for wallet instances #11402
Closed
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
f0e9d5b to
3191f84
Compare
luke-jr
suggested changes
Sep 25, 2017
src/wallet/wallet.cpp
Outdated
| @@ -37,7 +37,36 @@ | |||
| #include <boost/algorithm/string/replace.hpp> | |||
| #include <boost/thread.hpp> | |||
|
|
|||
| std::vector<CWalletRef> vpwallets; | |||
| static std::vector<std::shared_ptr<CWallet>> vpwallets; | |||
There was a problem hiding this comment.
Should typedef std::shared_ptr<CWallet> CWalletRef
src/wallet/wallet.cpp
Outdated
| std::vector<CWalletRef> vpwallets; | ||
| static std::vector<std::shared_ptr<CWallet>> vpwallets; | ||
|
|
||
| bool AddWallet(std::shared_ptr<CWallet> pwallet) |
There was a problem hiding this comment.
At the moment no because wallets are added at startup. This is mentioned in the PR description:
make the new interface thread safe.
57a0659 to
85b2b4f
Compare
|
@jnewbery ping. |
|
What's the status of this PR now that #12587 is open? |
|
I think both are valid. This PR changes the ownership of the wallets. For example this PR allows:
I'm happy to update this once there is more positive feedback. |
|
This PR includes some changes that are now in #13017. Please review that first. |
This was referenced Apr 20, 2018
laanwj
added a commit
that referenced
this pull request
May 24, 2018
80b4910 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once #13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces #11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce
xdustinface
pushed a commit
to xdustinface/dash
that referenced
this pull request
Mar 31, 2021
80b4910 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once bitcoin#13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces bitcoin#11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce
xdustinface
pushed a commit
to xdustinface/dash
that referenced
this pull request
Mar 31, 2021
80b4910 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once bitcoin#13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces bitcoin#11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce
xdustinface
pushed a commit
to xdustinface/dash
that referenced
this pull request
Mar 31, 2021
80b4910 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once bitcoin#13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces bitcoin#11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce
xdustinface
pushed a commit
to xdustinface/dash
that referenced
this pull request
Apr 1, 2021
80b4910 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once bitcoin#13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces bitcoin#11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce
PastaPastaPasta
pushed a commit
to dashpay/dash
that referenced
this pull request
Apr 1, 2021
) 80b4910 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once bitcoin#13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces bitcoin#11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
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.
This is a small step towards better multi wallet management. Starts by removing the global
vpwalletsand adding an interface to add/remove/retrieve wallets.The way it is now it is possible to have, for instance,
listunspentRPC and in parallel unload the wallet (once #10740 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet.It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers.
I would like to include either here or in follow ups:
WalletManagerclass;Please consider this RFC.
This is related to #10615, #11383 and #10740.