aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/interfaces/wallet.h13
-rw-r--r--src/qt/addresstablemodel.cpp42
-rw-r--r--src/qt/addresstablemodel.h11
-rw-r--r--src/qt/editaddressdialog.cpp6
-rw-r--r--src/qt/test/addressbooktests.cpp4
-rw-r--r--src/qt/test/wallettests.cpp2
-rw-r--r--src/qt/walletmodel.cpp13
-rw-r--r--src/qt/walletmodel.h2
-rw-r--r--src/wallet/interfaces.cpp21
-rw-r--r--src/wallet/rpc/addresses.cpp20
-rw-r--r--src/wallet/rpc/backup.cpp4
-rw-r--r--src/wallet/rpc/transactions.cpp4
-rw-r--r--src/wallet/wallet.cpp67
-rw-r--r--src/wallet/wallet.h75
-rw-r--r--src/wallet/walletdb.cpp8
-rwxr-xr-xtest/functional/wallet_labels.py4
16 files changed, 177 insertions, 119 deletions
diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
index b37763e686..8c31112fc9 100644
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -35,6 +35,7 @@ struct bilingual_str;
namespace wallet {
class CCoinControl;
class CWallet;
+enum class AddressPurpose;
enum isminetype : unsigned int;
struct CRecipient;
struct WalletContext;
@@ -103,7 +104,7 @@ public:
virtual bool haveWatchOnly() = 0;
//! Add or update address.
- virtual bool setAddressBook(const CTxDestination& dest, const std::string& name, const std::string& purpose) = 0;
+ virtual bool setAddressBook(const CTxDestination& dest, const std::string& name, const std::optional<wallet::AddressPurpose>& purpose) = 0;
// Remove address.
virtual bool delAddressBook(const CTxDestination& dest) = 0;
@@ -112,7 +113,7 @@ public:
virtual bool getAddress(const CTxDestination& dest,
std::string* name,
wallet::isminetype* is_mine,
- std::string* purpose) = 0;
+ wallet::AddressPurpose* purpose) = 0;
//! Get wallet address list.
virtual std::vector<WalletAddress> getAddresses() const = 0;
@@ -293,7 +294,7 @@ public:
using AddressBookChangedFn = std::function<void(const CTxDestination& address,
const std::string& label,
bool is_mine,
- const std::string& purpose,
+ wallet::AddressPurpose purpose,
ChangeType status)>;
virtual std::unique_ptr<Handler> handleAddressBookChanged(AddressBookChangedFn fn) = 0;
@@ -352,11 +353,11 @@ struct WalletAddress
{
CTxDestination dest;
wallet::isminetype is_mine;
+ wallet::AddressPurpose purpose;
std::string name;
- std::string purpose;
- WalletAddress(CTxDestination dest, wallet::isminetype is_mine, std::string name, std::string purpose)
- : dest(std::move(dest)), is_mine(is_mine), name(std::move(name)), purpose(std::move(purpose))
+ 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))
{
}
};
diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp
index e402c51ac4..0d0f1a4d15 100644
--- a/src/qt/addresstablemodel.cpp
+++ b/src/qt/addresstablemodel.cpp
@@ -8,6 +8,7 @@
#include <qt/walletmodel.h>
#include <key_io.h>
+#include <wallet/types.h>
#include <wallet/wallet.h>
#include <algorithm>
@@ -52,17 +53,16 @@ struct AddressTableEntryLessThan
};
/* Determine address type from address purpose */
-static AddressTableEntry::Type translateTransactionType(const QString &strPurpose, bool isMine)
+static AddressTableEntry::Type translateTransactionType(wallet::AddressPurpose purpose, bool isMine)
{
- AddressTableEntry::Type addressType = AddressTableEntry::Hidden;
// "refund" addresses aren't shown, and change addresses aren't returned by getAddresses at all.
- if (strPurpose == "send")
- addressType = AddressTableEntry::Sending;
- else if (strPurpose == "receive")
- addressType = AddressTableEntry::Receiving;
- else if (strPurpose == "unknown" || strPurpose == "") // if purpose not set, guess
- addressType = (isMine ? AddressTableEntry::Receiving : AddressTableEntry::Sending);
- return addressType;
+ switch (purpose) {
+ case wallet::AddressPurpose::SEND: return AddressTableEntry::Sending;
+ case wallet::AddressPurpose::RECEIVE: return AddressTableEntry::Receiving;
+ case wallet::AddressPurpose::REFUND: return AddressTableEntry::Hidden;
+ // No default case to allow for compiler to warn
+ }
+ assert(false);
}
// Private implementation
@@ -85,7 +85,7 @@ public:
continue;
}
AddressTableEntry::Type addressType = translateTransactionType(
- QString::fromStdString(address.purpose), address.is_mine);
+ address.purpose, address.is_mine);
cachedAddressTable.append(AddressTableEntry(addressType,
QString::fromStdString(address.name),
QString::fromStdString(EncodeDestination(address.dest))));
@@ -97,7 +97,7 @@ public:
std::sort(cachedAddressTable.begin(), cachedAddressTable.end(), AddressTableEntryLessThan());
}
- void updateEntry(const QString &address, const QString &label, bool isMine, const QString &purpose, int status)
+ void updateEntry(const QString &address, const QString &label, bool isMine, wallet::AddressPurpose purpose, int status)
{
// Find address / label in model
QList<AddressTableEntry>::iterator lower = std::lower_bound(
@@ -239,7 +239,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
if(!index.isValid())
return false;
AddressTableEntry *rec = static_cast<AddressTableEntry*>(index.internalPointer());
- std::string strPurpose = (rec->type == AddressTableEntry::Sending ? "send" : "receive");
+ wallet::AddressPurpose purpose = rec->type == AddressTableEntry::Sending ? wallet::AddressPurpose::SEND : wallet::AddressPurpose::RECEIVE;
editStatus = OK;
if(role == Qt::EditRole)
@@ -253,7 +253,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
editStatus = NO_CHANGES;
return false;
}
- walletModel->wallet().setAddressBook(curAddress, value.toString().toStdString(), strPurpose);
+ walletModel->wallet().setAddressBook(curAddress, value.toString().toStdString(), purpose);
} else if(index.column() == Address) {
CTxDestination newAddress = DecodeDestination(value.toString().toStdString());
// Refuse to set invalid address, set error status and return false
@@ -282,7 +282,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value,
// Remove old entry
walletModel->wallet().delAddressBook(curAddress);
// Add new entry with new address
- walletModel->wallet().setAddressBook(newAddress, value.toString().toStdString(), strPurpose);
+ walletModel->wallet().setAddressBook(newAddress, value.toString().toStdString(), purpose);
}
}
return true;
@@ -334,7 +334,7 @@ QModelIndex AddressTableModel::index(int row, int column, const QModelIndex &par
}
void AddressTableModel::updateEntry(const QString &address,
- const QString &label, bool isMine, const QString &purpose, int status)
+ const QString &label, bool isMine, wallet::AddressPurpose purpose, int status)
{
// Update address book model from Bitcoin core
priv->updateEntry(address, label, isMine, purpose, status);
@@ -365,7 +365,7 @@ QString AddressTableModel::addRow(const QString &type, const QString &label, con
}
// Add entry
- walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel, "send");
+ walletModel->wallet().setAddressBook(DecodeDestination(strAddress), strLabel, wallet::AddressPurpose::SEND);
}
else if(type == Receive)
{
@@ -416,18 +416,18 @@ QString AddressTableModel::labelForAddress(const QString &address) const
return QString();
}
-QString AddressTableModel::purposeForAddress(const QString &address) const
+std::optional<wallet::AddressPurpose> AddressTableModel::purposeForAddress(const QString &address) const
{
- std::string purpose;
+ wallet::AddressPurpose purpose;
if (getAddressData(address, /* name= */ nullptr, &purpose)) {
- return QString::fromStdString(purpose);
+ return purpose;
}
- return QString();
+ return std::nullopt;
}
bool AddressTableModel::getAddressData(const QString &address,
std::string* name,
- std::string* purpose) const {
+ wallet::AddressPurpose* purpose) const {
CTxDestination destination = DecodeDestination(address.toStdString());
return walletModel->wallet().getAddress(destination, name, /* is_mine= */ nullptr, purpose);
}
diff --git a/src/qt/addresstablemodel.h b/src/qt/addresstablemodel.h
index 6cc14654ef..599aa89cad 100644
--- a/src/qt/addresstablemodel.h
+++ b/src/qt/addresstablemodel.h
@@ -5,6 +5,8 @@
#ifndef BITCOIN_QT_ADDRESSTABLEMODEL_H
#define BITCOIN_QT_ADDRESSTABLEMODEL_H
+#include <optional>
+
#include <QAbstractTableModel>
#include <QStringList>
@@ -16,6 +18,9 @@ class WalletModel;
namespace interfaces {
class Wallet;
}
+namespace wallet {
+enum class AddressPurpose;
+} // namespace wallet
/**
Qt model of the address book in the core. This allows views to access and modify the address book.
@@ -71,7 +76,7 @@ public:
QString labelForAddress(const QString &address) const;
/** Look up purpose for address in address book, if not found return empty string. */
- QString purposeForAddress(const QString &address) const;
+ std::optional<wallet::AddressPurpose> purposeForAddress(const QString &address) const;
/* Look up row index of an address in the model.
Return -1 if not found.
@@ -89,7 +94,7 @@ private:
EditStatus editStatus = OK;
/** Look up address book data given an address string. */
- bool getAddressData(const QString &address, std::string* name, std::string* purpose) const;
+ bool getAddressData(const QString &address, std::string* name, wallet::AddressPurpose* purpose) const;
/** Notify listeners that data changed. */
void emitDataChanged(int index);
@@ -97,7 +102,7 @@ private:
public Q_SLOTS:
/* Update address list from core.
*/
- void updateEntry(const QString &address, const QString &label, bool isMine, const QString &purpose, int status);
+ void updateEntry(const QString &address, const QString &label, bool isMine, wallet::AddressPurpose purpose, int status);
friend class AddressTablePriv;
};
diff --git a/src/qt/editaddressdialog.cpp b/src/qt/editaddressdialog.cpp
index 9b3319415d..092a89fa11 100644
--- a/src/qt/editaddressdialog.cpp
+++ b/src/qt/editaddressdialog.cpp
@@ -8,6 +8,8 @@
#include <qt/addresstablemodel.h>
#include <qt/guiutil.h>
+#include <wallet/types.h>
+
#include <QDataWidgetMapper>
#include <QMessageBox>
@@ -137,9 +139,9 @@ QString EditAddressDialog::getDuplicateAddressWarning() const
{
QString dup_address = ui->addressEdit->text();
QString existing_label = model->labelForAddress(dup_address);
- QString existing_purpose = model->purposeForAddress(dup_address);
+ auto existing_purpose = model->purposeForAddress(dup_address);
- if (existing_purpose == "receive" &&
+ if (existing_purpose == wallet::AddressPurpose::RECEIVE &&
(mode == NewSendingAddress || mode == EditSendingAddress)) {
return tr(
"Address \"%1\" already exists as a receiving address with label "
diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp
index d005e08d14..5706964cc9 100644
--- a/src/qt/test/addressbooktests.cpp
+++ b/src/qt/test/addressbooktests.cpp
@@ -113,8 +113,8 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
{
LOCK(wallet->cs_wallet);
- wallet->SetAddressBook(r_key_dest, r_label.toStdString(), "receive");
- wallet->SetAddressBook(s_key_dest, s_label.toStdString(), "send");
+ wallet->SetAddressBook(r_key_dest, r_label.toStdString(), wallet::AddressPurpose::RECEIVE);
+ wallet->SetAddressBook(s_key_dest, s_label.toStdString(), wallet::AddressPurpose::SEND);
}
auto check_addbook_size = [&wallet](int expected_size) {
diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index eb7bf33a32..072718fe15 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -221,7 +221,7 @@ std::shared_ptr<CWallet> SetupDescriptorsWallet(interfaces::Node& node, TestChai
WalletDescriptor w_desc(std::move(desc), 0, 0, 1, 1);
if (!wallet->AddWalletDescriptor(w_desc, provider, "", false)) assert(false);
CTxDestination dest = GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type);
- wallet->SetAddressBook(dest, "", "receive");
+ wallet->SetAddressBook(dest, "", wallet::AddressPurpose::RECEIVE);
wallet->SetLastBlockProcessed(105, WITH_LOCK(node.context()->chainman->GetMutex(), return node.context()->chainman->ActiveChain().Tip()->GetBlockHash()));
SyncUpWallet(wallet, node);
wallet->SetBroadcastTransactions(true);
diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp
index 565b732bf0..fdd96c664a 100644
--- a/src/qt/walletmodel.cpp
+++ b/src/qt/walletmodel.cpp
@@ -138,7 +138,7 @@ void WalletModel::updateTransaction()
}
void WalletModel::updateAddressBook(const QString &address, const QString &label,
- bool isMine, const QString &purpose, int status)
+ bool isMine, wallet::AddressPurpose purpose, int status)
{
if(addressTableModel)
addressTableModel->updateEntry(address, label, isMine, purpose, status);
@@ -280,11 +280,11 @@ void WalletModel::sendCoins(WalletModelTransaction& transaction)
if (!m_wallet->getAddress(
dest, &name, /* is_mine= */ nullptr, /* purpose= */ nullptr))
{
- m_wallet->setAddressBook(dest, strLabel, "send");
+ m_wallet->setAddressBook(dest, strLabel, wallet::AddressPurpose::SEND);
}
else if (name != strLabel)
{
- m_wallet->setAddressBook(dest, strLabel, ""); // "" means don't change purpose
+ m_wallet->setAddressBook(dest, strLabel, {}); // {} means don't change purpose
}
}
}
@@ -377,18 +377,17 @@ static void NotifyKeyStoreStatusChanged(WalletModel *walletmodel)
static void NotifyAddressBookChanged(WalletModel *walletmodel,
const CTxDestination &address, const std::string &label, bool isMine,
- const std::string &purpose, ChangeType status)
+ wallet::AddressPurpose purpose, ChangeType status)
{
QString strAddress = QString::fromStdString(EncodeDestination(address));
QString strLabel = QString::fromStdString(label);
- QString strPurpose = QString::fromStdString(purpose);
- qDebug() << "NotifyAddressBookChanged: " + strAddress + " " + strLabel + " isMine=" + QString::number(isMine) + " purpose=" + strPurpose + " status=" + QString::number(status);
+ qDebug() << "NotifyAddressBookChanged: " + strAddress + " " + strLabel + " isMine=" + QString::number(isMine) + " purpose=" + QString::number(static_cast<uint8_t>(purpose)) + " status=" + QString::number(status);
bool invoked = QMetaObject::invokeMethod(walletmodel, "updateAddressBook",
Q_ARG(QString, strAddress),
Q_ARG(QString, strLabel),
Q_ARG(bool, isMine),
- Q_ARG(QString, strPurpose),
+ Q_ARG(wallet::AddressPurpose, purpose),
Q_ARG(int, status));
assert(invoked);
}
diff --git a/src/qt/walletmodel.h b/src/qt/walletmodel.h
index 17a39349f3..4f75d41404 100644
--- a/src/qt/walletmodel.h
+++ b/src/qt/walletmodel.h
@@ -236,7 +236,7 @@ public Q_SLOTS:
/* New transaction, or transaction changed status */
void updateTransaction();
/* New, updated or removed address book entry */
- void updateAddressBook(const QString &address, const QString &label, bool isMine, const QString &purpose, int status);
+ void updateAddressBook(const QString &address, const QString &label, bool isMine, wallet::AddressPurpose purpose, int status);
/* Watch-only added */
void updateWatchOnlyFlag(bool fHaveWatchonly);
/* Current, immature or unconfirmed balance might have changed - emit 'balanceChanged' if so */
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index eeac147d8b..086f6d9de8 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -179,7 +179,7 @@ public:
}
return false;
};
- bool setAddressBook(const CTxDestination& dest, const std::string& name, const std::string& purpose) override
+ bool setAddressBook(const CTxDestination& dest, const std::string& name, const std::optional<AddressPurpose>& purpose) override
{
return m_wallet->SetAddressBook(dest, name, purpose);
}
@@ -190,7 +190,7 @@ public:
bool getAddress(const CTxDestination& dest,
std::string* name,
isminetype* is_mine,
- std::string* purpose) override
+ AddressPurpose* purpose) override
{
LOCK(m_wallet->cs_wallet);
const auto& entry = m_wallet->FindAddressBookEntry(dest, /*allow_change=*/false);
@@ -198,11 +198,16 @@ public:
if (name) {
*name = entry->GetLabel();
}
+ std::optional<isminetype> dest_is_mine;
+ if (is_mine || purpose) {
+ dest_is_mine = m_wallet->IsMine(dest);
+ }
if (is_mine) {
- *is_mine = m_wallet->IsMine(dest);
+ *is_mine = *dest_is_mine;
}
if (purpose) {
- *purpose = entry->purpose;
+ // In very old wallets, address purpose may not be recorded so we derive it from IsMine
+ *purpose = entry->purpose.value_or(*dest_is_mine ? AddressPurpose::RECEIVE : AddressPurpose::SEND);
}
return true;
}
@@ -210,9 +215,11 @@ public:
{
LOCK(m_wallet->cs_wallet);
std::vector<WalletAddress> result;
- m_wallet->ForEachAddrBookEntry([&](const CTxDestination& dest, const std::string& label, const std::string& purpose, bool is_change) EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet) {
+ m_wallet->ForEachAddrBookEntry([&](const CTxDestination& dest, const std::string& label, bool is_change, const std::optional<AddressPurpose>& purpose) EXCLUSIVE_LOCKS_REQUIRED(m_wallet->cs_wallet) {
if (is_change) return;
- result.emplace_back(dest, m_wallet->IsMine(dest), label, purpose);
+ isminetype is_mine = m_wallet->IsMine(dest);
+ // In very old wallets, address purpose may not be recorded so we derive it from IsMine
+ result.emplace_back(dest, is_mine, purpose.value_or(is_mine ? AddressPurpose::RECEIVE : AddressPurpose::SEND), label);
});
return result;
}
@@ -519,7 +526,7 @@ public:
{
return MakeSignalHandler(m_wallet->NotifyAddressBookChanged.connect(
[fn](const CTxDestination& address, const std::string& label, bool is_mine,
- const std::string& purpose, ChangeType status) { fn(address, label, is_mine, purpose, status); }));
+ AddressPurpose purpose, ChangeType status) { fn(address, label, is_mine, purpose, status); }));
}
std::unique_ptr<Handler> handleTransactionChanged(TransactionChangedFn fn) override
{
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index da63d45d11..838d66c1e1 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -141,9 +141,9 @@ RPCHelpMan setlabel()
const std::string label{LabelFromValue(request.params[1])};
if (pwallet->IsMine(dest)) {
- pwallet->SetAddressBook(dest, label, "receive");
+ pwallet->SetAddressBook(dest, label, AddressPurpose::RECEIVE);
} else {
- pwallet->SetAddressBook(dest, label, "send");
+ pwallet->SetAddressBook(dest, label, AddressPurpose::SEND);
}
return UniValue::VNULL;
@@ -285,7 +285,7 @@ RPCHelpMan addmultisigaddress()
// Construct using pay-to-script-hash:
CScript inner;
CTxDestination dest = AddAndGetMultisigDestination(required, pubkeys, output_type, spk_man, inner);
- pwallet->SetAddressBook(dest, label, "send");
+ pwallet->SetAddressBook(dest, label, AddressPurpose::SEND);
// Make the descriptor
std::unique_ptr<Descriptor> descriptor = InferDescriptor(GetScriptForDestination(dest), spk_man);
@@ -663,7 +663,7 @@ RPCHelpMan getaddressesbylabel()
// Find all addresses that have the given label
UniValue ret(UniValue::VOBJ);
std::set<std::string> addresses;
- pwallet->ForEachAddrBookEntry([&](const CTxDestination& _dest, const std::string& _label, const std::string& _purpose, bool _is_change) {
+ pwallet->ForEachAddrBookEntry([&](const CTxDestination& _dest, const std::string& _label, bool _is_change, const std::optional<AddressPurpose>& _purpose) {
if (_is_change) return;
if (_label == label) {
std::string address = EncodeDestination(_dest);
@@ -677,7 +677,7 @@ RPCHelpMan getaddressesbylabel()
// std::set in O(log(N))), UniValue::__pushKV is used instead,
// which currently is O(1).
UniValue value(UniValue::VOBJ);
- value.pushKV("purpose", _purpose);
+ value.pushKV("purpose", _purpose ? PurposeToString(*_purpose) : "unknown");
ret.__pushKV(address, value);
}
});
@@ -721,9 +721,15 @@ RPCHelpMan listlabels()
LOCK(pwallet->cs_wallet);
- std::string purpose;
+ std::optional<AddressPurpose> purpose;
if (!request.params[0].isNull()) {
- purpose = request.params[0].get_str();
+ std::string purpose_str = request.params[0].get_str();
+ if (!purpose_str.empty()) {
+ purpose = PurposeFromString(purpose_str);
+ if (!purpose) {
+ throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid 'purpose' argument, must be a known purpose string, typically 'send', or 'receive'.");
+ }
+ }
}
// Add to a set to sort by label name, then insert into Univalue array
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index f4ea66c833..31a8954259 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -188,7 +188,7 @@ RPCHelpMan importprivkey()
// label was passed.
for (const auto& dest : GetAllDestinationsForKey(pubkey)) {
if (!request.params[1].isNull() || !pwallet->FindAddressBookEntry(dest)) {
- pwallet->SetAddressBook(dest, strLabel, "receive");
+ pwallet->SetAddressBook(dest, strLabel, AddressPurpose::RECEIVE);
}
}
@@ -608,7 +608,7 @@ RPCHelpMan importwallet()
}
if (has_label)
- pwallet->SetAddressBook(PKHash(keyid), label, "receive");
+ pwallet->SetAddressBook(PKHash(keyid), label, AddressPurpose::RECEIVE);
progress++;
}
for (const auto& script_pair : scripts) {
diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
index 3bfe296d90..5c9de7de47 100644
--- a/src/wallet/rpc/transactions.cpp
+++ b/src/wallet/rpc/transactions.cpp
@@ -134,7 +134,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
UniValue ret(UniValue::VARR);
std::map<std::string, tallyitem> label_tally;
- const auto& func = [&](const CTxDestination& address, const std::string& label, const std::string& purpose, bool is_change) {
+ const auto& func = [&](const CTxDestination& address, const std::string& label, bool is_change, const std::optional<AddressPurpose>& purpose) {
if (is_change) return; // no change addresses
auto it = mapTally.find(address);
@@ -175,7 +175,7 @@ static UniValue ListReceived(const CWallet& wallet, const UniValue& params, cons
if (filtered_address) {
const auto& entry = wallet.FindAddressBookEntry(*filtered_address, /*allow_change=*/false);
- if (entry) func(*filtered_address, entry->GetLabel(), entry->purpose, /*is_change=*/false);
+ if (entry) func(*filtered_address, entry->GetLabel(), entry->IsChange(), entry->purpose);
} else {
// No filtered addr, walk-through the addressbook entry
wallet.ForEachAddrBookEntry(func);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 50405b78fe..6df0d12df6 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1229,7 +1229,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS
// (e.g. it wasn't generated on this node or we're restoring from backup)
// add it to the address book for proper transaction accounting
if (!*dest.internal && !FindAddressBookEntry(dest.dest, /* allow_change= */ false)) {
- SetAddressBook(dest.dest, "", "receive");
+ SetAddressBook(dest.dest, "", AddressPurpose::RECEIVE);
}
}
}
@@ -1777,7 +1777,7 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set<CScri
CTxDestination dest;
ExtractDestination(script, dest);
if (IsValidDestination(dest)) {
- SetAddressBookWithDB(batch, dest, label, "receive");
+ SetAddressBookWithDB(batch, dest, label, AddressPurpose::RECEIVE);
}
}
}
@@ -2400,35 +2400,40 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256
return DBErrors::LOAD_OK;
}
-bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::string& strPurpose)
+bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::optional<AddressPurpose>& new_purpose)
{
bool fUpdated = false;
bool is_mine;
+ std::optional<AddressPurpose> purpose;
{
LOCK(cs_wallet);
std::map<CTxDestination, CAddressBookData>::iterator mi = m_address_book.find(address);
fUpdated = (mi != m_address_book.end() && !mi->second.IsChange());
m_address_book[address].SetLabel(strName);
- if (!strPurpose.empty()) /* update purpose only if requested */
- m_address_book[address].purpose = strPurpose;
is_mine = IsMine(address) != ISMINE_NO;
+ if (new_purpose) { /* update purpose only if requested */
+ purpose = m_address_book[address].purpose = new_purpose;
+ } else {
+ purpose = m_address_book[address].purpose;
+ }
}
+ // In very old wallets, address purpose may not be recorded so we derive it from IsMine
NotifyAddressBookChanged(address, strName, is_mine,
- strPurpose, (fUpdated ? CT_UPDATED : CT_NEW));
- if (!strPurpose.empty() && !batch.WritePurpose(EncodeDestination(address), strPurpose))
+ purpose.value_or(is_mine ? AddressPurpose::RECEIVE : AddressPurpose::SEND),
+ (fUpdated ? CT_UPDATED : CT_NEW));
+ if (new_purpose && !batch.WritePurpose(EncodeDestination(address), PurposeToString(*new_purpose)))
return false;
return batch.WriteName(EncodeDestination(address), strName);
}
-bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& strPurpose)
+bool CWallet::SetAddressBook(const CTxDestination& address, const std::string& strName, const std::optional<AddressPurpose>& purpose)
{
WalletBatch batch(GetDatabase());
- return SetAddressBookWithDB(batch, address, strName, strPurpose);
+ return SetAddressBookWithDB(batch, address, strName, purpose);
}
bool CWallet::DelAddressBook(const CTxDestination& address)
{
- bool is_mine;
WalletBatch batch(GetDatabase());
{
LOCK(cs_wallet);
@@ -2446,10 +2451,9 @@ bool CWallet::DelAddressBook(const CTxDestination& address)
batch.EraseDestData(strAddress, item.first);
}
m_address_book.erase(address);
- is_mine = IsMine(address) != ISMINE_NO;
}
- NotifyAddressBookChanged(address, "", is_mine, "", CT_DELETED);
+ NotifyAddressBookChanged(address, "", /*is_mine=*/false, AddressPurpose::SEND, CT_DELETED);
batch.ErasePurpose(EncodeDestination(address));
return batch.EraseName(EncodeDestination(address));
@@ -2503,7 +2507,7 @@ util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, c
auto op_dest = spk_man->GetNewDestination(type);
if (op_dest) {
- SetAddressBook(*op_dest, label, "receive");
+ SetAddressBook(*op_dest, label, AddressPurpose::RECEIVE);
}
return op_dest;
@@ -2553,7 +2557,7 @@ void CWallet::ForEachAddrBookEntry(const ListAddrBookFunc& func) const
AssertLockHeld(cs_wallet);
for (const std::pair<const CTxDestination, CAddressBookData>& item : m_address_book) {
const auto& entry = item.second;
- func(item.first, entry.GetLabel(), entry.purpose, entry.IsChange());
+ func(item.first, entry.GetLabel(), entry.IsChange(), entry.purpose);
}
}
@@ -2562,7 +2566,7 @@ std::vector<CTxDestination> CWallet::ListAddrBookAddresses(const std::optional<A
AssertLockHeld(cs_wallet);
std::vector<CTxDestination> result;
AddrBookFilter filter = _filter ? *_filter : AddrBookFilter();
- ForEachAddrBookEntry([&result, &filter](const CTxDestination& dest, const std::string& label, const std::string& purpose, bool is_change) {
+ ForEachAddrBookEntry([&result, &filter](const CTxDestination& dest, const std::string& label, bool is_change, const std::optional<AddressPurpose>& purpose) {
// Filter by change
if (filter.ignore_change && is_change) return;
// Filter by label
@@ -2573,14 +2577,14 @@ std::vector<CTxDestination> CWallet::ListAddrBookAddresses(const std::optional<A
return result;
}
-std::set<std::string> CWallet::ListAddrBookLabels(const std::string& purpose) const
+std::set<std::string> CWallet::ListAddrBookLabels(const std::optional<AddressPurpose> purpose) const
{
AssertLockHeld(cs_wallet);
std::set<std::string> label_set;
ForEachAddrBookEntry([&](const CTxDestination& _dest, const std::string& _label,
- const std::string& _purpose, bool _is_change) {
+ bool _is_change, const std::optional<AddressPurpose>& _purpose) {
if (_is_change) return;
- if (purpose.empty() || _purpose == purpose) {
+ if (!purpose || purpose == _purpose) {
label_set.insert(_label);
}
});
@@ -3792,7 +3796,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat
for (const auto& script : script_pub_keys) {
CTxDestination dest;
if (ExtractDestination(script, dest)) {
- SetAddressBook(dest, label, "receive");
+ SetAddressBook(dest, label, AddressPurpose::RECEIVE);
}
}
}
@@ -3978,7 +3982,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
std::vector<CTxDestination> dests_to_delete;
for (const auto& addr_pair : m_address_book) {
// Labels applied to receiving addresses should go based on IsMine
- if (addr_pair.second.purpose == "receive") {
+ if (addr_pair.second.purpose == AddressPurpose::RECEIVE) {
if (!IsMine(addr_pair.first)) {
// Check the address book data is the watchonly wallet's
if (data.watchonly_wallet) {
@@ -3986,10 +3990,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
if (data.watchonly_wallet->IsMine(addr_pair.first)) {
// Add to the watchonly. Preserve the labels, purpose, and change-ness
std::string label = addr_pair.second.GetLabel();
- std::string purpose = addr_pair.second.purpose;
- if (!purpose.empty()) {
- data.watchonly_wallet->m_address_book[addr_pair.first].purpose = purpose;
- }
+ data.watchonly_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
if (!addr_pair.second.IsChange()) {
data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label);
}
@@ -4002,10 +4003,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
if (data.solvable_wallet->IsMine(addr_pair.first)) {
// Add to the solvable. Preserve the labels, purpose, and change-ness
std::string label = addr_pair.second.GetLabel();
- std::string purpose = addr_pair.second.purpose;
- if (!purpose.empty()) {
- data.solvable_wallet->m_address_book[addr_pair.first].purpose = purpose;
- }
+ data.solvable_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
if (!addr_pair.second.IsChange()) {
data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label);
}
@@ -4023,10 +4021,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
LOCK(data.watchonly_wallet->cs_wallet);
// Add to the watchonly. Preserve the labels, purpose, and change-ness
std::string label = addr_pair.second.GetLabel();
- std::string purpose = addr_pair.second.purpose;
- if (!purpose.empty()) {
- data.watchonly_wallet->m_address_book[addr_pair.first].purpose = purpose;
- }
+ data.watchonly_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
if (!addr_pair.second.IsChange()) {
data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label);
}
@@ -4036,10 +4031,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
LOCK(data.solvable_wallet->cs_wallet);
// Add to the solvable. Preserve the labels, purpose, and change-ness
std::string label = addr_pair.second.GetLabel();
- std::string purpose = addr_pair.second.purpose;
- if (!purpose.empty()) {
- data.solvable_wallet->m_address_book[addr_pair.first].purpose = purpose;
- }
+ data.solvable_wallet->m_address_book[addr_pair.first].purpose = addr_pair.second.purpose;
if (!addr_pair.second.IsChange()) {
data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label);
}
@@ -4054,10 +4046,9 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
WalletBatch batch{wallet.GetDatabase()};
for (const auto& [destination, addr_book_data] : wallet.m_address_book) {
auto address{EncodeDestination(destination)};
- auto purpose{addr_book_data.purpose};
auto label{addr_book_data.GetLabel()};
// don't bother writing default values (unknown purpose, empty label)
- if (purpose != "unknown") batch.WritePurpose(address, purpose);
+ if (addr_book_data.purpose) batch.WritePurpose(address, PurposeToString(*addr_book_data.purpose));
if (!label.empty()) batch.WriteName(address, label);
}
};
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 23c2de8191..586f215499 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -9,6 +9,7 @@
#include <consensus/amount.h>
#include <interfaces/chain.h>
#include <interfaces/handler.h>
+#include <interfaces/wallet.h>
#include <logging.h>
#include <outputtype.h>
#include <policy/feerate.h>
@@ -200,28 +201,64 @@ public:
void KeepDestination();
};
-/** Address book data */
-class CAddressBookData
+/**
+ * Address book data.
+ */
+struct CAddressBookData
{
-private:
- bool m_change{true};
- std::string m_label;
-public:
- std::string purpose;
+ /**
+ * Address label which is always nullopt for change addresses. For sending
+ * and receiving addresses, it will be set to an arbitrary label string
+ * provided by the user, or to "", which is the default label. The presence
+ * or absence of a label is used to distinguish change addresses from
+ * non-change addresses by wallet transaction listing and fee bumping code.
+ */
+ std::optional<std::string> label;
- CAddressBookData() : purpose("unknown") {}
+ /**
+ * Address purpose which was originally recorded for payment protocol
+ * support but now serves as a cached IsMine value. Wallet code should
+ * not rely on this field being set.
+ */
+ std::optional<AddressPurpose> purpose;
+ /**
+ * Additional address metadata map that can currently hold two types of keys:
+ *
+ * "used" keys with values always set to "1" or "p" if present. This is set on
+ * IsMine addresses that have already been spent from if the
+ * avoid_reuse option is enabled
+ *
+ * "rr##" keys where ## is a decimal number, with serialized
+ * RecentRequestEntry objects as values
+ */
typedef std::map<std::string, std::string> StringMap;
StringMap destdata;
- bool IsChange() const { return m_change; }
- const std::string& GetLabel() const { return m_label; }
- void SetLabel(const std::string& label) {
- m_change = false;
- m_label = label;
- }
+ /** Accessor methods. */
+ bool IsChange() const { return !label.has_value(); }
+ std::string GetLabel() const { return label ? *label : std::string{}; }
+ void SetLabel(std::string name) { label = std::move(name); }
};
+inline std::string PurposeToString(AddressPurpose p)
+{
+ switch(p) {
+ case AddressPurpose::RECEIVE: return "receive";
+ case AddressPurpose::SEND: return "send";
+ case AddressPurpose::REFUND: return "refund";
+ } // no default case so the compiler will warn when a new enum as added
+ assert(false);
+}
+
+inline std::optional<AddressPurpose> PurposeFromString(std::string_view s)
+{
+ if (s == "receive") return AddressPurpose::RECEIVE;
+ else if (s == "send") return AddressPurpose::SEND;
+ else if (s == "refund") return AddressPurpose::REFUND;
+ return {};
+}
+
struct CRecipient
{
CScript scriptPubKey;
@@ -300,7 +337,7 @@ private:
/** WalletFlags set on this wallet. */
std::atomic<uint64_t> m_wallet_flags{0};
- bool SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::string& strPurpose);
+ bool SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::optional<AddressPurpose>& strPurpose);
//! Unsets a wallet flag and saves it to disk
void UnsetWalletFlagWithDB(WalletBatch& batch, uint64_t flag);
@@ -664,13 +701,13 @@ public:
/**
* Retrieve all the known labels in the address book
*/
- std::set<std::string> ListAddrBookLabels(const std::string& purpose) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ std::set<std::string> ListAddrBookLabels(const std::optional<AddressPurpose> purpose) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/**
* Walk-through the address book entries.
* Stops when the provided 'ListAddrBookFunc' returns false.
*/
- using ListAddrBookFunc = std::function<void(const CTxDestination& dest, const std::string& label, const std::string& purpose, bool is_change)>;
+ using ListAddrBookFunc = std::function<void(const CTxDestination& dest, const std::string& label, bool is_change, const std::optional<AddressPurpose> purpose)>;
void ForEachAddrBookEntry(const ListAddrBookFunc& func) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/**
@@ -700,7 +737,7 @@ public:
DBErrors LoadWallet();
DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
- bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::string& purpose);
+ bool SetAddressBook(const CTxDestination& address, const std::string& strName, const std::optional<AddressPurpose>& purpose);
bool DelAddressBook(const CTxDestination& address);
@@ -739,7 +776,7 @@ public:
*/
boost::signals2::signal<void(const CTxDestination& address,
const std::string& label, bool isMine,
- const std::string& purpose, ChangeType status)>
+ AddressPurpose purpose, ChangeType status)>
NotifyAddressBookChanged;
/**
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index 3c0ed21b66..c8e8ce4614 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -347,7 +347,13 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue,
} else if (strType == DBKeys::PURPOSE) {
std::string strAddress;
ssKey >> strAddress;
- ssValue >> pwallet->m_address_book[DecodeDestination(strAddress)].purpose;
+ std::string purpose_str;
+ ssValue >> purpose_str;
+ std::optional<AddressPurpose> purpose{PurposeFromString(purpose_str)};
+ if (!purpose) {
+ pwallet->WalletLogPrintf("Warning: nonstandard purpose string '%s' for address '%s'\n", purpose_str, strAddress);
+ }
+ pwallet->m_address_book[DecodeDestination(strAddress)].purpose = purpose;
} else if (strType == DBKeys::TX) {
uint256 hash;
ssKey >> hash;
diff --git a/test/functional/wallet_labels.py b/test/functional/wallet_labels.py
index a39700f73a..f074339a2b 100755
--- a/test/functional/wallet_labels.py
+++ b/test/functional/wallet_labels.py
@@ -71,6 +71,10 @@ class WalletLabelsTest(BitcoinTestFramework):
node = self.nodes[0]
assert_equal(len(node.listunspent()), 0)
+ self.log.info("Checking listlabels' invalid parameters")
+ assert_raises_rpc_error(-8, "Invalid 'purpose' argument, must be a known purpose string, typically 'send', or 'receive'.", node.listlabels, "notavalidpurpose")
+ assert_raises_rpc_error(-8, "Invalid 'purpose' argument, must be a known purpose string, typically 'send', or 'receive'.", node.listlabels, "unknown")
+
# Note each time we call generate, all generated coins go into
# the same address, so we call twice to get two addresses w/50 each
self.generatetoaddress(node, nblocks=1, address=node.getnewaddress(label='coinbase'))