wallet encapsulation #2130

1. 
   mikegogulski commented at 8:45 pm on December 26, 2012: none

   hopefully a proper rebasing of #2075

   The goal here is to work toward a clean interface to the wallet object. For now this involves moving code out of rpc*.cpp which deals with wallet internals and making that code into methods on the CWallet object.

2. Just some comments on what I'm aiming to do here 2ec0d5f0b4
3. more TODO comments for things to be moved to rpchelpers.cpp dc90bea8ad
4. Make GetAccountAddress and SetAccount into methods of CWallet f08745e910
5. move some stuff touched by setaccount into methods of CWallet 4e32558e14
6. Some touch-ups per Diapolo's diff comments. 539e241d81
7. make GetAccountBalance a method of CWallet; clean up some spacing 6f8f35bda8
8. make GetAccountAddresses into a method of CWallet d4c4aadb7b
9. snip an unneeded comment; inline a couple of variables in

   CWallet::GetAccountAddresses()

   981ff1fc12
10. use CWallet::GetAccountAddresses() in getaddressesbyaccount() 79fd78ad2c
11. indentation fix 15cf6f3db5
12. Remove CBitcoinAddress deps in wallet.cpp; Remove an inappropriate TODO. 603970d71e
13. replace CWallet::IsMine(CBitcoinAddress&) with

    CWallet::IsMine(CTxDestination&)

    cf9949e191
14. RPC commands now no longer depend on walletdb.h/CWalletDB 943e7768f3
15. in src/wallet.cpp: in 603970d71e outdated

    1857+    }
    1858+
    1859+    return SetAddressBookName(dest, strAccount);
    1860+}
    1861+
    1862+bool CWallet::IsMine(const CBitcoinAddress& address) const
    

    ---

    ---

    

    sipa commented at 2:05 pm on December 27, 2012:

    Here’s still a CBitcoinAddress left.

    ---

    

    mikegogulski commented at 2:07 pm on December 27, 2012:

    I need to hunt that one down yet since I think it’s used outside of rpc*.cpp. I’ll post a new commit working on that soon.
16. 
    BitcoinPullTester commented at 8:01 am on January 3, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/943e7768f33a7c766d1be5db8aea64a46e37253b for binaries and test log.
17. in src/rpcwallet.cpp: in 943e7768f3 outdated

    169-            GetAccountAddress(strOldAccount, true);
    170-    }
    171-
    172-    pwalletMain->SetAddressBookName(address.Get(), strAccount);
    173-
    174+    // TODO: Use the return code for something
    

    ---

    ---

    

    sipa commented at 0:27 am on January 6, 2013:

    Why is this a TODO?

    ---

    

    mikegogulski commented at 0:38 am on January 6, 2013:

    I saw a method returning bool and the return value being ignored, so I put in that note. Looking around the codebase now I see that it’s hardly ever used, so maybe it can be ignored. Want a new commit?
18. 
    sipa commented at 0:43 am on January 6, 2013: member

    Many of your TODO’s look like todo’s for yourself rather than concrete plans for changes in the source code - leave those out. If there is a concrete plan, and it’s obvious: just add a commit that actually implements the change. If it’s something big you’d rather leave for a follow-up pull request, a TODO in the code is fine.

    Also, squash some commits together. “Indentation fix” certainly doesn’t require a separate commit.

19. Removed all my note-to-self TODOs 4debb8059a
20. 
    mikegogulski commented at 1:00 am on January 6, 2013: none

    Okay, I stripped out the todos. Should I squash everything in this branch down to one commit in order to get it pulled? (total git noob here)
21. 
    BitcoinPullTester commented at 1:17 am on January 6, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4debb8059a3d29a88b72b2bc61a4e0f6bfc7e522 for binaries and test log.
22. 
    BitcoinPullTester commented at 4:38 am on January 24, 2013: none

    Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/4debb8059a3d29a88b72b2bc61a4e0f6bfc7e522 for test log.

    This pull does not merge cleanly onto current master

23. 
    luke-jr commented at 6:36 pm on January 30, 2013: member

    Rebase needed
24. 
    jgarzik commented at 6:27 pm on June 19, 2013: contributor

    Poke @mikegogulski

    The general sentiment towards these changes seems positive. Let’s rebase and get this moving, or close.

25. 
    luke-jr commented at 4:09 am on July 17, 2013: member

    I’ve rebased this as my mikegogulski_walletencap3 branch. It does, however, create a dependency from wallet.cpp -> main.h; I don’t see any obvious way to fix this, considering that IsFinalTx is not tied to any class.
26. 
    jgarzik commented at 3:25 am on August 25, 2013: contributor

    Closing - non-responsive. Feel free to rebase and reopen.
27. jgarzik closed this on Aug 25, 2013

    ---

28. owlhooter referenced this in commit 97b9b4fedc on Oct 11, 2018
29. DrahtBot locked this on Sep 8, 2021

Contributors
mikegogulski sipa BitcoinPullTester luke-jr jgarzik