aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/bdb.cpp25
-rw-r--r--src/wallet/bdb.h5
-rw-r--r--src/wallet/db.h2
-rw-r--r--src/wallet/feebumper.cpp16
-rw-r--r--src/wallet/interfaces.cpp15
-rw-r--r--src/wallet/rpc/addresses.cpp3
-rw-r--r--src/wallet/rpc/backup.cpp20
-rw-r--r--src/wallet/rpc/wallet.cpp4
-rw-r--r--src/wallet/sqlite.cpp24
-rw-r--r--src/wallet/sqlite.h3
-rw-r--r--src/wallet/test/wallet_tests.cpp69
-rw-r--r--src/wallet/wallet.cpp87
-rw-r--r--src/wallet/wallet.h36
-rw-r--r--src/wallet/walletdb.cpp35
-rw-r--r--src/wallet/walletdb.h9
15 files changed, 248 insertions, 105 deletions
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 4eab342495..fa6e56c6e4 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -668,12 +668,14 @@ void BerkeleyDatabase::ReloadDbEnv()
env->ReloadDbEnv();
}
-BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database)
+BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database, BerkeleyBatch* batch)
{
if (!database.m_db.get()) {
throw std::runtime_error(STR_INTERNAL_BUG("BerkeleyDatabase does not exist"));
}
- int ret = database.m_db->cursor(nullptr, &m_cursor, 0);
+ // Transaction argument to cursor is only needed when using the cursor to
+ // write to the database. Read-only cursors do not need a txn pointer.
+ int ret = database.m_db->cursor(batch ? batch->txn() : nullptr, &m_cursor, 0);
if (ret != 0) {
throw std::runtime_error(STR_INTERNAL_BUG(strprintf("BDB Cursor could not be created. Returned %d", ret)));
}
@@ -820,6 +822,25 @@ bool BerkeleyBatch::HasKey(DataStream&& key)
return ret == 0;
}
+bool BerkeleyBatch::ErasePrefix(Span<const std::byte> prefix)
+{
+ if (!TxnBegin()) return false;
+ auto cursor{std::make_unique<BerkeleyCursor>(m_database, this)};
+ // const_cast is safe below even though prefix_key is an in/out parameter,
+ // because we are not using the DB_DBT_USERMEM flag, so BDB will allocate
+ // and return a different output data pointer
+ Dbt prefix_key{const_cast<std::byte*>(prefix.data()), static_cast<uint32_t>(prefix.size())}, prefix_value{};
+ int ret{cursor->dbc()->get(&prefix_key, &prefix_value, DB_SET_RANGE)};
+ for (int flag{DB_CURRENT}; ret == 0; flag = DB_NEXT) {
+ SafeDbt key, value;
+ ret = cursor->dbc()->get(key, value, flag);
+ if (ret != 0 || key.get_size() < prefix.size() || memcmp(key.get_data(), prefix.data(), prefix.size()) != 0) break;
+ ret = cursor->dbc()->del(0);
+ }
+ cursor.reset();
+ return TxnCommit() && (ret == 0 || ret == DB_NOTFOUND);
+}
+
void BerkeleyDatabase::AddRef()
{
LOCK(cs_db);
diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
index ff24081696..4fac128bf1 100644
--- a/src/wallet/bdb.h
+++ b/src/wallet/bdb.h
@@ -192,10 +192,11 @@ private:
Dbc* m_cursor;
public:
- explicit BerkeleyCursor(BerkeleyDatabase& database);
+ explicit BerkeleyCursor(BerkeleyDatabase& database, BerkeleyBatch* batch=nullptr);
~BerkeleyCursor() override;
Status Next(DataStream& key, DataStream& value) override;
+ Dbc* dbc() const { return m_cursor; }
};
/** RAII class that provides access to a Berkeley database */
@@ -206,6 +207,7 @@ private:
bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override;
bool EraseKey(DataStream&& key) override;
bool HasKey(DataStream&& key) override;
+ bool ErasePrefix(Span<const std::byte> prefix) override;
protected:
Db* pdb{nullptr};
@@ -230,6 +232,7 @@ public:
bool TxnBegin() override;
bool TxnCommit() override;
bool TxnAbort() override;
+ DbTxn* txn() const { return activeTxn; }
};
std::string BerkeleyDatabaseVersion();
diff --git a/src/wallet/db.h b/src/wallet/db.h
index 2da94885be..6834ba6963 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -110,6 +110,7 @@ public:
return HasKey(std::move(ssKey));
}
+ virtual bool ErasePrefix(Span<const std::byte> prefix) = 0;
virtual std::unique_ptr<DatabaseCursor> GetNewCursor() = 0;
virtual bool TxnBegin() = 0;
@@ -186,6 +187,7 @@ private:
bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override { return true; }
bool EraseKey(DataStream&& key) override { return true; }
bool HasKey(DataStream&& key) override { return true; }
+ bool ErasePrefix(Span<const std::byte> prefix) override { return true; }
public:
void Flush() override {}
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index d127c41c43..b6b1fa1d3e 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -231,6 +231,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
// is one). If outputs vector is non-empty, replace original
// outputs with its contents, otherwise use original outputs.
std::vector<CRecipient> recipients;
+ CAmount new_outputs_value = 0;
const auto& txouts = outputs.empty() ? wtx.tx->vout : outputs;
for (const auto& output : txouts) {
if (!OutputIsChange(wallet, output)) {
@@ -241,6 +242,21 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
ExtractDestination(output.scriptPubKey, change_dest);
new_coin_control.destChange = change_dest;
}
+ new_outputs_value += output.nValue;
+ }
+
+ // If no recipients, means that we are sending coins to a change address
+ if (recipients.empty()) {
+ // Just as a sanity check, ensure that the change address exist
+ if (std::get_if<CNoDestination>(&new_coin_control.destChange)) {
+ errors.emplace_back(Untranslated("Unable to create transaction. Transaction must have at least one recipient"));
+ return Result::INVALID_PARAMETER;
+ }
+
+ // Add change as recipient with SFFO flag enabled, so fees are deduced from it.
+ // If the output differs from the original tx output (because the user customized it) a new change output will be created.
+ recipients.emplace_back(CRecipient{GetScriptForDestination(new_coin_control.destChange), new_outputs_value, /*fSubtractFeeFromAmount=*/true});
+ new_coin_control.destChange = CNoDestination();
}
if (coin_control.m_feerate) {
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 1cde4207e2..cd438cfe2f 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -228,9 +228,22 @@ public:
return m_wallet->GetAddressReceiveRequests();
}
bool setAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& value) override {
+ // Note: The setAddressReceiveRequest interface used by the GUI to store
+ // receive requests is a little awkward and could be improved in the
+ // future:
+ //
+ // - The same method is used to save requests and erase them, but
+ // having separate methods could be clearer and prevent bugs.
+ //
+ // - Request ids are passed as strings even though they are generated as
+ // integers.
+ //
+ // - Multiple requests can be stored for the same address, but it might
+ // be better to only allow one request or only keep the current one.
LOCK(m_wallet->cs_wallet);
WalletBatch batch{m_wallet->GetDatabase()};
- return m_wallet->SetAddressReceiveRequest(batch, dest, id, value);
+ return value.empty() ? m_wallet->EraseAddressReceiveRequest(batch, dest, id)
+ : m_wallet->SetAddressReceiveRequest(batch, dest, id, value);
}
bool displayAddress(const CTxDestination& dest) override
{
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index 633f606747..a5be0739a9 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -218,7 +218,8 @@ RPCHelpMan addmultisigaddress()
"Each key is a Bitcoin address or hex-encoded public key.\n"
"This functionality is only intended for use with non-watchonly addresses.\n"
"See `importaddress` for watchonly p2sh address support.\n"
- "If 'label' is specified, assign address to that label.\n",
+ "If 'label' is specified, assign address to that label.\n"
+ "Note: This command is only compatible with legacy wallets.\n",
{
{"nrequired", RPCArg::Type::NUM, RPCArg::Optional::NO, "The number of required signatures out of the n keys or addresses."},
{"keys", RPCArg::Type::ARR, RPCArg::Optional::NO, "The bitcoin addresses or hex-encoded public keys",
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index d9712f05d2..553bbfb62f 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -119,7 +119,8 @@ RPCHelpMan importprivkey()
"may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n"
"The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n"
"but the key was used to create transactions, rescanblockchain needs to be called with the appropriate block range.\n"
- "Note: Use \"getwalletinfo\" to query the scanning progress.\n",
+ "Note: Use \"getwalletinfo\" to query the scanning progress.\n"
+ "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" with \"combo(X)\" for descriptor wallets.\n",
{
{"privkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The private key (see dumpprivkey)"},
{"label", RPCArg::Type::STR, RPCArg::DefaultHint{"current label if address exists, otherwise \"\""}, "An optional label"},
@@ -225,7 +226,7 @@ RPCHelpMan importaddress()
"\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n"
"as change, and not show up in many RPCs.\n"
"Note: Use \"getwalletinfo\" to query the scanning progress.\n"
- "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" with \"addr(X)\" for descriptor wallets.\n",
+ "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" for descriptor wallets.\n",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Bitcoin address (or hex-encoded script)"},
{"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"},
@@ -417,7 +418,8 @@ RPCHelpMan importpubkey()
"may report that the imported pubkey exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n"
"The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n"
"but the key was used to create transactions, rescanblockchain needs to be called with the appropriate block range.\n"
- "Note: Use \"getwalletinfo\" to query the scanning progress.\n",
+ "Note: Use \"getwalletinfo\" to query the scanning progress.\n"
+ "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" with \"combo(X)\" for descriptor wallets.\n",
{
{"pubkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The hex-encoded public key"},
{"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"},
@@ -495,7 +497,8 @@ RPCHelpMan importwallet()
{
return RPCHelpMan{"importwallet",
"\nImports keys from a wallet dump file (see dumpwallet). Requires a new wallet backup to include imported keys.\n"
- "Note: Blockchain and Mempool will be rescanned after a successful import. Use \"getwalletinfo\" to query the scanning progress.\n",
+ "Note: Blockchain and Mempool will be rescanned after a successful import. Use \"getwalletinfo\" to query the scanning progress.\n"
+ "Note: This command is only compatible with legacy wallets.\n",
{
{"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet file"},
},
@@ -642,7 +645,8 @@ RPCHelpMan dumpprivkey()
{
return RPCHelpMan{"dumpprivkey",
"\nReveals the private key corresponding to 'address'.\n"
- "Then the importprivkey can be used with this output\n",
+ "Then the importprivkey can be used with this output\n"
+ "Note: This command is only compatible with legacy wallets.\n",
{
{"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address for the private key"},
},
@@ -690,7 +694,8 @@ RPCHelpMan dumpwallet()
"\nDumps all wallet keys in a human-readable format to a server-side file. This does not allow overwriting existing files.\n"
"Imported scripts are included in the dumpfile, but corresponding BIP173 addresses, etc. may not be added automatically by importwallet.\n"
"Note that if your wallet contains keys which are not derived from your HD seed (e.g. imported keys), these are not covered by\n"
- "only backing up the seed itself, and must be backed up too (e.g. ensure you back up the whole dumpfile).\n",
+ "only backing up the seed itself, and must be backed up too (e.g. ensure you back up the whole dumpfile).\n"
+ "Note: This command is only compatible with legacy wallets.\n",
{
{"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The filename with path (absolute path recommended)"},
},
@@ -1253,7 +1258,8 @@ RPCHelpMan importmulti()
"may report that the imported keys, addresses or scripts exist but related transactions are still missing.\n"
"The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n"
"but the key was used to create transactions, rescanblockchain needs to be called with the appropriate block range.\n"
- "Note: Use \"getwalletinfo\" to query the scanning progress.\n",
+ "Note: Use \"getwalletinfo\" to query the scanning progress.\n"
+ "Note: This command is only compatible with legacy wallets. Use \"importdescriptors\" for descriptor wallets.\n",
{
{"requests", RPCArg::Type::ARR, RPCArg::Optional::NO, "Data to be imported",
{
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index a3a2a7b89c..d728b2fb96 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -507,8 +507,8 @@ static RPCHelpMan sethdseed()
return RPCHelpMan{"sethdseed",
"\nSet or generate a new HD wallet seed. Non-HD wallets will not be upgraded to being a HD wallet. Wallets that are already\n"
"HD will have a new HD seed set so that new keys added to the keypool will be derived from this new seed.\n"
- "\nNote that you will need to MAKE A NEW BACKUP of your wallet after setting the HD wallet seed." +
- HELP_REQUIRING_PASSPHRASE,
+ "\nNote that you will need to MAKE A NEW BACKUP of your wallet after setting the HD wallet seed." + HELP_REQUIRING_PASSPHRASE +
+ "Note: This command is only compatible with legacy wallets.\n",
{
{"newkeypool", RPCArg::Type::BOOL, RPCArg::Default{true}, "Whether to flush old unused addresses, including change addresses, from the keypool and regenerate it.\n"
"If true, the next address from getnewaddress and change address from getrawchangeaddress will be from this new seed.\n"
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index d6259e095e..77e8a4e9c1 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -125,6 +125,7 @@ void SQLiteBatch::SetupSQLStatements()
{&m_insert_stmt, "INSERT INTO main VALUES(?, ?)"},
{&m_overwrite_stmt, "INSERT or REPLACE into main values(?, ?)"},
{&m_delete_stmt, "DELETE FROM main WHERE key = ?"},
+ {&m_delete_prefix_stmt, "DELETE FROM main WHERE instr(key, ?) = 1"},
};
for (const auto& [stmt_prepared, stmt_text] : statements) {
@@ -375,6 +376,7 @@ void SQLiteBatch::Close()
{&m_insert_stmt, "insert"},
{&m_overwrite_stmt, "overwrite"},
{&m_delete_stmt, "delete"},
+ {&m_delete_prefix_stmt, "delete prefix"},
};
for (const auto& [stmt_prepared, stmt_description] : statements) {
@@ -441,24 +443,34 @@ bool SQLiteBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite)
return res == SQLITE_DONE;
}
-bool SQLiteBatch::EraseKey(DataStream&& key)
+bool SQLiteBatch::ExecStatement(sqlite3_stmt* stmt, Span<const std::byte> blob)
{
if (!m_database.m_db) return false;
- assert(m_delete_stmt);
+ assert(stmt);
// Bind: leftmost parameter in statement is index 1
- if (!BindBlobToStatement(m_delete_stmt, 1, key, "key")) return false;
+ if (!BindBlobToStatement(stmt, 1, blob, "key")) return false;
// Execute
- int res = sqlite3_step(m_delete_stmt);
- sqlite3_clear_bindings(m_delete_stmt);
- sqlite3_reset(m_delete_stmt);
+ int res = sqlite3_step(stmt);
+ sqlite3_clear_bindings(stmt);
+ sqlite3_reset(stmt);
if (res != SQLITE_DONE) {
LogPrintf("%s: Unable to execute statement: %s\n", __func__, sqlite3_errstr(res));
}
return res == SQLITE_DONE;
}
+bool SQLiteBatch::EraseKey(DataStream&& key)
+{
+ return ExecStatement(m_delete_stmt, key);
+}
+
+bool SQLiteBatch::ErasePrefix(Span<const std::byte> prefix)
+{
+ return ExecStatement(m_delete_prefix_stmt, prefix);
+}
+
bool SQLiteBatch::HasKey(DataStream&& key)
{
if (!m_database.m_db) return false;
diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h
index 5745a1d4cf..d9de40569b 100644
--- a/src/wallet/sqlite.h
+++ b/src/wallet/sqlite.h
@@ -36,13 +36,16 @@ private:
sqlite3_stmt* m_insert_stmt{nullptr};
sqlite3_stmt* m_overwrite_stmt{nullptr};
sqlite3_stmt* m_delete_stmt{nullptr};
+ sqlite3_stmt* m_delete_prefix_stmt{nullptr};
void SetupSQLStatements();
+ bool ExecStatement(sqlite3_stmt* stmt, Span<const std::byte> blob);
bool ReadKey(DataStream&& key, DataStream& value) override;
bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override;
bool EraseKey(DataStream&& key) override;
bool HasKey(DataStream&& key) override;
+ bool ErasePrefix(Span<const std::byte> prefix) override;
public:
explicit SQLiteBatch(SQLiteDatabase& database);
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index edcfaa24e5..da4b553394 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -427,19 +427,63 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart)
BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 5, 50, 600), 300);
}
-BOOST_AUTO_TEST_CASE(LoadReceiveRequests)
+static const DatabaseFormat DATABASE_FORMATS[] = {
+#ifdef USE_SQLITE
+ DatabaseFormat::SQLITE,
+#endif
+#ifdef USE_BDB
+ DatabaseFormat::BERKELEY,
+#endif
+};
+
+void TestLoadWallet(const std::string& name, DatabaseFormat format, std::function<void(std::shared_ptr<CWallet>)> f)
{
- CTxDestination dest = PKHash();
- LOCK(m_wallet.cs_wallet);
- WalletBatch batch{m_wallet.GetDatabase()};
- m_wallet.SetAddressUsed(batch, dest, true);
- m_wallet.SetAddressReceiveRequest(batch, dest, "0", "val_rr0");
- m_wallet.SetAddressReceiveRequest(batch, dest, "1", "val_rr1");
-
- auto values = m_wallet.GetAddressReceiveRequests();
- BOOST_CHECK_EQUAL(values.size(), 2U);
- BOOST_CHECK_EQUAL(values[0], "val_rr0");
- BOOST_CHECK_EQUAL(values[1], "val_rr1");
+ node::NodeContext node;
+ auto chain{interfaces::MakeChain(node)};
+ DatabaseOptions options;
+ options.require_format = format;
+ DatabaseStatus status;
+ bilingual_str error;
+ std::vector<bilingual_str> warnings;
+ auto database{MakeWalletDatabase(name, options, status, error)};
+ auto wallet{std::make_shared<CWallet>(chain.get(), "", std::move(database))};
+ BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK);
+ WITH_LOCK(wallet->cs_wallet, f(wallet));
+}
+
+BOOST_FIXTURE_TEST_CASE(LoadReceiveRequests, TestingSetup)
+{
+ for (DatabaseFormat format : DATABASE_FORMATS) {
+ const std::string name{strprintf("receive-requests-%i", format)};
+ TestLoadWallet(name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
+ BOOST_CHECK(!wallet->IsAddressPreviouslySpent(PKHash()));
+ WalletBatch batch{wallet->GetDatabase()};
+ BOOST_CHECK(batch.WriteAddressPreviouslySpent(PKHash(), true));
+ BOOST_CHECK(batch.WriteAddressPreviouslySpent(ScriptHash(), true));
+ BOOST_CHECK(wallet->SetAddressReceiveRequest(batch, PKHash(), "0", "val_rr00"));
+ BOOST_CHECK(wallet->EraseAddressReceiveRequest(batch, PKHash(), "0"));
+ BOOST_CHECK(wallet->SetAddressReceiveRequest(batch, PKHash(), "1", "val_rr10"));
+ BOOST_CHECK(wallet->SetAddressReceiveRequest(batch, PKHash(), "1", "val_rr11"));
+ BOOST_CHECK(wallet->SetAddressReceiveRequest(batch, ScriptHash(), "2", "val_rr20"));
+ });
+ TestLoadWallet(name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
+ BOOST_CHECK(wallet->IsAddressPreviouslySpent(PKHash()));
+ BOOST_CHECK(wallet->IsAddressPreviouslySpent(ScriptHash()));
+ auto requests = wallet->GetAddressReceiveRequests();
+ auto erequests = {"val_rr11", "val_rr20"};
+ BOOST_CHECK_EQUAL_COLLECTIONS(requests.begin(), requests.end(), std::begin(erequests), std::end(erequests));
+ WalletBatch batch{wallet->GetDatabase()};
+ BOOST_CHECK(batch.WriteAddressPreviouslySpent(PKHash(), false));
+ BOOST_CHECK(batch.EraseAddressData(ScriptHash()));
+ });
+ TestLoadWallet(name, format, [](std::shared_ptr<CWallet> wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet) {
+ BOOST_CHECK(!wallet->IsAddressPreviouslySpent(PKHash()));
+ BOOST_CHECK(!wallet->IsAddressPreviouslySpent(ScriptHash()));
+ auto requests = wallet->GetAddressReceiveRequests();
+ auto erequests = {"val_rr11"};
+ BOOST_CHECK_EQUAL_COLLECTIONS(requests.begin(), requests.end(), std::begin(erequests), std::end(erequests));
+ });
+ }
}
// Test some watch-only LegacyScriptPubKeyMan methods by the procedure of loading (LoadWatchOnly),
@@ -922,6 +966,7 @@ private:
bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) override { return m_pass; }
bool EraseKey(DataStream&& key) override { return m_pass; }
bool HasKey(DataStream&& key) override { return m_pass; }
+ bool ErasePrefix(Span<const std::byte> prefix) override { return m_pass; }
public:
explicit FailBatch(bool pass) : m_pass(pass) {}
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 56bd25b90a..caf95a3f03 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -967,11 +967,11 @@ void CWallet::SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned
CTxDestination dst;
if (ExtractDestination(srctx->tx->vout[n].scriptPubKey, dst)) {
if (IsMine(dst)) {
- if (used != IsAddressUsed(dst)) {
+ if (used != IsAddressPreviouslySpent(dst)) {
if (used) {
tx_destinations.insert(dst);
}
- SetAddressUsed(batch, dst, used);
+ SetAddressPreviouslySpent(batch, dst, used);
}
}
}
@@ -984,7 +984,7 @@ bool CWallet::IsSpentKey(const CScript& scriptPubKey) const
if (!ExtractDestination(scriptPubKey, dest)) {
return false;
}
- if (IsAddressUsed(dest)) {
+ if (IsAddressPreviouslySpent(dest)) {
return true;
}
if (IsLegacy()) {
@@ -992,15 +992,15 @@ bool CWallet::IsSpentKey(const CScript& scriptPubKey) const
assert(spk_man != nullptr);
for (const auto& keyid : GetAffectedKeys(scriptPubKey, *spk_man)) {
WitnessV0KeyHash wpkh_dest(keyid);
- if (IsAddressUsed(wpkh_dest)) {
+ if (IsAddressPreviouslySpent(wpkh_dest)) {
return true;
}
ScriptHash sh_wpkh_dest(GetScriptForDestination(wpkh_dest));
- if (IsAddressUsed(sh_wpkh_dest)) {
+ if (IsAddressPreviouslySpent(sh_wpkh_dest)) {
return true;
}
PKHash pkh_dest(keyid);
- if (IsAddressUsed(pkh_dest)) {
+ if (IsAddressPreviouslySpent(pkh_dest)) {
return true;
}
}
@@ -2403,19 +2403,15 @@ bool CWallet::DelAddressBook(const CTxDestination& address)
WalletBatch batch(GetDatabase());
{
LOCK(cs_wallet);
- // If we want to delete receiving addresses, we need to take care that DestData "used" (and possibly newer DestData) gets preserved (and the "deleted" address transformed into a change entry instead of actually being deleted)
- // NOTE: This isn't a problem for sending addresses because they never have any DestData yet!
- // When adding new DestData, it should be considered here whether to retain or delete it (or move it?).
+ // If we want to delete receiving addresses, we should avoid calling EraseAddressData because it will delete the previously_spent value. Could instead just erase the label so it becomes a change address, and keep the data.
+ // NOTE: This isn't a problem for sending addresses because they don't have any data that needs to be kept.
+ // When adding new address data, it should be considered here whether to retain or delete it.
if (IsMine(address)) {
WalletLogPrintf("%s called with IsMine address, NOT SUPPORTED. Please report this bug! %s\n", __func__, PACKAGE_BUGREPORT);
return false;
}
- // Delete destdata tuples associated with address
- std::string strAddress = EncodeDestination(address);
- for (const std::pair<const std::string, std::string> &item : m_address_book[address].destdata)
- {
- batch.EraseDestData(strAddress, item.first);
- }
+ // Delete data rows associated with this address
+ batch.EraseAddressData(address);
m_address_book.erase(address);
}
@@ -2790,51 +2786,42 @@ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx, bool rescanning_old
return nTimeSmart;
}
-bool CWallet::SetAddressUsed(WalletBatch& batch, const CTxDestination& dest, bool used)
+bool CWallet::SetAddressPreviouslySpent(WalletBatch& batch, const CTxDestination& dest, bool used)
{
- const std::string key{"used"};
if (std::get_if<CNoDestination>(&dest))
return false;
if (!used) {
- if (auto* data = util::FindKey(m_address_book, dest)) data->destdata.erase(key);
- return batch.EraseDestData(EncodeDestination(dest), key);
+ if (auto* data{util::FindKey(m_address_book, dest)}) data->previously_spent = false;
+ return batch.WriteAddressPreviouslySpent(dest, false);
}
- const std::string value{"1"};
- m_address_book[dest].destdata.insert(std::make_pair(key, value));
- return batch.WriteDestData(EncodeDestination(dest), key, value);
+ LoadAddressPreviouslySpent(dest);
+ return batch.WriteAddressPreviouslySpent(dest, true);
}
-void CWallet::LoadDestData(const CTxDestination &dest, const std::string &key, const std::string &value)
+void CWallet::LoadAddressPreviouslySpent(const CTxDestination& dest)
{
- m_address_book[dest].destdata.insert(std::make_pair(key, value));
+ m_address_book[dest].previously_spent = true;
}
-bool CWallet::IsAddressUsed(const CTxDestination& dest) const
+void CWallet::LoadAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& request)
{
- const std::string key{"used"};
- std::map<CTxDestination, CAddressBookData>::const_iterator i = m_address_book.find(dest);
- if(i != m_address_book.end())
- {
- CAddressBookData::StringMap::const_iterator j = i->second.destdata.find(key);
- if(j != i->second.destdata.end())
- {
- return true;
- }
- }
+ m_address_book[dest].receive_requests[id] = request;
+}
+
+bool CWallet::IsAddressPreviouslySpent(const CTxDestination& dest) const
+{
+ if (auto* data{util::FindKey(m_address_book, dest)}) return data->previously_spent;
return false;
}
std::vector<std::string> CWallet::GetAddressReceiveRequests() const
{
- const std::string prefix{"rr"};
std::vector<std::string> values;
- for (const auto& address : m_address_book) {
- for (const auto& data : address.second.destdata) {
- if (!data.first.compare(0, prefix.size(), prefix)) {
- values.emplace_back(data.second);
- }
+ for (const auto& [dest, entry] : m_address_book) {
+ for (const auto& [id, request] : entry.receive_requests) {
+ values.emplace_back(request);
}
}
return values;
@@ -2842,15 +2829,15 @@ std::vector<std::string> CWallet::GetAddressReceiveRequests() const
bool CWallet::SetAddressReceiveRequest(WalletBatch& batch, const CTxDestination& dest, const std::string& id, const std::string& value)
{
- const std::string key{"rr" + id}; // "rr" prefix = "receive request" in destdata
- CAddressBookData& data = m_address_book.at(dest);
- if (value.empty()) {
- if (!batch.EraseDestData(EncodeDestination(dest), key)) return false;
- data.destdata.erase(key);
- } else {
- if (!batch.WriteDestData(EncodeDestination(dest), key, value)) return false;
- data.destdata[key] = value;
- }
+ if (!batch.WriteAddressReceiveRequest(dest, id, value)) return false;
+ m_address_book[dest].receive_requests[id] = value;
+ return true;
+}
+
+bool CWallet::EraseAddressReceiveRequest(WalletBatch& batch, const CTxDestination& dest, const std::string& id)
+{
+ if (!batch.EraseAddressReceiveRequest(dest, id)) return false;
+ m_address_book[dest].receive_requests.erase(id);
return true;
}
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 581a6bd9cb..5252b46cdc 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -221,17 +221,22 @@ struct CAddressBookData
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
+ * Whether coins with this address have previously been spent. Set when the
+ * the wallet avoid_reuse option is enabled and this is an IsMine address
+ * that has already received funds and spent them. This is used during coin
+ * selection to increase privacy by not creating different transactions
+ * that spend from the same addresses.
+ */
+ bool previously_spent{false};
+
+ /**
+ * Map containing data about previously generated receive requests
+ * requesting funds to be sent to this address. Only present for IsMine
+ * addresses. Map keys are decimal numbers uniquely identifying each
+ * request, and map values are serialized RecentRequestEntry objects
+ * containing BIP21 URI information including message and amount.
*/
- typedef std::map<std::string, std::string> StringMap;
- StringMap destdata;
+ std::map<std::string, std::string> receive_requests{};
/** Accessor methods. */
bool IsChange() const { return !label.has_value(); }
@@ -511,8 +516,10 @@ public:
bool LoadMinVersion(int nVersion) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet) { AssertLockHeld(cs_wallet); nWalletVersion = nVersion; return true; }
- //! Adds a destination data tuple to the store, without saving it to disk
- void LoadDestData(const CTxDestination& dest, const std::string& key, const std::string& value) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ //! Marks destination as previously spent.
+ void LoadAddressPreviouslySpent(const CTxDestination& dest) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ //! Appends payment request to destination.
+ void LoadAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& request) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
//! Holds a timestamp at which point the wallet is scheduled (externally) to be relocked. Caller must arrange for actual relocking to occur via Lock().
int64_t nRelockTime GUARDED_BY(cs_wallet){0};
@@ -739,11 +746,12 @@ public:
bool DelAddressBook(const CTxDestination& address);
- bool IsAddressUsed(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
- bool SetAddressUsed(WalletBatch& batch, const CTxDestination& dest, bool used) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ bool IsAddressPreviouslySpent(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ bool SetAddressPreviouslySpent(WalletBatch& batch, const CTxDestination& dest, bool used) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
std::vector<std::string> GetAddressReceiveRequests() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
bool SetAddressReceiveRequest(WalletBatch& batch, const CTxDestination& dest, const std::string& id, const std::string& value) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+ bool EraseAddressReceiveRequest(WalletBatch& batch, const CTxDestination& dest, const std::string& id) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
unsigned int GetKeyPoolSize() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index c8e8ce4614..005592d720 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -615,7 +615,20 @@ ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue,
ssKey >> strAddress;
ssKey >> strKey;
ssValue >> strValue;
- pwallet->LoadDestData(DecodeDestination(strAddress), strKey, strValue);
+ const CTxDestination& dest{DecodeDestination(strAddress)};
+ if (strKey.compare("used") == 0) {
+ // Load "used" key indicating if an IsMine address has
+ // previously been spent from with avoid_reuse option enabled.
+ // The strValue is not used for anything currently, but could
+ // hold more information in the future. Current values are just
+ // "1" or "p" for present (which was written prior to
+ // f5ba424cd44619d9b9be88b8593d69a7ba96db26).
+ pwallet->LoadAddressPreviouslySpent(dest);
+ } else if (strKey.compare(0, 2, "rr") == 0) {
+ // Load "rr##" keys where ## is a decimal number, and strValue
+ // is a serialized RecentRequestEntry object.
+ pwallet->LoadAddressReceiveRequest(dest, strKey.substr(2), strValue);
+ }
} else if (strType == DBKeys::HDCHAIN) {
CHDChain chain;
ssValue >> chain;
@@ -1088,16 +1101,28 @@ void MaybeCompactWalletDB(WalletContext& context)
fOneThread = false;
}
-bool WalletBatch::WriteDestData(const std::string &address, const std::string &key, const std::string &value)
+bool WalletBatch::WriteAddressPreviouslySpent(const CTxDestination& dest, bool previously_spent)
+{
+ auto key{std::make_pair(DBKeys::DESTDATA, std::make_pair(EncodeDestination(dest), std::string("used")))};
+ return previously_spent ? WriteIC(key, std::string("1")) : EraseIC(key);
+}
+
+bool WalletBatch::WriteAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& receive_request)
{
- return WriteIC(std::make_pair(DBKeys::DESTDATA, std::make_pair(address, key)), value);
+ return WriteIC(std::make_pair(DBKeys::DESTDATA, std::make_pair(EncodeDestination(dest), "rr" + id)), receive_request);
}
-bool WalletBatch::EraseDestData(const std::string &address, const std::string &key)
+bool WalletBatch::EraseAddressReceiveRequest(const CTxDestination& dest, const std::string& id)
{
- return EraseIC(std::make_pair(DBKeys::DESTDATA, std::make_pair(address, key)));
+ return EraseIC(std::make_pair(DBKeys::DESTDATA, std::make_pair(EncodeDestination(dest), "rr" + id)));
}
+bool WalletBatch::EraseAddressData(const CTxDestination& dest)
+{
+ DataStream prefix;
+ prefix << DBKeys::DESTDATA << EncodeDestination(dest);
+ return m_batch->ErasePrefix(prefix);
+}
bool WalletBatch::WriteHDChain(const CHDChain& chain)
{
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index c97356a71f..72086e950a 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -7,6 +7,7 @@
#define BITCOIN_WALLET_WALLETDB_H
#include <script/sign.h>
+#include <script/standard.h>
#include <wallet/db.h>
#include <wallet/walletutil.h>
#include <key.h>
@@ -264,10 +265,10 @@ public:
bool WriteLockedUTXO(const COutPoint& output);
bool EraseLockedUTXO(const COutPoint& output);
- /// Write destination data key,value tuple to database
- bool WriteDestData(const std::string &address, const std::string &key, const std::string &value);
- /// Erase destination data tuple from wallet database
- bool EraseDestData(const std::string &address, const std::string &key);
+ bool WriteAddressPreviouslySpent(const CTxDestination& dest, bool previously_spent);
+ bool WriteAddressReceiveRequest(const CTxDestination& dest, const std::string& id, const std::string& receive_request);
+ bool EraseAddressReceiveRequest(const CTxDestination& dest, const std::string& id);
+ bool EraseAddressData(const CTxDestination& dest);
bool WriteActiveScriptPubKeyMan(uint8_t type, const uint256& id, bool internal);
bool EraseActiveScriptPubKeyMan(uint8_t type, bool internal);