Make WalletInitInterface and DummyWalletInit private, fix nullptr deref #12836

1. 
   promag commented at 8:00 am on March 30, 2018: member

   Implementations of WalletInitInterface don’t have to be public, so make them private. This makes the interface instantiation static. Also reduces ENABLE_WALLET usage and removes the unnecessary src/wallet/init.h header.
2. fanquake added the label Wallet on Mar 30, 2018
3. 
   jnewbery commented at 1:41 pm on March 30, 2018: member

   This change fixes the nullpointer dereference here: #10762#commitcomment-28345993
4. 
   jamesob commented at 1:44 pm on March 30, 2018: member

   utACK https://github.com/bitcoin/bitcoin/pull/12836/commits/208fad6b5e6c5f19a917e0dd06a75927281f31eb
5. in src/init.cpp:91 in 208fad6b5e outdated

   87+    void Flush() override {}
   88+    void Stop() override {}
   89+    void Close() override {}
   90+};
   91+
   92+static DummyWalletInit g_dymmy_wallet_init;
   

   ---

   ---

   

   jnewbery commented at 6:43 pm on March 30, 2018:

   typo dymmy.

   Why does this need to be static?

   ---

   

   promag commented at 9:02 pm on March 30, 2018:

   Dummy typo 😄It doesn’t have to be static. IMO these can have internal linkage. WDYT?

   ---

   

   jnewbery commented at 1:39 pm on April 2, 2018:

   IMO these can have internal linkage. WDYT?

   Yes, seems reasonable.

6. MarcoFalke referenced this in commit d454e39f2a on Apr 1, 2018
7. promag force-pushed on Apr 1, 2018
8. 
   jnewbery commented at 1:42 pm on April 2, 2018: member

   Tested ACK 57c00059bb51be64a670edebc00416fc2050d5f1 once this has been rebased on master to fix the linter error.
9. promag force-pushed on Apr 2, 2018
10. 
    promag commented at 2:19 pm on April 2, 2018: member

    Rebased.
11. 
    jamesob commented at 3:19 pm on April 2, 2018: member

    re-utACK https://github.com/bitcoin/bitcoin/pull/12836/commits/465a8eed157a637391f60d4681a5d7c6a885cdbd
12. 
    jnewbery commented at 3:46 pm on April 2, 2018: member

    Looks good. ReACK 465a8eed157a637391f60d4681a5d7c6a885cdbd
13. 
    instagibbs commented at 7:03 pm on April 5, 2018: member

    ^ Needs rebase
14. laanwj assigned laanwj on Apr 5, 2018
15. MarcoFalke renamed this:
    Make WalletInitInterface and DummyWalletInit private
    Make WalletInitInterface and DummyWalletInit private, fix nullptr deref
    on Apr 5, 2018
16. wallet: Make WalletInitInterface and DummyWalletInit private 39bc2faa2e
17. wallet: Refactor to WalletInitInterface* const g_wallet_init_interface d894894aab
18. in src/init.cpp:77 in 465a8eed15 outdated

    71@@ -72,7 +72,25 @@ static const bool DEFAULT_STOPAFTERBLOCKIMPORT = false;
    72 
    73 std::unique_ptr<CConnman> g_connman;
    74 std::unique_ptr<PeerLogicValidation> peerLogic;
    75-std::unique_ptr<WalletInitInterface> g_wallet_init_interface;
    76+
    77+#if !(ENABLE_WALLET)
    78+class DummyWalletInit : public WalletInitInterface {
    

    ---

    ---

    

    PierreRochard commented at 7:23 pm on April 5, 2018:

    What’s the reason for moving this class from src/walletinitinterface.h?

    ---

    

    jamesob commented at 7:57 pm on April 5, 2018:

    I think the idea is to narrow the scope of this class to where it’s needed and reduce unnecessary header bloat.

    ---

    

    promag commented at 8:06 pm on April 5, 2018:

    Right.
19. promag force-pushed on Apr 5, 2018
20. 
    promag commented at 8:10 pm on April 5, 2018: member

    Rebased.
21. 
    jamesob commented at 8:37 pm on April 5, 2018: member

    Tested ACK https://github.com/bitcoin/bitcoin/pull/12836/commits/d894894aab78dbd5e32267313237648933248377

    For both --enable-wallet and --disable-wallet:

    • Ran bitcoin-qt -h
    • Verified preexisting wallet data loaded
22. 
    PierreRochard commented at 1:20 pm on April 6, 2018: contributor

    Tested ACK d894894aab78dbd5e32267313237648933248377

    Did the same as @jamesob and manually added in logging to DummyWalletInit’s Open() to see that it is being used with --disable-wallet and not being used with --enable-wallet

23. laanwj added the label Bug on Apr 7, 2018
24. laanwj merged this on Apr 7, 2018
25. laanwj closed this on Apr 7, 2018

    ---

26. laanwj referenced this in commit 9abdb7c67e on Apr 7, 2018
27. PastaPastaPasta referenced this in commit 0ae2a12ba0 on Mar 14, 2020
28. PastaPastaPasta referenced this in commit 4879de76ef on Mar 19, 2020
29. PastaPastaPasta referenced this in commit 599df2b325 on Mar 21, 2020
30. PastaPastaPasta referenced this in commit 5fc1686919 on Mar 24, 2020
31. PastaPastaPasta referenced this in commit 2acf2a058e on May 20, 2020
32. UdjinM6 referenced this in commit e78db55314 on May 20, 2020
33. UdjinM6 referenced this in commit 0662f170f0 on May 20, 2020
34. ckti referenced this in commit 6efccd83cb on Mar 28, 2021
35. ckti referenced this in commit f10aa62724 on Mar 28, 2021
36. gades referenced this in commit 368a862171 on Jun 30, 2021
37. DrahtBot locked this on Sep 8, 2021

Contributors
promag jnewbery jamesob instagibbs PierreRochard

Labels
Bug Wallet