Refactor ZapWalletTxes to avoid layer violations #9143

1. 
   jonasschnelli commented at 9:59 am on November 12, 2016: contributor

   Currently, CWallet::ZapWalletTx() calls CWalletDB::ZapWalletTx() which then uses CWallet*.

   This simple refactoring results in a clean layering between CWallet and CWalletDB. CWalletDB should have know knowledge of CWallet.

   Together with something like #9101, this slowly allows to use a different wallet database layer.

2. jonasschnelli added the label Refactoring on Nov 12, 2016
3. jonasschnelli added the label Wallet on Nov 12, 2016
4. 
   ryanofsky commented at 5:17 pm on November 18, 2016: member

   utACK 448c7d8f6b5ab5320891b6906b3a5b36596c71f1

   Seems straightfoward. Just moves some cs_wallet lock calls and a mapWallet erase call from CWalletDB to CWallet.

5. in src/wallet/walletdb.cpp: in 448c7d8f6b outdated

   645@@ -646,20 +646,17 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
   646     return result;
   647 }
   648 
   649-DBErrors CWalletDB::FindWalletTx(CWallet* pwallet, vector<uint256>& vTxHash, vector<CWalletTx>& vWtx)
   650+DBErrors CWalletDB::FindWalletTx(vector<uint256>& vTxHash, vector<CWalletTx>& vWtx)
   651 {
   652-    pwallet->vchDefaultKey = CPubKey();
   

   ---

   ---

   

   MarcoFalke commented at 11:10 am on November 25, 2016:

   What was the purpose of this line? (added in 518f3bda)

   ---

   

   sipa commented at 1:24 am on December 1, 2016:

   That probably should have been moved to ZapWalletTx. It wiped the default key, to make sure a new key would be generated next time. The default key is effectively unused currently, we should remove it entirely.

   ---

   

   jonasschnelli commented at 8:20 am on December 1, 2016:

   I think I removed this because it’s no longer in use. But agree with @sipa. We should remove it entirely.

   ---

   

   sipa commented at 8:35 am on December 1, 2016:

   I tried writing a patch to remove it, but I believe we basically can’t without breaking backward compatibility for wallet files.
6. MarcoFalke approved
7. 
   MarcoFalke commented at 11:26 am on November 25, 2016: member

   utACK 448c7d8 mod question
8. laanwj renamed this:
   Refactor ZapWalletTxes to avoid layer vialotions
   Refactor ZapWalletTxes to avoid layer violations
   on Nov 29, 2016
9. 
   sipa commented at 1:32 am on December 1, 2016: member

   utACK
10. Refactor ZapWalletTxes to avoid layer vialotions 0165a56f20
11. jonasschnelli force-pushed on Jan 24, 2017
12. 
    jonasschnelli commented at 1:26 pm on January 24, 2017: contributor

    Rebased and restored vchDefaultKey behaviour (see comment #9143 (review)).
13. in src/wallet/walletdb.cpp: in 0165a56f20

    658         int nMinVersion = 0;
    659         if (Read((string)"minversion", nMinVersion))
    660         {
    661             if (nMinVersion > CLIENT_VERSION)
    662                 return DB_TOO_NEW;
    663-            pwallet->LoadMinVersion(nMinVersion);
    

    ---

    ---

    

    ryanofsky commented at 10:00 pm on January 24, 2017:

    Just noting: This call is dropped, but presumably this isn’t a problem because the same min version should have already been set when the wallet was first loaded.
14. ryanofsky approved
15. 
    ryanofsky commented at 10:04 pm on January 24, 2017: member

    re-utACK 0165a56f20bf0666c9a0850d5634bf5547cee29b.

    (which is identical to 448c7d8f6b5ab5320891b6906b3a5b36596c71f1, except for the restored vchDefaultKey resets in ZapSelectTx and ZapWalletTx)

16. 
    jtimon commented at 4:10 am on February 3, 2017: contributor

    fast review ACK 0165a56
17. 
    laanwj commented at 10:32 am on March 2, 2017: member

    utACK 0165a56
18. laanwj merged this on Mar 2, 2017
19. laanwj closed this on Mar 2, 2017

    ---

20. laanwj referenced this in commit 65d90f585a on Mar 2, 2017
21. PastaPastaPasta referenced this in commit 09f0fc1cf9 on Dec 29, 2018
22. PastaPastaPasta referenced this in commit f4f38655c2 on Dec 30, 2018
23. PastaPastaPasta referenced this in commit 8d29096f24 on Dec 30, 2018
24. PastaPastaPasta referenced this in commit da6c9e7658 on Dec 30, 2018
25. PastaPastaPasta referenced this in commit 0be52065a5 on Dec 30, 2018
26. PastaPastaPasta referenced this in commit 5d094e50b5 on Dec 31, 2018
27. PastaPastaPasta referenced this in commit bfd412ca8c on Dec 31, 2018
28. PastaPastaPasta referenced this in commit 52fd733b55 on Dec 31, 2018
29. PastaPastaPasta referenced this in commit ef8e016338 on Dec 31, 2018
30. PastaPastaPasta referenced this in commit e07da1f475 on Dec 31, 2018
31. PastaPastaPasta referenced this in commit 552aa878c3 on Dec 31, 2018
32. PastaPastaPasta referenced this in commit 62f0d33cd5 on Dec 31, 2018
33. PastaPastaPasta referenced this in commit da7aca4efd on Jan 2, 2019
34. PastaPastaPasta referenced this in commit 7bc950681a on Jan 2, 2019
35. PastaPastaPasta referenced this in commit e3df94b245 on Jan 3, 2019
36. PastaPastaPasta referenced this in commit cc1a04a942 on Jan 3, 2019
37. PastaPastaPasta referenced this in commit 5ff3a47791 on Jan 5, 2019
38. PastaPastaPasta referenced this in commit d7847450d3 on Jan 5, 2019
39. PastaPastaPasta referenced this in commit 5f0324a7de on Jan 7, 2019
40. PastaPastaPasta referenced this in commit cbe422e46e on Jan 7, 2019
41. PastaPastaPasta referenced this in commit 3b77588922 on Jan 7, 2019
42. PastaPastaPasta referenced this in commit e8cb812983 on Jan 7, 2019
43. PastaPastaPasta referenced this in commit bba55e262f on Jan 23, 2019
44. PastaPastaPasta referenced this in commit 68f6b43d15 on Jan 23, 2019
45. PastaPastaPasta referenced this in commit 069b692de0 on Jan 25, 2019
46. PastaPastaPasta referenced this in commit 5283f8da83 on Jan 25, 2019
47. MarcoFalke locked this on Sep 8, 2021

Contributors
jonasschnelli ryanofsky MarcoFalke sipa jtimon laanwj

Labels
Refactoring Wallet