gui: Drop RecentRequestsTableModel dependency to WalletModel #18618

1. 
   promag commented at 9:30 am on April 13, 2020: member
2. fanquake added the label GUI on Apr 13, 2020
3. 
   promag commented at 9:32 am on April 13, 2020: member

   Currently based on #18608.
4. promag force-pushed on Apr 13, 2020
5. in src/qt/recentrequeststablemodel.cpp:19 in 8aa9b53e9a outdated

   15@@ -17,18 +16,18 @@
   16 
   17 #include <utility>
   18 
   19-RecentRequestsTableModel::RecentRequestsTableModel(WalletModel *parent) :
   20-    QAbstractTableModel(parent), walletModel(parent)
   21+RecentRequestsTableModel::RecentRequestsTableModel(interfaces::Wallet& wallet, OptionsModel* options_model, QObject* parent) :
   

   ---

   ---

   

   ryanofsky commented at 3:50 pm on April 13, 2020:

   In commit “gui: Drop RecentRequestsTableModel dependency to WalletModel” (8aa9b53e9a47810d3ebe90743123b130cc4493c3)

   Would be nice to make options_model and m_options_model references instead of pointers if they are not allowed to be null

   ---

   

   promag commented at 11:52 pm on April 19, 2020:

   Sure, I think its unrelated to this PR though. For instance, the check in L126 could be removed. Lots of other members could be references including cases like WalletView::walletModel where there’s a setter.
6. ryanofsky approved
7. 
   ryanofsky commented at 3:51 pm on April 13, 2020: member

   Code review ACK 8aa9b53e9a47810d3ebe90743123b130cc4493c3
8. 
   DrahtBot commented at 4:38 pm on April 13, 2020: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #19292 (wallet: Refactor BerkeleyBatch Read, Write, Erase, and Exists functions into non-template functions by achow101)
   • #19290 (wallet: move BDB specific classes to bdb.{cpp/h} by achow101)
   • #19289 (wallet: GetWalletTx and IsMine require cs_wallet lock by promag)
   • #19245 ([WIP DONOTMERGE] Replace boost::filesystem with std::filesystem (in c++17) by kiminuo)
   • #19137 (wallettool: Add dump and createfromdump commands by achow101)
   • #19102 (wallet: Introduce and use DummyDatabase instead of dummy BerkeleyDatabase by achow101)
   • #19077 (wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets by achow101)
   • #18971 (wallet: Refactor the classes in wallet/db.{cpp/h} by achow101)
   • #18354 (Use shared pointers only in validation interface by bvbfan)
   • #17966 (qt, refactor: Optimize signal-slot connections logic by hebasto)
   • #17877 (qt, refactor: Make enums in BitcoinUnits class scoped by hebasto)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

9. DrahtBot added the label Needs rebase on Apr 27, 2020
10. promag force-pushed on May 4, 2020
11. DrahtBot removed the label Needs rebase on May 4, 2020
12. DrahtBot added the label Needs rebase on May 4, 2020
13. ryanofsky approved
14. 
    ryanofsky commented at 8:59 pm on May 13, 2020: member

    Code review ACK aa2b55a8845de5f4be6a0f5a9aee60447838112d. No changes since last review, just rebased base PR
15. promag force-pushed on May 17, 2020
16. DrahtBot removed the label Needs rebase on May 17, 2020
17. 
    ryanofsky commented at 8:26 pm on May 19, 2020: member

    Code review ACK c5ebb4db6894bec60e00f9247f6a6806cf30d06a. Only change since last review is rebasing to fix bilingual_str conflict
18. DrahtBot added the label Needs rebase on May 27, 2020
19. refactor: Remove CAddressBookData::destdata

    This is cleanup that doesn't change external behavior.
    
    - Removes awkward `StringMap` intermediate representation
    - Deals with receive request "rr" keys in walletdb.cpp instead of all over qt, wallet, and interfaces code
    - Deals with destination "used" keys in walletdb.cpp instead of all over wallet code
    - Adds test coverage
    - Reduces code (+85/-138 lines)
    - Reduces memory usage
    
    This PR doesn't change externally observable behavior. Internally, only change in behavior is that EraseDestData deletes directly from database because the `StringMap` is gone. This is more direct and efficient because it uses a single btree lookup and scan instead of multiple lookups
    
    Motivation for this cleanup is making changes like #18550, #18192, #13756 easier to reason about and less likely to result in unintended behavior and bugs

    3a58f2ae17
20. gui: Drop RecentRequestsTableModel dependency to WalletModel 47a85bce35
21. promag force-pushed on Jun 1, 2020
22. DrahtBot removed the label Needs rebase on Jun 1, 2020
23. DrahtBot added the label Needs rebase on Jun 17, 2020
24. 
    DrahtBot commented at 10:01 am on June 17, 2020: member

    🐙 This pull request conflicts with the target branch and needs rebase.

25. promag marked this as a draft on Jun 17, 2020
26. 
    fanquake commented at 1:08 pm on July 9, 2020: member

    This is based on #18608 (which itself has 1, I think, refuted Concept NACK & needs a rebase), is purely a qt circular dependency refactor and also needs a rebase. I’m going to suggest reopening this in https://github.com/bitcoin-core/gui.
27. fanquake closed this on Jul 9, 2020

    ---

28. 
    ryanofsky commented at 8:12 pm on July 10, 2020: member

    #18608 is up to date (and I think was only conflicted for a few hours)
29. DrahtBot locked this on Feb 15, 2022

Contributors
promag ryanofsky DrahtBot fanquake

Labels
GUI Needs rebase