gui: Drop RecentRequestsTableModel dependency to WalletModel #18618
Conversation
|
Currently based on #18608. |
5730554 to
8aa9b53
Compare
| @@ -17,18 +16,18 @@ | |||
|
|
|||
| #include <utility> | |||
|
|
|||
| RecentRequestsTableModel::RecentRequestsTableModel(WalletModel *parent) : | |||
| QAbstractTableModel(parent), walletModel(parent) | |||
| RecentRequestsTableModel::RecentRequestsTableModel(interfaces::Wallet& wallet, OptionsModel* options_model, QObject* parent) : | |||
There was a problem hiding this comment.
In commit "gui: Drop RecentRequestsTableModel dependency to WalletModel" (8aa9b53)
Would be nice to make options_model and m_options_model references instead of pointers if they are not allowed to be null
There was a problem hiding this comment.
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.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
8aa9b53 to
aa2b55a
Compare
aa2b55a to
c5ebb4d
Compare
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 bitcoin#18550, bitcoin#18192, bitcoin#13756 easier to reason about and less likely to result in unintended behavior and bugs
c5ebb4d to
47a85bc
Compare
|
🐙 This pull request conflicts with the target branch and needs rebase. |
|
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. |
|
#18608 is up to date (and I think was only conflicted for a few hours) |
No description provided.