Subaddress: cleanup

src/ReceiveWidget.cpp

@@ -6,6 +6,7 @@
 
 #include <QMenu>
 
+#include "libwalletqt/rows/SubaddressRow.h"
 #include "dialog/PaymentRequestDialog.h"
 #include "dialog/QrCodeDialog.h"
 #include "utils/config.h"
@@ -124,10 +125,12 @@ QString ReceiveWidget::getAddress(quint32 minorIndex) {
 }
 
 void ReceiveWidget::copyAddress() {
-    SubaddressRow* row = this->currentEntry();
-    if (!row) return;
+    QModelIndex index = getCurrentIndex();
+    if (!index.isValid()) {
+        return;
+    }
 
-    QString address = this->getAddress(row->getRow());
+    QString address = this->getAddress(index.row());
     Utils::copyToClipboard(address);
 }
 
@@ -143,8 +146,11 @@ void ReceiveWidget::editLabel() {
 }
 
 void ReceiveWidget::showContextMenu(const QPoint &point) {
-    SubaddressRow* row = this->currentEntry();
-    if (!row) return;
+    QModelIndex index = getCurrentIndex();
+    if (!index.isValid()) {
+        return;
+    }
+    auto& row = m_model->entryFromIndex(index);
 
     auto *menu = new QMenu(ui->addresses);
 
@@ -152,31 +158,35 @@ void ReceiveWidget::showContextMenu(const QPoint &point) {
     menu->addAction("Copy label", this, &ReceiveWidget::copyLabel);
     menu->addAction("Edit label", this, &ReceiveWidget::editLabel);
 
-    if (row->isUsed()) {
+    if (row.used) {
         menu->addAction(m_showTransactionsAction);
     }
 
     QAction *actionPin = menu->addAction("Pin address", [this](bool toggled){
-        SubaddressRow* row = this->currentEntry();
-        if (!row) return;
-        
-        QString address = row->getAddress();
-        m_wallet->subaddress()->setPinned(address, toggled);
+        QModelIndex index = getCurrentIndex();
+        if (!index.isValid()) {
+            return;
+        }
+        auto& row = m_model->entryFromIndex(index);
+
+        m_wallet->subaddress()->setPinned(row.address, toggled);
         m_proxyModel->invalidate();
     });
     actionPin->setCheckable(true);
-    actionPin->setChecked(row->isPinned());
+    actionPin->setChecked(row.pinned);
 
     QAction *actionHide = menu->addAction("Hide address", [this](bool toggled){
-        SubaddressRow* row = this->currentEntry();
-        if (!row) return;
-        
-        QString address = row->getAddress();
-        m_wallet->subaddress()->setHidden(address, toggled);
+        QModelIndex index = getCurrentIndex();
+        if (!index.isValid()) {
+            return;
+        }
+        auto& row = m_model->entryFromIndex(index);
+
+        m_wallet->subaddress()->setHidden(row.address, toggled);
         m_proxyModel->invalidate();
     });
     actionHide->setCheckable(true);
-    actionHide->setChecked(row->isHidden());
+    actionHide->setChecked(row.hidden);
 
     if (m_wallet->isHwBacked()) {
         menu->addAction("Show on device", this, &ReceiveWidget::showOnDevice);
@@ -186,20 +196,24 @@ void ReceiveWidget::showContextMenu(const QPoint &point) {
 }
 
 void ReceiveWidget::createPaymentRequest() {
-    SubaddressRow* row = this->currentEntry();
-    if (!row) return;
+    QModelIndex index = getCurrentIndex();
+    if (!index.isValid()) {
+        return;
+    }
 
-    QString address = this->getAddress(row->getRow());
+    QString address = this->getAddress(index.row());
 
     PaymentRequestDialog dialog{this, m_wallet, address};
     dialog.exec();
 }
 
 void ReceiveWidget::onShowTransactions() {
-    SubaddressRow* row = this->currentEntry();
-    if (!row) return;
+    QModelIndex index = getCurrentIndex();
+    if (!index.isValid()) {
+        return;
+    }
 
-    QString address = this->getAddress(row->getRow());
+    QString address = this->getAddress(index.row());
     emit showTransactions(address);
 }
 
@@ -214,9 +228,11 @@ void ReceiveWidget::showHeaderMenu(const QPoint& position)
 }
 
 void ReceiveWidget::showOnDevice() {
-    SubaddressRow* row = this->currentEntry();
-    if (!row) return;
-    m_wallet->deviceShowAddressAsync(m_wallet->currentSubaddressAccount(), row->getRow(), "");
+    QModelIndex index = getCurrentIndex();
+    if (!index.isValid()) {
+        return;
+    }
+    m_wallet->deviceShowAddressAsync(m_wallet->currentSubaddressAccount(), index.row(), "");
 }
 
 void ReceiveWidget::generateSubaddress() {
@@ -227,14 +243,14 @@ void ReceiveWidget::generateSubaddress() {
 }
 
 void ReceiveWidget::updateQrCode(){
-    SubaddressRow* row = this->currentEntry();
-    if (!row) {
+    QModelIndex index = getCurrentIndex();
+    if (!index.isValid()) {
         ui->qrCode->clear();
         ui->btn_createPaymentRequest->hide();
         return;
     }
 
-    QString address = this->getAddress(row->getRow());
+    QString address = this->getAddress(index.row());
     const QrCode qrc(address, QrCode::Version::AUTO, QrCode::ErrorCorrectionLevel::MEDIUM);
 
     int width = ui->qrCode->width() - 4;
@@ -245,22 +261,20 @@ void ReceiveWidget::updateQrCode(){
 }
 
 void ReceiveWidget::showQrCodeDialog() {
-    SubaddressRow* row = this->currentEntry();
-    if (!row) return;
+    QModelIndex index = getCurrentIndex();
+    if (!index.isValid()) {
+        return;
+    }
 
-    QString address = this->getAddress(row->getRow());
+    QString address = this->getAddress(index.row());
     QrCode qr(address, QrCode::Version::AUTO, QrCode::ErrorCorrectionLevel::HIGH);
     QrCodeDialog dialog{this, &qr, "Address"};
     dialog.exec();
 }
 
-SubaddressRow* ReceiveWidget::currentEntry() {
-    QModelIndexList list = ui->addresses->selectionModel()->selectedRows();
-    if (list.size() == 1) {
-        return m_model->entryFromIndex(m_proxyModel->mapToSource(list.first()));
-    } else {
-        return nullptr;
-    }
+QModelIndex ReceiveWidget::getCurrentIndex()
+{
+    return m_proxyModel->mapToSource(ui->addresses->currentIndex());
 }
 
-ReceiveWidget::~ReceiveWidget() = default;
+ReceiveWidget::~ReceiveWidget() = default;

src/ReceiveWidget.h

@@ -55,10 +55,11 @@ private:
     SubaddressModel *m_model;
     SubaddressProxyModel *m_proxyModel;
 
+    QModelIndex getCurrentIndex();
+
     void addOption(QMenu *menu, const QString &text, Config::ConfigKey key, const std::function<void(bool show)>& func);
     void updateQrCode();
     void showQrCodeDialog();
-    SubaddressRow* currentEntry();
 };
 
 #endif //FEATHER_RECEIVEWIDGET_H

src/libwalletqt/Subaddress.cpp

@@ -2,7 +2,6 @@
 // SPDX-FileCopyrightText: The Monero Project
 
 #include "Subaddress.h"
-#include <QDebug>
 
 #include <wallet/wallet2.h>
 
@@ -18,15 +17,17 @@ Subaddress::Subaddress(Wallet *wallet, tools::wallet2 *wallet2, QObject *parent)
     m_hidden = hidden.split(",");
 }
 
-bool Subaddress::getRow(int index, std::function<void (SubaddressRow &row)> callback) const
+const QList<SubaddressRow>& Subaddress::getRows()
 {
-    if (index < 0 || index >= m_rows.size())
-    {
-        return false;
-    }
+    return m_rows;
+}
 
-    callback(*m_rows.value(index));
-    return true;
+const SubaddressRow& Subaddress::getRow(const qsizetype i)
+{
+    if (i < 0 || i >= m_rows.size()) {
+        throw std::out_of_range("Index out of range");
+    }
+    return m_rows[i];
 }
 
 bool Subaddress::addRow(quint32 accountIndex, const QString &label)
@@ -87,7 +88,7 @@ bool Subaddress::setHidden(const QString &address, bool hidden)
 bool Subaddress::isHidden(const QString &address) 
 {
     return m_hidden.contains(address);
-};
+}
 
 bool Subaddress::setPinned(const QString &address, bool pinned) 
 {
@@ -143,16 +144,13 @@ bool Subaddress::refresh(quint32 accountIndex)
 
         QString addressStr = QString::fromStdString(cryptonote::get_account_address_as_str(m_wallet2->nettype(), !index.is_zero(), address));
 
-        auto* row = new SubaddressRow{this,
-                                      i,
-                                      addressStr,
-                                      QString::fromStdString(m_wallet2->get_subaddress_label(index)),
-                                      m_wallet2->get_subaddress_used({accountIndex, (uint32_t)i}),
-                                      this->isHidden(addressStr),
-                                      this->isPinned(addressStr)
-        };
-        
-        m_rows.append(row);
+        m_rows.emplace_back(
+            addressStr,
+            QString::fromStdString(m_wallet2->get_subaddress_label(index)),
+            m_wallet2->get_subaddress_used({accountIndex, (uint32_t)i}),
+            this->isHidden(addressStr),
+            this->isPinned(addressStr)
+        );
     }
 
     // Make sure keys are intact. We NEVER want to display incorrect addresses in case of memory corruption.
@@ -175,14 +173,13 @@ qsizetype Subaddress::count() const
 }
 
 void Subaddress::clearRows() {
-    qDeleteAll(m_rows);
     m_rows.clear();
 }
 
-SubaddressRow* Subaddress::row(int index) const {
-    return m_rows.value(index);
+const SubaddressRow& Subaddress::row(int index) const {
+    return m_rows[index];
 }
 
 QString Subaddress::getError() const {
     return m_errorString;
-}
+}

src/libwalletqt/Subaddress.h

@@ -21,14 +21,17 @@ class Subaddress : public QObject
     Q_OBJECT
 
 public:
-    bool getRow(int index, std::function<void (SubaddressRow &row)> callback) const;
+    const QList<SubaddressRow>& getRows();
+    const SubaddressRow& getRow(qsizetype i);
+
     bool addRow(quint32 accountIndex, const QString &label);
     
     bool setLabel(quint32 accountIndex, quint32 addressIndex, const QString &label);
 
     bool setHidden(const QString& address, bool hidden);
     bool isHidden(const QString& address);
-    
+    bool isHidden(const SubaddressRow& row);
+
     bool setPinned(const QString& address, bool pinned);
     bool isPinned(const QString& address);
     
@@ -37,7 +40,7 @@ public:
     [[nodiscard]] qsizetype count() const;
     void clearRows();
 
-    [[nodiscard]] SubaddressRow* row(int index) const;
+    const SubaddressRow& row(int index) const;
 
     QString getError() const;
 
@@ -52,7 +55,7 @@ private:
 
     Wallet* m_wallet;
     tools::wallet2 *m_wallet2;
-    QList<SubaddressRow*> m_rows;
+    QList<SubaddressRow> m_rows;
     
     QStringList m_pinned;
     QStringList m_hidden;

src/libwalletqt/rows/ContactRow.h

@@ -5,7 +5,6 @@
 #define FEATHER_CONTACTROW_H
 
 #include <QString>
-#include <utility>
 
 struct ContactRow
 {

src/libwalletqt/rows/SubaddressRow.cpp

@@ -1,28 +0,0 @@
-// SPDX-License-Identifier: BSD-3-Clause
-// SPDX-FileCopyrightText: The Monero Project
-
-#include "SubaddressRow.h"
-
-quint32 SubaddressRow::getRow() const {
-    return m_row;
-}
-
-const QString& SubaddressRow::getAddress() const {
-    return m_address;
-}
-
-const QString& SubaddressRow::getLabel() const {
-    return m_label;
-}
-
-bool SubaddressRow::isUsed() const {
-    return m_used;
-}
-
-bool SubaddressRow::isPinned() const {
-    return m_pinned;
-}
-
-bool SubaddressRow::isHidden() const {
-    return m_hidden;
-}

src/libwalletqt/rows/SubaddressRow.h

@@ -4,37 +4,22 @@
 #ifndef FEATHER_SUBADDRESSROW_H
 #define FEATHER_SUBADDRESSROW_H
 
-#include <QObject>
+#include <QString>
 
-class SubaddressRow : public QObject 
+struct SubaddressRow
 {
-    Q_OBJECT
-    
-public:
-    SubaddressRow(QObject *parent, quint32 row, const QString& address, const QString &label, bool used, bool hidden, bool pinned)
-        : QObject(parent)
-        , m_row(row)
-        , m_address(address)
-        , m_label(label)
-        , m_used(used) 
-        , m_hidden(hidden)
-        , m_pinned(pinned) {}
-    
-    [[nodiscard]] quint32 getRow() const;
-    const QString& getAddress() const;
-    const QString& getLabel() const;
-    bool isUsed() const;
-    bool isHidden() const;
-    bool isPinned() const;
-    
-private:
-    quint32 m_row;
-    QString m_address;
-    QString m_label;
-    bool m_used = false;
-    bool m_hidden = false;
-    bool m_pinned = false;
+    QString address;
+    QString label;
+    bool used = false;
+    bool hidden = false;
+    bool pinned = false;
+
+    SubaddressRow(const QString& address_, const QString &label_, bool used_, bool hidden_, bool pinned_)
+        : address(address_)
+        , label(label_)
+        , used(used_)
+        , hidden(hidden_)
+        , pinned(pinned_) {}
 };
 
-
 #endif //FEATHER_SUBADDRESSROW_H

src/model/SubaddressModel.cpp

@@ -40,60 +40,54 @@ int SubaddressModel::columnCount(const QModelIndex &parent) const
 
 QVariant SubaddressModel::data(const QModelIndex &index, int role) const
 {
-    if (!index.isValid() || index.row() < 0 || static_cast<quint64>(index.row()) >= m_subaddress->count())
-        return QVariant();
-
-    QVariant result;
+    const QList<SubaddressRow>& rows = m_subaddress->getRows();
+    if (index.row() < 0 || index.row() >= rows.size()) {
+        return {};
+    }
+    const SubaddressRow& row = rows[index.row()];
     
-    bool found = m_subaddress->getRow(index.row(), [this, &index, &role, &result](const SubaddressRow &subaddress) {
-        if (role == Qt::DisplayRole || role == Qt::EditRole || role == Qt::UserRole){
-            result = parseSubaddressRow(subaddress, index, role);
-        }
-        
-        else if (role == Qt::DecorationRole) {
-            if (subaddress.isPinned() && index.column() == ModelColumn::Index) {
-                result = QVariant(icons()->icon("pin.png"));
-            }
-            else if (subaddress.isHidden() && index.column() == ModelColumn::Index) {
-                result = QVariant(icons()->icon("eye_blind.png"));
-            }
-        }
-        else if (role == Qt::BackgroundRole) {
-            switch(index.column()) {
-                case Address:
-                {
-                    if (subaddress.isUsed()) {
-                        result = QBrush(ColorScheme::RED.asColor(true));
-                    }
-                }
-            }
-        }
-        else if (role == Qt::FontRole) {
-            switch(index.column()) {
-                case Address:
-                {
-                   result = Utils::getMonospaceFont();
-                }
-            }
-        }
-        else if (role == Qt::ToolTipRole) {
-            switch(index.column()) {
-                case Address:
-                {
-                    if (subaddress.isUsed()) {
-                        result = "This address is used.";
-                    }
-                }
-            }
-        }
-    });
-
-    if (!found)
-    {
-        qCritical("%s: internal error: invalid index %d", __FUNCTION__, index.row());
+    if (role == Qt::DisplayRole || role == Qt::EditRole || role == Qt::UserRole){
+        return parseSubaddressRow(row, index, role);
     }
 
-    return result;
+    else if (role == Qt::DecorationRole) {
+        if (row.pinned && index.column() == ModelColumn::Index) {
+            return QVariant(icons()->icon("pin.png"));
+        }
+        else if (row.hidden && index.column() == ModelColumn::Index) {
+            return QVariant(icons()->icon("eye_blind.png"));
+        }
+    }
+    else if (role == Qt::BackgroundRole) {
+        switch(index.column()) {
+            case Address:
+            {
+                if (row.used) {
+                    return QBrush(ColorScheme::RED.asColor(true));
+                }
+            }
+        }
+    }
+    else if (role == Qt::FontRole) {
+        switch(index.column()) {
+            case Address:
+            {
+               return Utils::getMonospaceFont();
+            }
+        }
+    }
+    else if (role == Qt::ToolTipRole) {
+        switch(index.column()) {
+            case Address:
+            {
+                if (row.used) {
+                    return "This address is used.";
+                }
+            }
+        }
+    }
+
+    return {};
 }
 
 QVariant SubaddressModel::parseSubaddressRow(const SubaddressRow &subaddress, const QModelIndex &index, int role) const
@@ -103,17 +97,17 @@ QVariant SubaddressModel::parseSubaddressRow(const SubaddressRow &subaddress, co
         case Index:
         {
             if (role == Qt::UserRole) {
-                if (subaddress.isPinned()) {
+                if (subaddress.pinned) {
                     return 0;
                 } else {
-                    return subaddress.getRow() + 1;
+                    return index.row() + 1;
                 }
             }
-            return "#" + QString::number(subaddress.getRow()) + " ";
+            return "#" + QString::number(index.row()) + " ";
         }
         case Address:
         {
-            QString address = subaddress.getAddress();
+            QString address = subaddress.address;
             if (!showFull && role != Qt::UserRole) {
                 address = Utils::displayAddress(address);
             }
@@ -127,10 +121,10 @@ QVariant SubaddressModel::parseSubaddressRow(const SubaddressRow &subaddress, co
             else if (index.row() == 0) {
                 return "Change";
             }
-            return subaddress.getLabel();
+            return subaddress.label;
         }
         case isUsed:
-            return subaddress.isUsed();
+            return subaddress.used;
         default:
             qCritical() << "Invalid column" << index.column();
             return QVariant();
@@ -195,7 +189,7 @@ void SubaddressModel::setCurrentSubaddressAccount(quint32 accountIndex) {
     m_currentSubaddressAccount = accountIndex;
 }
 
-SubaddressRow* SubaddressModel::entryFromIndex(const QModelIndex &index) const {
+const SubaddressRow& SubaddressModel::entryFromIndex(const QModelIndex &index) const {
     Q_ASSERT(index.isValid() && index.row() < m_subaddress->count());
     return m_subaddress->row(index.row());
-}
+}

src/model/SubaddressModel.h

@@ -34,7 +34,7 @@ public:
 
     bool setData(const QModelIndex &index, const QVariant &value, int role) override;
 
-    SubaddressRow* entryFromIndex(const QModelIndex &index) const;
+    const SubaddressRow& entryFromIndex(const QModelIndex &index) const;
 
     void setCurrentSubaddressAccount(quint32 accountIndex);
 

src/model/SubaddressProxyModel.cpp

@@ -22,13 +22,14 @@ bool SubaddressProxyModel::filterAcceptsRow(int sourceRow, const QModelIndex &so
     bool showHidden = conf()->get(Config::showHiddenAddresses).toBool();
     bool showChange = conf()->get(Config::showChangeAddresses).toBool();
 
-    SubaddressRow* subaddress = m_subaddress->row(sourceRow);
-    if (!subaddress) {
+    if (sourceRow < 0 || sourceRow >= m_subaddress->count()) {
         return false;
     }
+
+    const SubaddressRow& subaddress = m_subaddress->row(sourceRow);
     
     // Pinned addresses are always shown
-    if (subaddress->isPinned()) {
+    if (subaddress.pinned) {
         return true;
     }
     
@@ -37,13 +38,13 @@ bool SubaddressProxyModel::filterAcceptsRow(int sourceRow, const QModelIndex &so
         return false;
     }
 
-    if (!showHidden && subaddress->isHidden()) {
+    if (!showHidden && subaddress.hidden) {
         return false;
     }
 
     if (!m_searchRegExp.pattern().isEmpty()) {
-        return subaddress->getAddress().contains(m_searchCaseSensitiveRegExp) || subaddress->getLabel().contains(m_searchRegExp);
+        return subaddress.address.contains(m_searchCaseSensitiveRegExp) || subaddress.label.contains(m_searchRegExp);
     }
 
-    return (showUsed || !subaddress->isUsed());
+    return (showUsed || !subaddress.used);
 }