Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add new "address type" column to the "receiving tab" address book page #753

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+203 −37
Open

Add new "address type" column to the "receiving tab" address book page #753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc0e33f
gui: Refactor AddressType from ReceiveCoinsDialog
pablomartin4btc Sep 8, 2023
90284c7
wallet: Correct output type for Segwit address
pablomartin4btc Sep 9, 2023
7be6b79
gui: Add Address Type Column to AddressBookPage
pablomartin4btc Sep 9, 2023
5d597c0
gui: Extend address book filter for nested filtering
pablomartin4btc Sep 10, 2023
dffc37f
gui: Add a combo widget to filter by address type
pablomartin4btc Sep 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/interfaces/wallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ class Wallet
//! Get public key.
virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;

//! Get Output type from a Destination.
virtual OutputType getOutputType(const CTxDestination& dest) = 0;

//! Sign message
virtual SigningResult signMessage(const std::string& message, const PKHash& pkhash, std::string& str_sig) = 0;

Expand Down
101 changes: 91 additions & 10 deletions src/qt/addressbookpage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,31 @@
class AddressBookSortFilterProxyModel final : public QSortFilterProxyModel
{
const QString m_type;
const bool m_nestedFilterEnabled;

public:
AddressBookSortFilterProxyModel(const QString& type, QObject* parent)
AddressBookSortFilterProxyModel(const QString& type, QObject* parent, bool enableNestedFilter)
: QSortFilterProxyModel(parent)
, m_type(type)
, m_nestedFilterEnabled(enableNestedFilter)
{
setDynamicSortFilter(true);
setFilterCaseSensitivity(Qt::CaseInsensitive);
setSortCaseSensitivity(Qt::CaseInsensitive);

if (m_nestedFilterEnabled) {
nextedFilterProxyModel.reset(new AddressBookSortFilterProxyModel(type, this, false));
nextedFilterProxyModel->setSourceModel(this);
}
}

AddressBookSortFilterProxyModel* nestedProxyModel() const noexcept{
if (!m_nestedFilterEnabled) return const_cast<AddressBookSortFilterProxyModel*>(this);
return nextedFilterProxyModel.get();
}

bool isNestedFilterEnabled() const {
return m_nestedFilterEnabled;
}

protected:
Expand All @@ -50,15 +66,25 @@ class AddressBookSortFilterProxyModel final : public QSortFilterProxyModel
}

auto address = model->index(row, AddressTableModel::Address, parent);
auto addressType = model->index(row, AddressTableModel::Type, parent);

#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))
const auto pattern = filterRegularExpression();
#else
const auto pattern = filterRegExp();
#endif
return (model->data(address).toString().contains(pattern) ||
model->data(label).toString().contains(pattern));
auto filterBy = model->data(address).toString().contains(pattern) ||
model->data(label).toString().contains(pattern);

if (m_type == AddressTableModel::Receive) {
filterBy = filterBy || model->data(addressType).toString().contains(pattern);
}

return filterBy;
}

private:
std::unique_ptr<AddressBookSortFilterProxyModel> nextedFilterProxyModel{nullptr};
};

AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode, Tabs _tab, QWidget *parent) :
Expand Down Expand Up @@ -99,11 +125,13 @@ AddressBookPage::AddressBookPage(const PlatformStyle *platformStyle, Mode _mode,
ui->labelExplanation->setText(tr("These are your Bitcoin addresses for sending payments. Always check the amount and the receiving address before sending coins."));
ui->deleteAddress->setVisible(true);
ui->newAddress->setVisible(true);
ui->searchLineEdit->setPlaceholderText("Enter address or label to search");
break;
case ReceivingTab:
ui->labelExplanation->setText(tr("These are your Bitcoin addresses for receiving payments. Use the 'Create new receiving address' button in the receive tab to create new addresses.\nSigning is only possible with addresses of the type 'legacy'."));
ui->deleteAddress->setVisible(false);
ui->newAddress->setVisible(false);
ui->searchLineEdit->setPlaceholderText("Enter address, address type or label to search");
break;
}

Expand Down Expand Up @@ -135,17 +163,20 @@ void AddressBookPage::setModel(AddressTableModel *_model)
return;

auto type = tab == ReceivingTab ? AddressTableModel::Receive : AddressTableModel::Send;
proxyModel = new AddressBookSortFilterProxyModel(type, this);
proxyModel.reset(new AddressBookSortFilterProxyModel(type, this, true));
proxyModel->setSourceModel(_model);

connect(ui->searchLineEdit, &QLineEdit::textChanged, proxyModel, &QSortFilterProxyModel::setFilterWildcard);
connect(ui->searchLineEdit, &QLineEdit::textChanged, proxyModel.get(), &QSortFilterProxyModel::setFilterWildcard);

ui->tableView->setModel(proxyModel);
ui->tableView->setModel(proxyModel->nestedProxyModel());
ui->tableView->sortByColumn(0, Qt::AscendingOrder);

// Set column widths
ui->tableView->horizontalHeader()->setSectionResizeMode(AddressTableModel::Label, QHeaderView::Stretch);
ui->tableView->horizontalHeader()->setSectionResizeMode(AddressTableModel::Label, QHeaderView::ResizeToContents);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change?

from this
Screenshot from 2024-04-12 16-02-55

to this
Screenshot from 2024-04-12 16-03-38

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, this is something that could be subject for discussion, I'm open to any suggestions. Originally, as for this change, I decided to change the resize mode from Stretch to ResizeToContents as it looked tighter to me (resize mode enum - qt doc ref). I was thinking to make a bit of a mix of both of them, Stretch for some columns and ResizeToContents to others (e.g. check here), but I see there are other devs that also calculate the size of certain columns (in the latest link they also mentioned the property setStretchLastSection, which extends the size of the last column to cover the rest of the table so there's no gap as you see in your 2nd screenshot, and I played a bit with but I finally discarded it cos there was an unexpected behaviour that can't remember now). We could also allow users resize each column as their preference and save them in the settings (I think this is done on the "Peers" table).

Please let me know your opinion on this or if you have any suggestions. Thanks!

ui->tableView->horizontalHeader()->setSectionResizeMode(AddressTableModel::Address, QHeaderView::ResizeToContents);
ui->tableView->horizontalHeader()->setSectionResizeMode(AddressTableModel::Type, QHeaderView::ResizeToContents);
// Show the "Address type" column only on the Receiving tab
ui->tableView->setColumnHidden(AddressTableModel::ColumnIndex::Type, (tab == SendingTab));

connect(ui->tableView->selectionModel(), &QItemSelectionModel::selectionChanged,
this, &AddressBookPage::selectionChanged);
Expand All @@ -155,6 +186,8 @@ void AddressBookPage::setModel(AddressTableModel *_model)

selectionChanged();
this->updateWindowsTitleWithWalletName();

this->setupAddressTypeCombo();
}

void AddressBookPage::on_copyAddress_clicked()
Expand Down Expand Up @@ -183,7 +216,7 @@ void AddressBookPage::onEditAction()
EditAddressDialog::EditSendingAddress :
EditAddressDialog::EditReceivingAddress, this);
dlg->setModel(model);
QModelIndex origIndex = proxyModel->mapToSource(indexes.at(0));
QModelIndex origIndex = proxyModel->nestedProxyModel()->mapToSource(indexes.at(0));
dlg->loadRow(origIndex.row());
GUIUtil::ShowModalDialogAsynchronously(dlg);
}
Expand Down Expand Up @@ -287,8 +320,9 @@ void AddressBookPage::on_exportButton_clicked()
CSVModelWriter writer(filename);

// name, column, role
writer.setModel(proxyModel);
writer.setModel(proxyModel->nestedProxyModel());
writer.addColumn("Label", AddressTableModel::Label, Qt::EditRole);
writer.addColumn("Address Type", AddressTableModel::Type, Qt::EditRole);
writer.addColumn("Address", AddressTableModel::Address, Qt::EditRole);

if(!writer.write()) {
Expand All @@ -310,7 +344,7 @@ void AddressBookPage::contextualMenu(const QPoint &point)

void AddressBookPage::selectNewAddress(const QModelIndex &parent, int begin, int /*end*/)
{
QModelIndex idx = proxyModel->mapFromSource(model->index(begin, AddressTableModel::Address, parent));
QModelIndex idx = proxyModel.get()->mapFromSource(model->index(begin, AddressTableModel::Address, parent));
if(idx.isValid() && (idx.data(Qt::EditRole).toString() == newAddressToSelect))
{
// Select row of newly created address, once
Expand All @@ -332,3 +366,50 @@ void AddressBookPage::updateWindowsTitleWithWalletName()
}
}
}

std::map<OutputType, QString> AddressBookPage::addressTypeTooltipMap() {
return {{OutputType::LEGACY, QObject::tr("Not recommended due to higher fees and less protection against typos.")},
{OutputType::P2SH_SEGWIT, QObject::tr("An address compatible with older wallets.")},
{OutputType::BECH32, QObject::tr("Native segwit address (BIP-173). Some old wallets don't support it.")},
{OutputType::BECH32M, QObject::tr("Bech32m (BIP-350) is an upgrade to Bech32, wallet support is still limited.")}};
}

QString AddressBookPage::showAllTypes() const{
return QObject::tr("All");
}

QString AddressBookPage::showAllTypesToolTip() const{
return QObject::tr("Select an address type to filter by.");
}

void AddressBookPage::handleAddressTypeChanged(int index)
{
QString selectedValue = ui->addressType->currentText();
// If show all types is selected then clear the selected value
// that will be sent to the filter so it shows everything
if (selectedValue == showAllTypes()) selectedValue.clear();
// Emit a signal with the selected value
Q_EMIT addressTypeChanged(selectedValue);
// Forcing the resize as if it was selected an item with
// shorter content and right after a longer one, the
// columns are not resizing properly, this fixes it
ui->tableView->resizeColumnsToContents();
}

void AddressBookPage::initializeAddressTypeCombo()
{
const auto index = ui->addressType->count();
ui->addressType->addItem(showAllTypes(), index);
ui->addressType->setItemData(index, showAllTypesToolTip(), Qt::ToolTipRole);
ui->addressType->setCurrentIndex(index);
}

void AddressBookPage::setupAddressTypeCombo()
{
this->initializeAddressTypeCombo();
ui->labelAddressType->setVisible(tab == ReceivingTab);
ui->addressType->setVisible(tab == ReceivingTab);
GUIUtil::AddItemsToAddressTypeCombo(ui->addressType, true, this->addressTypeTooltipMap());
connect(ui->addressType, qOverload<int>(&QComboBox::currentIndexChanged), this, &AddressBookPage::handleAddressTypeChanged);
connect(this, &AddressBookPage::addressTypeChanged, proxyModel->nestedProxyModel(), &QSortFilterProxyModel::setFilterFixedString);
}
18 changes: 17 additions & 1 deletion src/qt/addressbookpage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef BITCOIN_QT_ADDRESSBOOKPAGE_H
#define BITCOIN_QT_ADDRESSBOOKPAGE_H

#include <outputtype.h>

#include <QDialog>

class AddressBookSortFilterProxyModel;
Expand Down Expand Up @@ -53,10 +55,17 @@ public Q_SLOTS:
Mode mode;
Tabs tab;
QString returnValue;
AddressBookSortFilterProxyModel *proxyModel;
std::unique_ptr<AddressBookSortFilterProxyModel> proxyModel{nullptr};
QMenu *contextMenu;
QString newAddressToSelect;
void updateWindowsTitleWithWalletName();
void initializeAddressTypeCombo();
/** Default selected item of the address type combo to display all address types */
QString showAllTypes() const;
QString showAllTypesToolTip() const;
/** Tooltip for each address type that will be displayed on the combo*/
std::map<OutputType, QString> addressTypeTooltipMap();
void setupAddressTypeCombo();

private Q_SLOTS:
/** Delete currently selected address entry */
Expand All @@ -78,9 +87,16 @@ private Q_SLOTS:
void contextualMenu(const QPoint &point);
/** New entry/entries were added to address table */
void selectNewAddress(const QModelIndex &parent, int begin, int /*end*/);
/** Address type combo selection changed */
void handleAddressTypeChanged(int index);

Q_SIGNALS:
void sendCoins(QString addr);
/** Emitted when the addressType combobox is changed (handled by handleAddressTypeChange).
* This signal is used as a workaround to connect the combobox and the proxy model filter,
* preventing the compiler error "Signal and slot arguments are not compatible."*/
void addressTypeChanged(const QString &addressType);

};

#endif // BITCOIN_QT_ADDRESSBOOKPAGE_H
25 changes: 16 additions & 9 deletions src/qt/addresstablemodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ struct AddressTableEntry
Type type;
QString label;
QString address;
QString addressType;

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, const OutputType &_addresstype):
type(_type), label(_label), address(_address) {
addressType = GUIUtil::outputTypeDescriptionsMap().at(_addresstype).description;
}
};

struct AddressTableEntryLessThan
Expand Down Expand Up @@ -87,7 +90,8 @@ class AddressTablePriv
address.purpose, address.is_mine);
cachedAddressTable.append(AddressTableEntry(addressType,
QString::fromStdString(address.name),
QString::fromStdString(EncodeDestination(address.dest))));
QString::fromStdString(EncodeDestination(address.dest)),
wallet.getOutputType(address.dest)));
}
}
// std::lower_bound() and std::upper_bound() require our cachedAddressTable list to be sorted in asc order
Expand All @@ -96,7 +100,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, const OutputType &addresstype)
{
// Find address / label in model
QList<AddressTableEntry>::iterator lower = std::lower_bound(
Expand All @@ -117,7 +121,7 @@ class AddressTablePriv
break;
}
parent->beginInsertRows(QModelIndex(), lowerIndex, lowerIndex);
cachedAddressTable.insert(lowerIndex, AddressTableEntry(newEntryType, label, address));
cachedAddressTable.insert(lowerIndex, AddressTableEntry(newEntryType, label, address, addresstype));
parent->endInsertRows();
break;
case CT_UPDATED:
Expand Down Expand Up @@ -164,7 +168,7 @@ class AddressTablePriv
AddressTableModel::AddressTableModel(WalletModel *parent, bool pk_hash_only) :
QAbstractTableModel(parent), walletModel(parent)
{
columns << tr("Label") << tr("Address");
columns << tr("Label") << tr("Address Type") << tr("Address");
priv = new AddressTablePriv(this);
priv->refreshAddressTable(parent->wallet(), pk_hash_only);
}
Expand Down Expand Up @@ -208,6 +212,8 @@ QVariant AddressTableModel::data(const QModelIndex &index, int role) const
}
case Address:
return rec->address;
case Type:
return rec->addressType;
} // no default case, so the compiler can warn about missing cases
assert(false);
} else if (role == Qt::FontRole) {
Expand All @@ -216,6 +222,8 @@ QVariant AddressTableModel::data(const QModelIndex &index, int role) const
return QFont();
case Address:
return GUIUtil::fixedPitchFont();
case Type:
return QFont();
} // no default case, so the compiler can warn about missing cases
assert(false);
} else if (role == TypeRole) {
Expand Down Expand Up @@ -332,11 +340,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, const OutputType &addressType)
{
// Update address book model from Bitcoin core
priv->updateEntry(address, label, isMine, purpose, status);
priv->updateEntry(address, label, isMine, purpose, status, addressType);
}

QString AddressTableModel::addRow(const QString &type, const QString &label, const QString &address, const OutputType address_type)
Expand Down
6 changes: 3 additions & 3 deletions src/qt/addresstablemodel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ class AddressTableModel : public QAbstractTableModel

enum ColumnIndex {
Label = 0, /**< User specified label */
Address = 1 /**< Bitcoin address */
Type = 1, /**< Address type */
Address = 2 /**< Bitcoin address */
};

enum RoleIndex {
Expand Down Expand Up @@ -104,8 +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, const OutputType &address_type);
friend class AddressTablePriv;
};

Expand Down
10 changes: 10 additions & 0 deletions src/qt/forms/addressbookpage.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,16 @@
</property>
</widget>
</item>
<item>
<widget class="QLabel" name="labelAddressType">
<property name="text">
<string>Show address type:</string>
</property>
</widget>
</item>
<item>
<widget class="QComboBox" name="addressType"/>
</item>
<item>
<spacer name="horizontalSpacer">
<property name="orientation">
Expand Down
Loading