Use shared pointer for wallet instances #11402

1. 
   promag commented at 11:05 pm on September 25, 2017: member

   This is a small step towards better multi wallet management. Starts by removing the global vpwallets and adding an interface to add/remove/retrieve wallets.

   The way it is now it is possible to have, for instance, listunspent RPC 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:

   • kind of WalletManager class;
   • keep a weak pointer so when the app terminates it is possible to gracefully unload the wallet;
   • make the new interface thread safe.

   Please consider this RFC.

   This is related to #10615, #11383 and #10740.

2. fanquake added the label Wallet on Sep 25, 2017
3. [wallet] Use shared pointer for wallet instances 3191f84214
4. promag force-pushed on Sep 25, 2017
5. in src/wallet/wallet.cpp:40 in 3191f84214 outdated

   36@@ -37,7 +37,36 @@
   37 #include <boost/algorithm/string/replace.hpp>
   38 #include <boost/thread.hpp>
   39 
   40-std::vector<CWalletRef> vpwallets;
   41+static std::vector<std::shared_ptr<CWallet>> vpwallets;
   

   ---

   ---

   

   luke-jr commented at 11:20 pm on September 25, 2017:

   Should typedef std::shared_ptr<CWallet> CWalletRef
6. in src/wallet/wallet.cpp:42 in 3191f84214 outdated

   36@@ -37,7 +37,36 @@
   37 #include <boost/algorithm/string/replace.hpp>
   38 #include <boost/thread.hpp>
   39 
   40-std::vector<CWalletRef> vpwallets;
   41+static std::vector<std::shared_ptr<CWallet>> vpwallets;
   42+
   43+bool AddWallet(std::shared_ptr<CWallet> pwallet)
   

   ---

   ---

   

   luke-jr commented at 11:21 pm on September 25, 2017:

   These all need locking.

   ---

   

   promag commented at 11:25 pm on September 25, 2017:

   At the moment no because wallets are added at startup. This is mentioned in the PR description:

   make the new interface thread safe.

7. luke-jr changes_requested
8. Squash me [wallet] Prevent adding the same wallet 7ff54b680e
9. [wallet] Make multi-wallet management thread safe 1a5bd9c762
10. Squash me [wallet] Use CWalletRef type 85b2b4f91d
11. promag force-pushed on Sep 26, 2017
12. fanquake added this to the "In progress" column in a project

    ---

13. 
    TheBlueMatt commented at 5:18 pm on November 9, 2017: member

    Needs rebase. Maybe @jnewbery wants to give this a concept review in how it interacts (or doesnt) with #10740?
14. 
    promag commented at 3:25 am on December 14, 2017: member

    @jnewbery ping.
15. 
    jnewbery commented at 2:09 pm on March 27, 2018: member

    What’s the status of this PR now that #12587 is open?
16. 
    promag commented at 2:19 pm on March 27, 2018: member

    I think both are valid. This PR changes the ownership of the wallets. For example this PR allows:

    listunspent RPC and in parallel unload the wallet (once #10740 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet.

    I’m happy to update this once there is more positive feedback.

17. promag renamed this:
    [wallet] Use shared pointer for wallet instances
    Use shared pointer for wallet instances
    on Apr 7, 2018
18. 
    promag commented at 1:17 pm on April 18, 2018: member

    This PR includes some changes that are now in #13017. Please review that first.
19. promag closed this on Apr 23, 2018

    ---

20. promag deleted the branch on Apr 23, 2018
21. fanquake removed this from the "In progress" column in a project

    ---

22. laanwj referenced this in commit 6378eef18f on May 24, 2018
23. xdustinface referenced this in commit 61c36b5b60 on Mar 31, 2021
24. xdustinface referenced this in commit 72349ea80c on Mar 31, 2021
25. xdustinface referenced this in commit 5f44ba0c03 on Mar 31, 2021
26. xdustinface referenced this in commit c376b64a4f on Apr 1, 2021
27. PastaPastaPasta referenced this in commit a3cc51118e on Apr 1, 2021
28. MarcoFalke locked this on Sep 8, 2021

Contributors
promag luke-jr TheBlueMatt jnewbery

Labels
Wallet