[qt] Move some WalletModel functions into CWallet #10295

pull ryanofsky wants to merge 4 commits into bitcoin:master from ryanofsky:pr/ipc-move changing 6 files +235 −54
  1. ryanofsky commented at 9:51 pm on April 28, 2017: member

    Motivation for moving these is to make supporting IPC simpler (#10102), so these lookups can be one-shot IPC requests, instead of back-and-forth interactions over the IPC channel.

    Also these functions are potentially useful outside of the bitcoin GUI (e.g. for RPCs).

  2. jonasschnelli commented at 10:28 pm on April 28, 2017: contributor

    Yes. They belong to CWallet rather then WalletModel. The coincontrol’s ListCoins function can potentially be useful for non GUI features. My only nit is if we should rename ListCoins to something more meaningful.

    Concept ACK.

  3. jonasschnelli added the label Refactoring on Apr 28, 2017
  4. jonasschnelli added the label Wallet on Apr 28, 2017
  5. in src/wallet/wallet.cpp:1991 in 861be88fa2 outdated 
    1989@@ -1983,6 +1990,19 @@ CAmount CWallet::GetLegacyBalance(const isminefilter& filter, int minDepth, cons
1990     return balance;
1991 }
1992 
1993+CAmount CWallet::GetAvailableBalance(const CCoinControl* coinControl) const
1994+{
1995+    CAmount balance = 0;
    TheBlueMatt commented at 9:22 pm on May 3, 2017:
    nit: might be nice to LOCK here.
    jonasschnelli commented at 6:18 pm on May 4, 2017:
    Not sure if a lock is required at this point. Do you want to protect const CCoinControl* coinControl? AvailableCoins() does protect via cs_main/cs_wallet.
    TheBlueMatt commented at 8:26 pm on May 4, 2017:
    We have pointers in the COutput to entires in mapWallet, which can (theoretically) be delted out from under us thanks to the zap rpc.
    ryanofsky commented at 8:44 pm on May 4, 2017:
    Added in 14c1174c9aadb446c9a9e16626611f476610f4ba
  6. in src/wallet/wallet.cpp:2150 in 861be88fa2 outdated 
    2105+            if (depth >= 0 && output.n < it->second.tx->vout.size() &&
2106+                IsMine(it->second.tx->vout[output.n]) == ISMINE_SPENDABLE) {
2107+                CTxDestination address;
2108+                if (ExtractDestination(FindNonChangeParentOutput(*it->second.tx, output.n).scriptPubKey, address)) {
2109+                    result[address].emplace_back(
2110+                        &it->second, output.n, depth, true /* spendable */, true /* solvable */, false /* safe */);
    TheBlueMatt commented at 5:02 pm on May 4, 2017:
    Why did you change to false for safe here…looks like the old code did true? Does that have any effect?
    ryanofsky commented at 8:44 pm on May 4, 2017:

    Why did you change to false for safe here…looks like the old code did true? Does that have any effect?

    It has no effect (I’m not even exposing it to Qt any more after 7a7795fdd5fe86b9918b8055666b8365e4342eb6 in #10244) and false seemed like a more correct value.

  7. in src/qt/walletmodel.cpp:591 in 861be88fa2 outdated 
    588@@ -596,37 +589,11 @@ bool WalletModel::isSpent(const COutPoint& outpoint) const
589 void WalletModel::listCoins(std::map<QString, std::vector<COutput> >& mapCoins) const
590 {
591     std::vector<COutput> vCoins;
    TheBlueMatt commented at 5:03 pm on May 4, 2017:
    We dont need vCoins anymore either, no?
    ryanofsky commented at 8:44 pm on May 4, 2017:
    Removed in f8bfecf09037f3c49594ddc1ccadb9f98a354a84.
  8. TheBlueMatt commented at 5:04 pm on May 4, 2017: member
    Looks good, seems reasonable, though I hope we can delete half the code here (see #10337).
  9. ryanofsky force-pushed on May 4, 2017
  10. ryanofsky commented at 9:26 pm on May 4, 2017: member
    Squashed 14c1174c9aadb446c9a9e16626611f476610f4ba -> b6a9cc48fb950d1f0daa6c3db1df2bbc2b591ab0 (pr/ipc-move.2 -> pr/ipc-move.3)
  11. in src/qt/test/wallettests.cpp:84 in b6a9cc48fb outdated 
    78@@ -80,8 +79,9 @@ QModelIndex FindTx(const QAbstractItemModel& model, const uint256& txid)
79 //     src/qt/test/test_bitcoin-qt -platform xcb      # Linux
80 //     src/qt/test/test_bitcoin-qt -platform windows  # Windows
81 //     src/qt/test/test_bitcoin-qt -platform cocoa    # macOS
82-void WalletTests::walletTests()
83+void TestSendCoins()
84 {
85+    return;
    TheBlueMatt commented at 3:16 pm on May 5, 2017:
    Were you planning on re-enabling this?
    ryanofsky commented at 3:47 pm on May 5, 2017:
    Wow, good catch. Fixed in 01f847bf2b0ea395f8533e4d52c9422807af0566. This was just an accident, I didn’t mean to disable it at all.
  12. ryanofsky force-pushed on May 5, 2017
  13. ryanofsky force-pushed on May 5, 2017
  14. ryanofsky force-pushed on May 5, 2017
  15. ryanofsky force-pushed on May 5, 2017
  16. ryanofsky commented at 3:48 pm on May 5, 2017: member
    Squashed 01f847bf2b0ea395f8533e4d52c9422807af0566 -> 38d3864eccd6abbbcf22619c8109a1b6e015f901 (pr/ipc-move.4 -> pr/ipc-move.5)
  17. in src/qt/walletmodel.cpp:625 in 38d3864ecc outdated 
    622@@ -657,10 +623,7 @@ void WalletModel::listLockedCoins(std::vector<COutPoint>& vOutpts)
623 void WalletModel::loadReceiveRequests(std::vector<std::string>& vReceiveRequests)
624 {
625     LOCK(wallet->cs_wallet);
    TheBlueMatt commented at 5:18 pm on May 5, 2017:
    nit: maybe push this lock down into GetDestValues?
    ryanofsky commented at 2:57 pm on May 9, 2017:
    Done in e60a3110c12a47fdcf7fb488b17ae145058c020e
  18. in src/wallet/wallet.cpp:2125 in 38d3864ecc outdated 
    2081@@ -2060,6 +2082,59 @@ void CWallet::AvailableCoins(std::vector<COutput>& vCoins, bool fOnlySafe, const
2082     }
2083 }
2084 
2085+std::map<CTxDestination, std::vector<COutput>> CWallet::ListCoins() const
2086+{
2087+    std::map<CTxDestination, std::vector<COutput>> result;
    TheBlueMatt commented at 5:19 pm on May 5, 2017:
    nit: might be nice to AssertLockHeld here (because you cant call this without cs_wallet due to the COutput returns).
    ryanofsky commented at 3:02 pm on May 9, 2017:
    Not currently possible to just add the assert because Qt doesn’t acquire the lock, but I added a todo in fe8c036cb87c62bee653fe581544c714ce8808d2
  19. TheBlueMatt commented at 5:19 pm on May 5, 2017: member
    utACK 38d3864eccd6abbbcf22619c8109a1b6e015f901
  20. ryanofsky force-pushed on May 9, 2017
  21. ryanofsky commented at 7:21 pm on May 9, 2017: member
    Squashed fe8c036cb87c62bee653fe581544c714ce8808d2 -> a226c97c336c44b4b8320328debea69e576f0fa8 (pr/ipc-move.6 -> pr/ipc-move.7)
  22. [test] Add tests for some walletmodel functions 
    Add unit tests for some walletmodel functions that will be refactored & moved
in the next commit.
    ef8ca179ef
  23. [qt] Move some WalletModel functions into CWallet 
    Motivation for moving these is to make supporting IPC simpler (#10102), so
these lookups can be one-shot IPC requests, instead of back-and-forth
interactions over the IPC channel.

Also these functions are potentially useful outside of the bitcoin GUI (e.g.
for RPCs).
    d944bd7a27
  24. [test] Move some tests from qt -> wallet 
    After previous refactoring, the tests make more sense here.
    429aa9eb51
  25. Add missing LOCK2 in CWallet::GetAvailableBalance 108f04f2d9
  26. ryanofsky force-pushed on May 17, 2017
  27. ryanofsky force-pushed on May 17, 2017
  28. ryanofsky commented at 3:34 pm on May 17, 2017: member
    Rebased a226c97c336c44b4b8320328debea69e576f0fa8 -> 108f04f2d973adac5313c7e4e17a59766a3cc1b6 (pr/ipc-move.7 -> pr/ipc-move.8) because of conflict with #8952.
  29. laanwj added this to the "Blockers" column in a project
  30. laanwj commented at 5:29 pm on May 23, 2017: member
    utACK 108f04f
  31. laanwj merged this on May 23, 2017
  32. laanwj closed this on May 23, 2017
  33. laanwj referenced this in commit ce8176d038 on May 23, 2017
  34. sipa removed this from the "Blockers" column in a project
  35. MarcoFalke referenced this in commit b012bbe358 on Aug 31, 2018
  36. PastaPastaPasta referenced this in commit b38543c673 on Jun 20, 2019
  37. PastaPastaPasta referenced this in commit dc7380243f on Jul 6, 2019
  38. PastaPastaPasta referenced this in commit e966163ecb on Jul 8, 2019
  39. PastaPastaPasta referenced this in commit 9424192419 on Jul 9, 2019
  40. PastaPastaPasta referenced this in commit 785fa07015 on Jul 11, 2019
  41. barrystyle referenced this in commit 6f326a9d83 on Jan 22, 2020
  42. random-zebra referenced this in commit 2d50b6e3b9 on Jun 9, 2021
  43. PastaPastaPasta referenced this in commit 3ed39fb7e8 on Jun 27, 2021
  44. PastaPastaPasta referenced this in commit 3639112c81 on Jun 28, 2021
  45. PastaPastaPasta referenced this in commit 1bbf70330f on Jun 29, 2021
  46. PastaPastaPasta referenced this in commit c079031e99 on Jun 29, 2021
  47. PastaPastaPasta referenced this in commit 017708af12 on Jun 29, 2021
  48. PastaPastaPasta referenced this in commit f411978924 on Jun 29, 2021
  49. PastaPastaPasta referenced this in commit 4c80a5f989 on Jun 29, 2021
  50. PastaPastaPasta referenced this in commit 7c120a7124 on Jul 1, 2021
  51. Munkybooty referenced this in commit 90c89bbfe8 on Jul 1, 2021
  52. MarcoFalke locked this on Sep 8, 2021


ryanofsky jonasschnelli TheBlueMatt laanwj

Labels
Refactoring Wallet

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 09:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me