Skip to content

Commit

Permalink
Merge bitcoin-core/gui#704: Correctly limit overview transaction list
Browse files Browse the repository at this point in the history
08209c0 Correctly limit overview transaction list (John Moffett)

Pull request description:

  Fixes dashpay#703

  The way the main overview page limits the number of transactions displayed (currently 5) is not an appropriate use of Qt. Our subclassed transaction sort/filter proxy model returns a maximum of `5` in `rowCount()`. However, the model itself actually may hold significantly more. While this has _worked_, it breaks the contract of `rowCount()`.

  If `bitcoin-qt` is run with a DEBUG build of Qt, it'll result in an assert-crash in certain relatively common situations (see dashpay#703 for details). Instead of artificially limiting the `rowCount()` in the subclassed filter, we can hide/unhide the rows in the displaying `QListView` upon any changes in the sorted proxy filter.

  I loaded a wallet with 20,000 transactions and did not notice any performance differences between master and this branch.

  For reference, this is the list I'm referring to:

  <img width="934" alt="image" src="https://user-images.githubusercontent.com/116917595/214947304-3f289380-3510-487b-80e8-d19428cf2f0f.png">

ACKs for top commit:
  Sjors:
    tACK 08209c0
  hebasto:
    ACK 08209c0, tested on Ubuntu 22.04.

Tree-SHA512: c2a7b1a2a6e6ff30694830d7c722274c4c47494a81ce9ef25f8e5587c24871b02343969f4437507693d4fd40ba7a212702b159cf54b3357d8d76c02bc8245113
  • Loading branch information
hebasto committed Feb 2, 2023
2 parents 21138fe + 08209c0 commit 526f67a
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 24 deletions.
15 changes: 14 additions & 1 deletion src/qt/overviewpage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ void OverviewPage::setWalletModel(WalletModel *model)
// Set up transaction list
filter.reset(new TransactionFilterProxy());
filter->setSourceModel(model->getTransactionTableModel());
filter->setLimit(NUM_ITEMS);
filter->setDynamicSortFilter(true);
filter->setSortRole(Qt::EditRole);
filter->setShowInactive(false);
Expand All @@ -271,6 +270,10 @@ void OverviewPage::setWalletModel(WalletModel *model)
ui->listTransactions->setModel(filter.get());
ui->listTransactions->setModelColumn(TransactionTableModel::ToAddress);

connect(filter.get(), &TransactionFilterProxy::rowsInserted, this, &OverviewPage::LimitTransactionRows);
connect(filter.get(), &TransactionFilterProxy::rowsRemoved, this, &OverviewPage::LimitTransactionRows);
connect(filter.get(), &TransactionFilterProxy::rowsMoved, this, &OverviewPage::LimitTransactionRows);
LimitTransactionRows();
// Keep up to date with wallet
setBalance(model->getCachedBalance());
connect(model, &WalletModel::balanceChanged, this, &OverviewPage::setBalance);
Expand Down Expand Up @@ -299,6 +302,16 @@ void OverviewPage::changeEvent(QEvent* e)
QWidget::changeEvent(e);
}

// Only show most recent NUM_ITEMS rows
void OverviewPage::LimitTransactionRows()
{
if (filter && ui->listTransactions && ui->listTransactions->model() && filter.get() == ui->listTransactions->model()) {
for (int i = 0; i < filter->rowCount(); ++i) {
ui->listTransactions->setRowHidden(i, i >= NUM_ITEMS);
}
}
}

void OverviewPage::updateDisplayUnit()
{
if (walletModel && walletModel->getOptionsModel()) {
Expand Down
1 change: 1 addition & 0 deletions src/qt/overviewpage.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public Q_SLOTS:
std::unique_ptr<TransactionFilterProxy> filter;

private Q_SLOTS:
void LimitTransactionRows();
void updateDisplayUnit();
void handleTransactionClicked(const QModelIndex &index);
void updateAlerts(const QString &warnings);
Expand Down
17 changes: 0 additions & 17 deletions src/qt/transactionfilterproxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,25 +88,8 @@ void TransactionFilterProxy::setWatchOnlyFilter(WatchOnlyFilter filter)
invalidateFilter();
}

void TransactionFilterProxy::setLimit(int limit)
{
this->limitRows = limit;
}

void TransactionFilterProxy::setShowInactive(bool _showInactive)
{
this->showInactive = _showInactive;
invalidateFilter();
}

int TransactionFilterProxy::rowCount(const QModelIndex &parent) const
{
if(limitRows != -1)
{
return std::min(QSortFilterProxyModel::rowCount(parent), limitRows);
}
else
{
return QSortFilterProxyModel::rowCount(parent);
}
}
6 changes: 0 additions & 6 deletions src/qt/transactionfilterproxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,9 @@ class TransactionFilterProxy : public QSortFilterProxyModel
void setMinAmount(const CAmount& minimum);
void setWatchOnlyFilter(WatchOnlyFilter filter);

/** Set maximum number of rows returned, -1 if unlimited. */
void setLimit(int limit);

/** Set whether to show conflicted transactions. */
void setShowInactive(bool showInactive);

int rowCount(const QModelIndex &parent = QModelIndex()) const override;

protected:
bool filterAcceptsRow(int source_row, const QModelIndex & source_parent) const override;

Expand All @@ -60,7 +55,6 @@ class TransactionFilterProxy : public QSortFilterProxyModel
quint32 typeFilter;
WatchOnlyFilter watchOnlyFilter{WatchOnlyFilter_All};
CAmount minAmount{0};
int limitRows{-1};
bool showInactive{true};
};

Expand Down

0 comments on commit 526f67a

Please sign in to comment.