Add warnings for non-active addresses in receive tab and address book

doc/release-notes/release-notes-27216.md

@@ -0,0 +1,8 @@
+Updated RPCs
+------------
+
+- RPC `getaddressinfo` now returns a new boolean field `"isactive"`, which is
+only `true` if the address is derived from an active descriptor or HD seed.
+This information can inform a user to avoid reusing certain addresses that may
+have been imported from external sources or generated by their wallet before it
+was encrypted. (#27216)

src/interfaces/wallet.h

@@ -46,6 +46,7 @@ namespace interfaces {
 
 class Handler;
 struct WalletAddress;
+struct ReceiveRequest;
 struct WalletBalances;
 struct WalletTx;
 struct WalletTxOut;
@@ -118,8 +119,8 @@ class Wallet
     //! Get wallet address list.
     virtual std::vector<WalletAddress> getAddresses() const = 0;
 
-    //! Get receive requests.
-    virtual std::vector<std::string> getAddressReceiveRequests() = 0;
+    //! Get receive requests. Bool indicating key is active, string containing serialized destination data
+    virtual std::vector<ReceiveRequest> getAddressReceiveRequests() = 0;
 
     //! Save or remove receive request.
     virtual bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) = 0;
@@ -292,10 +293,12 @@ class Wallet
 
     //! Register handler for address book changed messages.
     using AddressBookChangedFn = std::function<void(const CTxDestination& address,
-        const std::string& label,
-        bool is_mine,
-        wallet::AddressPurpose purpose,
-        ChangeType status)>;
+                                                    const std::string& label,
+                                                    bool is_mine,
+                                                    wallet::AddressPurpose purpose,
+                                                    ChangeType status,
+                                                    bool is_active)>;
+
     virtual std::unique_ptr<Handler> handleAddressBookChanged(AddressBookChangedFn fn) = 0;
 
     //! Register handler for transaction changed messages.
@@ -355,13 +358,25 @@ struct WalletAddress
     wallet::isminetype is_mine;
     wallet::AddressPurpose purpose;
     std::string name;
+    bool is_active{false};
 
-    WalletAddress(CTxDestination dest, wallet::isminetype is_mine, wallet::AddressPurpose purpose, std::string name)
-        : dest(std::move(dest)), is_mine(is_mine), purpose(std::move(purpose)), name(std::move(name))
+    WalletAddress(CTxDestination dest, wallet::isminetype is_mine, wallet::AddressPurpose purpose, std::string name, bool is_active)
+        : dest(std::move(dest)), is_mine(is_mine), purpose(std::move(purpose)), name(std::move(name)), is_active(is_active)
     {
     }
 };
 
+//! Information about one receive request.
+struct ReceiveRequest {
+    std::string m_data;
+    bool m_is_active;
+
+    ReceiveRequest(std::string data, bool is_active)
+        : m_data(std::move(data)), m_is_active(is_active) {}
+
+    bool operator==(const ReceiveRequest& a) const { return (m_data == a.m_data) && (m_is_active == a.m_is_active); };
+};
+
 //! Collection of wallet balances.
 struct WalletBalances
 {

src/qt/addresstablemodel.cpp

@@ -30,10 +30,12 @@ struct AddressTableEntry
     Type type;
     QString label;
     QString address;
+    bool is_active;
 
     AddressTableEntry() = default;
-    AddressTableEntry(Type _type, const QString &_label, const QString &_address):
-        type(_type), label(_label), address(_address) {}
+    AddressTableEntry(Type _type, const QString& _label, const QString& _address, bool is_active) : type(_type), label(_label), address(_address), is_active(is_active) {}
+
+    [[nodiscard]] QString GetAddressWarnings() const;
 };
 
 struct AddressTableEntryLessThan
@@ -65,6 +67,15 @@ static AddressTableEntry::Type translateTransactionType(wallet::AddressPurpose p
     assert(false);
 }
 
+[[nodiscard]] QString AddressTableEntry::GetAddressWarnings() const
+{
+    QString warnings;
+    if (!is_active)
+        warnings += QObject::tr("This address should not be used. It was derived from an inactive seed, was imported, or may have been stored unencrypted.\n");
+
+    return warnings;
+}
+
 // Private implementation
 class AddressTablePriv
 {
@@ -87,8 +98,9 @@ class AddressTablePriv
                 AddressTableEntry::Type addressType = translateTransactionType(
                         address.purpose, address.is_mine);
                 cachedAddressTable.append(AddressTableEntry(addressType,
-                                  QString::fromStdString(address.name),
-                                  QString::fromStdString(EncodeDestination(address.dest))));
+                                                            QString::fromStdString(address.name),
+                                                            QString::fromStdString(EncodeDestination(address.dest)),
+                                                            address.is_active));
             }
         }
         // std::lower_bound() and std::upper_bound() require our cachedAddressTable list to be sorted in asc order
@@ -97,7 +109,7 @@ class AddressTablePriv
         std::sort(cachedAddressTable.begin(), cachedAddressTable.end(), AddressTableEntryLessThan());
     }
 
-    void updateEntry(const QString &address, const QString &label, bool isMine, wallet::AddressPurpose purpose, int status)
+    void updateEntry(const QString& address, const QString& label, bool isMine, wallet::AddressPurpose purpose, int status, bool isActive)
     {
         // Find address / label in model
         QList<AddressTableEntry>::iterator lower = std::lower_bound(
@@ -118,7 +130,7 @@ class AddressTablePriv
                 break;
             }
             parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex);
-            cachedAddressTable.insert(lowerIndex, AddressTableEntry(newEntryType, label, address));
+            cachedAddressTable.insert(lowerIndex, AddressTableEntry(newEntryType, label, address, isActive));
             parent->endInsertRows();
             break;
         case CT_UPDATED:
@@ -162,10 +174,9 @@ class AddressTablePriv
     }
 };
 
-AddressTableModel::AddressTableModel(WalletModel *parent, bool pk_hash_only) :
-    QAbstractTableModel(parent), walletModel(parent)
+AddressTableModel::AddressTableModel(const PlatformStyle* platformStyle, WalletModel* parent, bool pk_hash_only) : QAbstractTableModel(parent), walletModel(parent), platformStyle(platformStyle)
 {
-    columns << tr("Label") << tr("Address");
+    columns << tr("Warnings") << tr("Label") << tr("Address");
     priv = new AddressTablePriv(this);
     priv->refreshAddressTable(parent->wallet(), pk_hash_only);
 }
@@ -201,6 +212,8 @@ QVariant AddressTableModel::data(const QModelIndex &index, int role) const
     const auto column = static_cast<ColumnIndex>(index.column());
     if (role == Qt::DisplayRole || role == Qt::EditRole) {
         switch (column) {
+        case Warnings:
+            return {};
         case Label:
             if (rec->label.isEmpty() && role == Qt::DisplayRole) {
                 return tr("(no label)");
@@ -213,6 +226,7 @@ QVariant AddressTableModel::data(const QModelIndex &index, int role) const
         assert(false);
     } else if (role == Qt::FontRole) {
         switch (column) {
+        case Warnings:
         case Label:
             return QFont();
         case Address:
@@ -230,6 +244,16 @@ QVariant AddressTableModel::data(const QModelIndex &index, int role) const
             return {};
         } // no default case, so the compiler can warn about missing cases
         assert(false);
+    } else if (role == Qt::DecorationRole) {
+        if (index.column() == Warnings) {
+            if (rec->GetAddressWarnings().isEmpty()) {
+                return {};
+            } else {
+                return platformStyle->TextColorIcon(QIcon(":/icons/warning"));
+            }
+        }
+    } else if (role == Qt::ToolTipRole) {
+        if (index.column() == Warnings) return rec->GetAddressWarnings();
     }
     return QVariant();
 }
@@ -333,11 +357,10 @@ QModelIndex AddressTableModel::index(int row, int column, const QModelIndex &par
     }
 }
 
-void AddressTableModel::updateEntry(const QString &address,
-        const QString &label, bool isMine, wallet::AddressPurpose purpose, int status)
+void AddressTableModel::updateEntry(const QString& address, const QString& label, bool isMine, wallet::AddressPurpose purpose, int status, bool isActive)
 {
     // Update address book model from Bitcoin core
-    priv->updateEntry(address, label, isMine, purpose, status);
+    priv->updateEntry(address, label, isMine, purpose, status, isActive);
 }
 
 QString AddressTableModel::addRow(const QString &type, const QString &label, const QString &address, const OutputType address_type)

src/qt/addresstablemodel.h

@@ -6,6 +6,7 @@
 #define BITCOIN_QT_ADDRESSTABLEMODEL_H
 
 #include <optional>
+#include <qt/platformstyle.h>
 
 #include <QAbstractTableModel>
 #include <QStringList>
@@ -30,12 +31,13 @@ class AddressTableModel : public QAbstractTableModel
     Q_OBJECT
 
 public:
-    explicit AddressTableModel(WalletModel *parent = nullptr, bool pk_hash_only = false);
+    explicit AddressTableModel(const PlatformStyle* platformStyle, WalletModel* parent = nullptr, bool pk_hash_only = false);
     ~AddressTableModel();
 
     enum ColumnIndex {
-        Label = 0,   /**< User specified label */
-        Address = 1  /**< Bitcoin address */
+        Warnings = 0, /**< Warn user about using this address with tooltip explanation */
+        Label,        /**< User specified label */
+        Address       /**< Bitcoin address */
     };
 
     enum RoleIndex {
@@ -92,6 +94,7 @@ class AddressTableModel : public QAbstractTableModel
     AddressTablePriv *priv = nullptr;
     QStringList columns;
     EditStatus editStatus = OK;
+    const PlatformStyle* platformStyle;
 
     /** Look up address book data given an address string. */
     bool getAddressData(const QString &address, std::string* name, wallet::AddressPurpose* purpose) const;
@@ -102,7 +105,7 @@ class AddressTableModel : public QAbstractTableModel
 public Q_SLOTS:
     /* Update address list from core.
      */
-    void updateEntry(const QString &address, const QString &label, bool isMine, wallet::AddressPurpose purpose, int status);
+    void updateEntry(const QString& address, const QString& label, bool isMine, wallet::AddressPurpose purpose, int status, bool isActive);
 
     friend class AddressTablePriv;
 };

src/qt/bitcoingui.cpp

@@ -1418,6 +1418,9 @@ void BitcoinGUI::updateWalletStatus()
     WalletModel * const walletModel = walletView->getWalletModel();
     setEncryptionStatus(walletModel->getEncryptionStatus());
     setHDStatus(walletModel->wallet().privateKeysDisabled(), walletModel->wallet().hdEnabled());
+
+    // Refresh requests table and address book to show warnings on old unencrypted keys
+    walletView->refreshAddressTables();
 }
 #endif // ENABLE_WALLET
 

src/qt/forms/receiverequestdialog.ui

@@ -232,7 +232,39 @@
      </property>
     </widget>
    </item>
-   <item row="8" column="0" colspan="2">
+   <item row="8" column="0" alignment="Qt::AlignRight|Qt::AlignTop">
+    <widget class="QLabel" name="warnings_tag">
+     <property name="font">
+      <font>
+       <weight>75</weight>
+       <bold>true</bold>
+      </font>
+     </property>
+     <property name="text">
+      <string>Warnings:</string>
+     </property>
+     <property name="textInteractionFlags">
+      <set>Qt::NoTextInteraction</set>
+     </property>
+    </widget>
+   </item>
+   <item row="8" column="1" alignment="Qt::AlignTop">
+    <widget class="QLabel" name="warnings_content">
+     <property name="text">
+      <string notr="true">warnings</string>
+     </property>
+     <property name="textFormat">
+      <enum>Qt::PlainText</enum>
+     </property>
+     <property name="wordWrap">
+      <bool>true</bool>
+     </property>
+     <property name="textInteractionFlags">
+      <set>Qt::TextSelectableByMouse</set>
+     </property>
+    </widget>
+   </item>
+   <item row="9" column="0" colspan="2">
     <layout class="QHBoxLayout" name="horizontalLayout">
      <item>
       <widget class="QPushButton" name="btnCopyURI">

src/qt/receivecoinsdialog.cpp

@@ -160,14 +160,19 @@ void ReceiveCoinsDialog::on_receiveButton_clicked()
         // Success
         SendCoinsRecipient info(address, label,
             ui->reqAmount->value(), ui->reqMessage->text());
+
+        /* Store request for later reference */
+        std::optional<RecentRequestEntry> entry = model->getRecentRequestsTableModel()->addNewRequest(info);
+
+        // Wallet failed to set address receive request
+        if (!entry.has_value()) break;
+
         ReceiveRequestDialog *dialog = new ReceiveRequestDialog(this);
         dialog->setAttribute(Qt::WA_DeleteOnClose);
         dialog->setModel(model);
-        dialog->setInfo(info);
+        dialog->setInfo(entry.value());
         dialog->show();
 
-        /* Store request for later reference */
-        model->getRecentRequestsTableModel()->addNewRequest(info);
         break;
     }
     case AddressTableModel::EditStatus::WALLET_UNLOCK_FAILURE:
@@ -194,7 +199,7 @@ void ReceiveCoinsDialog::on_recentRequestsView_doubleClicked(const QModelIndex &
     const RecentRequestsTableModel *submodel = model->getRecentRequestsTableModel();
     ReceiveRequestDialog *dialog = new ReceiveRequestDialog(this);
     dialog->setModel(model);
-    dialog->setInfo(submodel->entry(index.row()).recipient);
+    dialog->setInfo(submodel->entry(index.row()));
     dialog->setAttribute(Qt::WA_DeleteOnClose);
     dialog->show();
 }

src/qt/receiverequestdialog.cpp

@@ -42,9 +42,9 @@ void ReceiveRequestDialog::setModel(WalletModel *_model)
     update();
 }
 
-void ReceiveRequestDialog::setInfo(const SendCoinsRecipient &_info)
+void ReceiveRequestDialog::setInfo(const RecentRequestEntry& entry)
 {
-    this->info = _info;
+    this->info = entry.recipient;
     setWindowTitle(tr("Request payment to %1").arg(info.label.isEmpty() ? info.address : info.label));
     QString uri = GUIUtil::formatBitcoinURI(info);
 
@@ -89,6 +89,14 @@ void ReceiveRequestDialog::setInfo(const SendCoinsRecipient &_info)
         ui->wallet_content->hide();
     }
 
+    QString warnings{entry.GetAddressWarnings()};
+    if (warnings.isEmpty()) {
+        ui->warnings_tag->hide();
+        ui->warnings_content->hide();
+    } else {
+        ui->warnings_content->setText(warnings);
+    }
+
     ui->btnVerify->setVisible(model->wallet().hasExternalSigner());
 
     connect(ui->btnVerify, &QPushButton::clicked, [this] {

src/qt/receiverequestdialog.h

@@ -5,7 +5,7 @@
 #ifndef BITCOIN_QT_RECEIVEREQUESTDIALOG_H
 #define BITCOIN_QT_RECEIVEREQUESTDIALOG_H
 
-#include <qt/sendcoinsrecipient.h>
+#include <qt/recentrequeststablemodel.h>
 
 #include <QDialog>
 
@@ -24,7 +24,7 @@ class ReceiveRequestDialog : public QDialog
     ~ReceiveRequestDialog();
 
     void setModel(WalletModel *model);
-    void setInfo(const SendCoinsRecipient &info);
+    void setInfo(const RecentRequestEntry& entry);
 
 private Q_SLOTS:
     void on_btnCopyURI_clicked();