aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/bdb.cpp2
-rw-r--r--src/wallet/coinselection.cpp16
-rw-r--r--src/wallet/coinselection.h14
-rw-r--r--src/wallet/rpc/addresses.cpp12
-rw-r--r--src/wallet/rpc/backup.cpp16
-rw-r--r--src/wallet/rpc/spend.cpp18
-rw-r--r--src/wallet/rpc/transactions.cpp8
-rw-r--r--src/wallet/rpc/util.cpp5
-rw-r--r--src/wallet/rpc/wallet.cpp27
-rw-r--r--src/wallet/scriptpubkeyman.cpp2
-rw-r--r--src/wallet/spend.cpp133
-rw-r--r--src/wallet/spend.h26
-rw-r--r--src/wallet/test/availablecoins_tests.cpp107
-rw-r--r--src/wallet/test/coinselector_tests.cpp11
-rw-r--r--src/wallet/test/wallet_tests.cpp52
-rw-r--r--src/wallet/wallet.cpp55
-rw-r--r--src/wallet/wallet.h4
-rw-r--r--src/wallet/walletdb.cpp11
-rw-r--r--src/wallet/walletdb.h2
19 files changed, 263 insertions, 258 deletions
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 06650a73cc..4ec3ac2189 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -100,7 +100,7 @@ void BerkeleyEnvironment::Close()
if (ret != 0)
LogPrintf("BerkeleyEnvironment::Close: Error %d closing database environment: %s\n", ret, DbEnv::strerror(ret));
if (!fMockDb)
- DbEnv((uint32_t)0).remove(strPath.c_str(), 0);
+ DbEnv(uint32_t{0}).remove(strPath.c_str(), 0);
if (error_file) fclose(error_file);
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 4fba567b7f..e6ba89627c 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -88,13 +88,15 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo
std::vector<size_t> best_selection;
CAmount best_waste = MAX_MONEY;
+ bool is_feerate_high = utxo_pool.at(0).fee > utxo_pool.at(0).long_term_fee;
+
// Depth First search loop for choosing the UTXOs
for (size_t curr_try = 0, utxo_pool_index = 0; curr_try < TOTAL_TRIES; ++curr_try, ++utxo_pool_index) {
// Conditions for starting a backtrack
bool backtrack = false;
if (curr_value + curr_available_value < selection_target || // Cannot possibly reach target with the amount remaining in the curr_available_value.
curr_value > selection_target + cost_of_change || // Selected value is out of range, go back and try other branch
- (curr_waste > best_waste && (utxo_pool.at(0).fee - utxo_pool.at(0).long_term_fee) > 0)) { // Don't select things which we know will be more wasteful if the waste is increasing
+ (curr_waste > best_waste && is_feerate_high)) { // Don't select things which we know will be more wasteful if the waste is increasing
backtrack = true;
} else if (curr_value >= selection_target) { // Selected value is within range
curr_waste += (curr_value - selection_target); // This is the excess value which is added to the waste for the below comparison
@@ -446,7 +448,8 @@ void SelectionResult::Clear()
void SelectionResult::AddInput(const OutputGroup& group)
{
- util::insert(m_selected_inputs, group.m_outputs);
+ // As it can fail, combine inputs first
+ InsertInputs(group.m_outputs);
m_use_effective = !group.m_subtract_fee_outputs;
m_weight += group.m_weight;
@@ -454,7 +457,8 @@ void SelectionResult::AddInput(const OutputGroup& group)
void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs)
{
- util::insert(m_selected_inputs, inputs);
+ // As it can fail, combine inputs first
+ InsertInputs(inputs);
m_use_effective = !subtract_fee_outputs;
m_weight += std::accumulate(inputs.cbegin(), inputs.cend(), 0, [](int sum, const auto& coin) {
@@ -464,16 +468,14 @@ void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_f
void SelectionResult::Merge(const SelectionResult& other)
{
- // Obtain the expected selected inputs count after the merge (for now, duplicates are not allowed)
- const size_t expected_count = m_selected_inputs.size() + other.m_selected_inputs.size();
+ // As it can fail, combine inputs first
+ InsertInputs(other.m_selected_inputs);
m_target += other.m_target;
m_use_effective |= other.m_use_effective;
if (m_algo == SelectionAlgorithm::MANUAL) {
m_algo = other.m_algo;
}
- util::insert(m_selected_inputs, other.m_selected_inputs);
- assert(m_selected_inputs.size() == expected_count);
m_weight += other.m_weight;
}
diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h
index 2efeb8476d..9ff2011ce3 100644
--- a/src/wallet/coinselection.h
+++ b/src/wallet/coinselection.h
@@ -10,6 +10,8 @@
#include <policy/feerate.h>
#include <primitives/transaction.h>
#include <random.h>
+#include <util/system.h>
+#include <util/check.h>
#include <optional>
@@ -186,6 +188,7 @@ struct CoinEligibilityFilter
/** When avoid_reuse=true and there are full groups (OUTPUT_GROUP_MAX_ENTRIES), whether or not to use any partial groups.*/
const bool m_include_partial_groups{false};
+ CoinEligibilityFilter() = delete;
CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_ancestors) {}
CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants) {}
CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants, bool include_partial) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants), m_include_partial_groups(include_partial) {}
@@ -301,6 +304,17 @@ private:
/** Total weight of the selected inputs */
int m_weight{0};
+ template<typename T>
+ void InsertInputs(const T& inputs)
+ {
+ // Store sum of combined input sets to check that the results have no shared UTXOs
+ const size_t expected_count = m_selected_inputs.size() + inputs.size();
+ util::insert(m_selected_inputs, inputs);
+ if (m_selected_inputs.size() != expected_count) {
+ throw std::runtime_error(STR_INTERNAL_BUG("Shared UTXOs among selection results"));
+ }
+ }
+
public:
explicit SelectionResult(const CAmount target, SelectionAlgorithm algo)
: m_target(target), m_algo(algo) {}
diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp
index 7b49bad3f5..95e1ba4dd9 100644
--- a/src/wallet/rpc/addresses.cpp
+++ b/src/wallet/rpc/addresses.cpp
@@ -43,9 +43,7 @@ RPCHelpMan getnewaddress()
}
// Parse the label first so we don't generate a key if there's an error
- std::string label;
- if (!request.params[0].isNull())
- label = LabelFromValue(request.params[0]);
+ const std::string label{LabelFromValue(request.params[0])};
OutputType output_type = pwallet->m_default_address_type;
if (!request.params[1].isNull()) {
@@ -140,7 +138,7 @@ RPCHelpMan setlabel()
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid Bitcoin address");
}
- std::string label = LabelFromValue(request.params[1]);
+ const std::string label{LabelFromValue(request.params[1])};
if (pwallet->IsMine(dest)) {
pwallet->SetAddressBook(dest, label, "receive");
@@ -258,9 +256,7 @@ RPCHelpMan addmultisigaddress()
LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore);
- std::string label;
- if (!request.params[2].isNull())
- label = LabelFromValue(request.params[2]);
+ const std::string label{LabelFromValue(request.params[2])};
int required = request.params[0].getInt<int>();
@@ -662,7 +658,7 @@ RPCHelpMan getaddressesbylabel()
LOCK(pwallet->cs_wallet);
- std::string label = LabelFromValue(request.params[0]);
+ const std::string label{LabelFromValue(request.params[0])};
// Find all addresses that have the given label
UniValue ret(UniValue::VOBJ);
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
index 95117b6c19..64253789ec 100644
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -156,9 +156,7 @@ RPCHelpMan importprivkey()
EnsureWalletIsUnlocked(*pwallet);
std::string strSecret = request.params[0].get_str();
- std::string strLabel;
- if (!request.params[1].isNull())
- strLabel = request.params[1].get_str();
+ const std::string strLabel{LabelFromValue(request.params[1])};
// Whether to perform rescan after import
if (!request.params[2].isNull())
@@ -249,9 +247,7 @@ RPCHelpMan importaddress()
EnsureLegacyScriptPubKeyMan(*pwallet, true);
- std::string strLabel;
- if (!request.params[1].isNull())
- strLabel = request.params[1].get_str();
+ const std::string strLabel{LabelFromValue(request.params[1])};
// Whether to perform rescan after import
bool fRescan = true;
@@ -442,9 +438,7 @@ RPCHelpMan importpubkey()
EnsureLegacyScriptPubKeyMan(*pwallet, true);
- std::string strLabel;
- if (!request.params[1].isNull())
- strLabel = request.params[1].get_str();
+ const std::string strLabel{LabelFromValue(request.params[1])};
// Whether to perform rescan after import
bool fRescan = true;
@@ -1170,7 +1164,7 @@ static UniValue ProcessImport(CWallet& wallet, const UniValue& data, const int64
if (internal && data.exists("label")) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Internal addresses should not have a label");
}
- const std::string& label = data.exists("label") ? data["label"].get_str() : "";
+ const std::string label{LabelFromValue(data["label"])};
const bool add_keypool = data.exists("keypool") ? data["keypool"].get_bool() : false;
// Add to keypool only works with privkeys disabled
@@ -1464,7 +1458,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c
const std::string& descriptor = data["desc"].get_str();
const bool active = data.exists("active") ? data["active"].get_bool() : false;
const bool internal = data.exists("internal") ? data["internal"].get_bool() : false;
- const std::string& label = data.exists("label") ? data["label"].get_str() : "";
+ const std::string label{LabelFromValue(data["label"])};
// Parse descriptor string
FlatSigningProvider keys;
diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp
index 5f31c1d72c..be1ad37592 100644
--- a/src/wallet/rpc/spend.cpp
+++ b/src/wallet/rpc/spend.cpp
@@ -763,13 +763,17 @@ RPCHelpMan fundrawtransaction()
},
{"input_weights", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Inputs and their corresponding weights",
{
- {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"},
- {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output index"},
- {"weight", RPCArg::Type::NUM, RPCArg::Optional::NO, "The maximum weight for this input, "
- "including the weight of the outpoint and sequence number. "
- "Note that serialized signature sizes are not guaranteed to be consistent, "
- "so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures."
- "Remember to convert serialized sizes to weight units when necessary."},
+ {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "",
+ {
+ {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"},
+ {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output index"},
+ {"weight", RPCArg::Type::NUM, RPCArg::Optional::NO, "The maximum weight for this input, "
+ "including the weight of the outpoint and sequence number. "
+ "Note that serialized signature sizes are not guaranteed to be consistent, "
+ "so the maximum DER signatures size of 73 bytes should be used when considering ECDSA signatures."
+ "Remember to convert serialized sizes to weight units when necessary."},
+ },
+ },
},
},
},
diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
index cff371c55b..f571f8bcb2 100644
--- a/src/wallet/rpc/transactions.cpp
+++ b/src/wallet/rpc/transactions.cpp
@@ -416,7 +416,6 @@ static const std::vector<RPCResult> TransactionDescriptionString()
}},
{RPCResult::Type::STR_HEX, "replaced_by_txid", /*optional=*/true, "The txid if this tx was replaced."},
{RPCResult::Type::STR_HEX, "replaces_txid", /*optional=*/true, "The txid if the tx replaces one."},
- {RPCResult::Type::STR, "comment", /*optional=*/true, ""},
{RPCResult::Type::STR, "to", /*optional=*/true, "If a comment to is associated with the transaction."},
{RPCResult::Type::NUM_TIME, "time", "The transaction time expressed in " + UNIX_EPOCH_TIME + "."},
{RPCResult::Type::NUM_TIME, "timereceived", "The time received expressed in " + UNIX_EPOCH_TIME + "."},
@@ -487,7 +486,7 @@ RPCHelpMan listtransactions()
std::optional<std::string> filter_label;
if (!request.params[0].isNull() && request.params[0].get_str() != "*") {
- filter_label = request.params[0].get_str();
+ filter_label.emplace(LabelFromValue(request.params[0]));
if (filter_label.value().empty()) {
throw JSONRPCError(RPC_INVALID_PARAMETER, "Label argument must be a valid label name or \"*\".");
}
@@ -635,10 +634,9 @@ RPCHelpMan listsinceblock()
bool include_removed = (request.params[3].isNull() || request.params[3].get_bool());
bool include_change = (!request.params[4].isNull() && request.params[4].get_bool());
+ // Only set it if 'label' was provided.
std::optional<std::string> filter_label;
- if (!request.params[5].isNull()) {
- filter_label = request.params[5].get_str();
- }
+ if (!request.params[5].isNull()) filter_label.emplace(LabelFromValue(request.params[5]));
int depth = height ? wallet.GetLastBlockHeight() + 1 - *height : -1;
diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp
index b80f86ddc9..31435a69ba 100644
--- a/src/wallet/rpc/util.cpp
+++ b/src/wallet/rpc/util.cpp
@@ -132,7 +132,10 @@ const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wal
std::string LabelFromValue(const UniValue& value)
{
- std::string label = value.get_str();
+ static const std::string empty_string;
+ if (value.isNull()) return empty_string;
+
+ const std::string& label{value.get_str()};
if (label == "*")
throw JSONRPCError(RPC_WALLET_INVALID_LABEL_NAME, "Invalid label name");
return label;
diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp
index f0ecd13f26..aeab9b7634 100644
--- a/src/wallet/rpc/wallet.cpp
+++ b/src/wallet/rpc/wallet.cpp
@@ -226,6 +226,14 @@ static RPCHelpMan loadwallet()
bilingual_str error;
std::vector<bilingual_str> warnings;
std::optional<bool> load_on_start = request.params[1].isNull() ? std::nullopt : std::optional<bool>(request.params[1].get_bool());
+
+ {
+ LOCK(context.wallets_mutex);
+ if (std::any_of(context.wallets.begin(), context.wallets.end(), [&name](const auto& wallet) { return wallet->GetName() == name; })) {
+ throw JSONRPCError(RPC_WALLET_ALREADY_LOADED, "Wallet \"" + name + "\" is already loaded.");
+ }
+ }
+
std::shared_ptr<CWallet> const wallet = LoadWallet(context, name, load_on_start, options, status, error, warnings);
HandleWalletError(wallet, status, error);
@@ -437,13 +445,20 @@ static RPCHelpMan unloadwallet()
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded");
}
- // Release the "main" shared pointer and prevent further notifications.
- // Note that any attempt to load the same wallet would fail until the wallet
- // is destroyed (see CheckUniqueFileid).
std::vector<bilingual_str> warnings;
- std::optional<bool> load_on_start = request.params[1].isNull() ? std::nullopt : std::optional<bool>(request.params[1].get_bool());
- if (!RemoveWallet(context, wallet, load_on_start, warnings)) {
- throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
+ {
+ WalletRescanReserver reserver(*wallet);
+ if (!reserver.reserve()) {
+ throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait.");
+ }
+
+ // Release the "main" shared pointer and prevent further notifications.
+ // Note that any attempt to load the same wallet would fail until the wallet
+ // is destroyed (see CheckUniqueFileid).
+ std::optional<bool> load_on_start = request.params[1].isNull() ? std::nullopt : std::optional<bool>(request.params[1].get_bool());
+ if (!RemoveWallet(context, wallet, load_on_start, warnings)) {
+ throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
+ }
}
UnloadWallet(std::move(wallet));
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index fbb52ea621..d8f34dd2b0 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -2123,7 +2123,7 @@ bool DescriptorScriptPubKeyMan::TopUp(unsigned int size)
if (size > 0) {
target_size = size;
} else {
- target_size = std::max(gArgs.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), (int64_t) 1);
+ target_size = std::max(gArgs.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1});
}
// Calculate the new range_end
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index 3bb3c17119..8bb970936b 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -287,7 +287,7 @@ CoinsResult AvailableCoins(const CWallet& wallet,
if (coinControl && coinControl->HasSelected() && coinControl->IsSelected(outpoint))
continue;
- if (wallet.IsLockedCoin(outpoint))
+ if (wallet.IsLockedCoin(outpoint) && params.skip_locked)
continue;
if (wallet.IsSpent(outpoint))
@@ -510,15 +510,20 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
return groups_out;
}
-std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins,
+// Returns true if the result contains an error and the message is not empty
+static bool HasErrorMsg(const util::Result<SelectionResult>& res) { return !util::ErrorString(res).empty(); }
+
+util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins,
const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types)
{
// Run coin selection on each OutputType and compute the Waste Metric
std::vector<SelectionResult> results;
for (const auto& it : available_coins.coins) {
- if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}) {
- results.push_back(*result);
- }
+ auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)};
+ // If any specific error message appears here, then something particularly wrong happened.
+ if (HasErrorMsg(result)) return result; // So let's return the specific error.
+ // Append the favorable result.
+ if (result) results.push_back(*result);
}
// If we have at least one solution for funding the transaction without mixing, choose the minimum one according to waste metric
// and return the result
@@ -528,16 +533,14 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
// over all available coins, which would allow mixing.
// If TypesCount() <= 1, there is nothing to mix.
if (allow_mixed_output_types && available_coins.TypesCount() > 1) {
- if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) {
- return result;
- }
+ return ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params);
}
// Either mixing is not allowed and we couldn't find a solution from any single OutputType, or mixing was allowed and we still couldn't
// find a solution using all available coins
- return std::nullopt;
+ return util::Error();
};
-std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, const CoinSelectionParams& coin_selection_params)
+util::Result<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins, const CoinSelectionParams& coin_selection_params)
{
// Vector of results. We will choose the best one based on waste.
std::vector<SelectionResult> results;
@@ -561,7 +564,7 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
if (results.empty()) {
// No solution found
- return std::nullopt;
+ return util::Error();
}
std::vector<SelectionResult> eligible_results;
@@ -571,7 +574,8 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
});
if (eligible_results.empty()) {
- return std::nullopt;
+ return util::Error{_("The inputs size exceeds the maximum weight. "
+ "Please try sending a smaller amount or manually consolidating your wallet's UTXOs")};
}
// Choose the result with the least waste
@@ -580,15 +584,18 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons
return best_result;
}
-std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
- const CAmount& nTargetValue, const CCoinControl& coin_control,
- const CoinSelectionParams& coin_selection_params)
+util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
+ const CAmount& nTargetValue, const CCoinControl& coin_control,
+ const CoinSelectionParams& coin_selection_params)
{
// Deduct preset inputs amount from the search target
CAmount selection_target = nTargetValue - pre_set_inputs.total_amount;
// Return if automatic coin selection is disabled, and we don't cover the selection target
- if (!coin_control.m_allow_other_inputs && selection_target > 0) return std::nullopt;
+ if (!coin_control.m_allow_other_inputs && selection_target > 0) {
+ return util::Error{_("The preselected coins total amount does not cover the transaction target. "
+ "Please allow other inputs to be automatically selected or include more coins manually")};
+ }
// Return if we can cover the target only with the preset inputs
if (selection_target <= 0) {
@@ -603,7 +610,7 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
CAmount available_coins_total_amount = coin_selection_params.m_subtract_fee_outputs ? available_coins.GetTotalAmount() :
(available_coins.GetEffectiveTotalAmount().has_value() ? *available_coins.GetEffectiveTotalAmount() : 0);
if (selection_target > available_coins_total_amount) {
- return std::nullopt; // Insufficient funds
+ return util::Error(); // Insufficient funds
}
// Start wallet Coin Selection procedure
@@ -622,7 +629,12 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a
return op_selection_result;
}
-std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params)
+struct SelectionFilter {
+ CoinEligibilityFilter filter;
+ bool allow_mixed_output_types{true};
+};
+
+util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CCoinControl& coin_control, const CoinSelectionParams& coin_selection_params)
{
unsigned int limit_ancestor_count = 0;
unsigned int limit_descendant_count = 0;
@@ -643,54 +655,56 @@ std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coi
// Coin Selection attempts to select inputs from a pool of eligible UTXOs to fund the
// transaction at a target feerate. If an attempt fails, more attempts may be made using a more
// permissive CoinEligibilityFilter.
- std::optional<SelectionResult> res = [&] {
- // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six
- // confirmations on outputs received from other wallets and only spend confirmed change.
- if (auto r1{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 6, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/false)}) return r1;
- // Allow mixing only if no solution from any single output type can be found
- if (auto r2{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(1, 1, 0), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) return r2;
-
+ util::Result<SelectionResult> res = [&] {
+ // Place coins eligibility filters on a scope increasing order.
+ std::vector<SelectionFilter> ordered_filters{
+ // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six
+ // confirmations on outputs received from other wallets and only spend confirmed change.
+ {CoinEligibilityFilter(1, 6, 0), /*allow_mixed_output_types=*/false},
+ {CoinEligibilityFilter(1, 1, 0)},
+ };
// Fall back to using zero confirmation change (but with as few ancestors in the mempool as
// possible) if we cannot fund the transaction otherwise.
if (wallet.m_spend_zero_conf_change) {
- if (auto r3{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, 2), available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) return r3;
- if (auto r4{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)),
- available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
- return r4;
- }
- if (auto r5{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2),
- available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
- return r5;
- }
+ ordered_filters.push_back({CoinEligibilityFilter(0, 1, 2)});
+ ordered_filters.push_back({CoinEligibilityFilter(0, 1, std::min(size_t{4}, max_ancestors/3), std::min(size_t{4}, max_descendants/3))});
+ ordered_filters.push_back({CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2)});
// If partial groups are allowed, relax the requirement of spending OutputGroups (groups
// of UTXOs sent to the same address, which are obviously controlled by a single wallet)
// in their entirety.
- if (auto r6{AttemptSelection(wallet, value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, /*include_partial=*/true),
- available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
- return r6;
- }
+ ordered_filters.push_back({CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, /*include_partial=*/true)});
// Try with unsafe inputs if they are allowed. This may spend unconfirmed outputs
// received from other wallets.
if (coin_control.m_include_unsafe_inputs) {
- if (auto r7{AttemptSelection(wallet, value_to_select,
- CoinEligibilityFilter(/*conf_mine=*/0, /*conf_theirs=*/0, max_ancestors-1, max_descendants-1, /*include_partial=*/true),
- available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
- return r7;
- }
+ ordered_filters.push_back({CoinEligibilityFilter(/*conf_mine=*/0, /*conf_theirs*/0, max_ancestors-1, max_descendants-1, /*include_partial=*/true)});
}
// Try with unlimited ancestors/descendants. The transaction will still need to meet
// mempool ancestor/descendant policy to be accepted to mempool and broadcasted, but
// OutputGroups use heuristics that may overestimate ancestor/descendant counts.
if (!fRejectLongChains) {
- if (auto r8{AttemptSelection(wallet, value_to_select,
- CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), std::numeric_limits<uint64_t>::max(), /*include_partial=*/true),
- available_coins, coin_selection_params, /*allow_mixed_output_types=*/true)}) {
- return r8;
- }
+ ordered_filters.push_back({CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(),
+ std::numeric_limits<uint64_t>::max(),
+ /*include_partial=*/true)});
+ }
+ }
+
+ // Walk-through the filters until the solution gets found.
+ // If no solution is found, return the first detailed error (if any).
+ // future: add "error level" so the worst one can be picked instead.
+ std::vector<util::Result<SelectionResult>> res_detailed_errors;
+ for (const auto& select_filter : ordered_filters) {
+ if (auto res{AttemptSelection(wallet, value_to_select, select_filter.filter, available_coins,
+ coin_selection_params, select_filter.allow_mixed_output_types)}) {
+ return res; // result found
+ } else {
+ // If any specific error message appears here, then something particularly wrong might have happened.
+ // Save the error and continue the selection process. So if no solutions gets found, we can return
+ // the detailed error to the upper layers.
+ if (HasErrorMsg(res)) res_detailed_errors.emplace_back(res);
}
}
// Coin Selection failed.
- return std::optional<SelectionResult>();
+ return res_detailed_errors.empty() ? util::Result<SelectionResult>(util::Error()) : res_detailed_errors.front();
}();
return res;
@@ -917,13 +931,16 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
}
// Choose coins to use
- std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
- if (!result) {
- return util::Error{_("Insufficient funds")};
+ auto select_coins_res = SelectCoins(wallet, available_coins, preset_inputs, /*nTargetValue=*/selection_target, coin_control, coin_selection_params);
+ if (!select_coins_res) {
+ // 'SelectCoins' either returns a specific error message or, if empty, means a general "Insufficient funds".
+ const bilingual_str& err = util::ErrorString(select_coins_res);
+ return util::Error{err.empty() ?_("Insufficient funds") : err};
}
- TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->GetAlgo()).c_str(), result->GetTarget(), result->GetWaste(), result->GetSelectedValue());
+ const SelectionResult& result = *select_coins_res;
+ TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result.GetAlgo()).c_str(), result.GetTarget(), result.GetWaste(), result.GetSelectedValue());
- const CAmount change_amount = result->GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee);
+ const CAmount change_amount = result.GetChange(coin_selection_params.min_viable_change, coin_selection_params.m_change_fee);
if (change_amount > 0) {
CTxOut newTxOut(change_amount, scriptChange);
if (nChangePosInOut == -1) {
@@ -938,7 +955,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
}
// Shuffle selected coins and fill in final vin
- std::vector<COutput> selected_coins = result->GetShuffledInputVector();
+ std::vector<COutput> selected_coins = result.GetShuffledInputVector();
// The sequence number is set to non-maxint so that DiscourageFeeSniping
// works.
@@ -963,7 +980,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes);
const CAmount output_value = CalculateOutputValue(txNew);
Assume(recipients_sum + change_amount == output_value);
- CAmount current_fee = result->GetSelectedValue() - output_value;
+ CAmount current_fee = result.GetSelectedValue() - output_value;
// Sanity check that the fee cannot be negative as that means we have more output value than input value
if (current_fee < 0) {
@@ -974,7 +991,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
if (nChangePosInOut != -1 && fee_needed < current_fee) {
auto& change = txNew.vout.at(nChangePosInOut);
change.nValue += current_fee - fee_needed;
- current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew);
+ current_fee = result.GetSelectedValue() - CalculateOutputValue(txNew);
if (fee_needed != current_fee) {
return util::Error{Untranslated(STR_INTERNAL_BUG("Change adjustment: Fee needed != fee paid"))};
}
@@ -1013,7 +1030,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
}
++i;
}
- current_fee = result->GetSelectedValue() - CalculateOutputValue(txNew);
+ current_fee = result.GetSelectedValue() - CalculateOutputValue(txNew);
if (fee_needed != current_fee) {
return util::Error{Untranslated(STR_INTERNAL_BUG("SFFO: Fee needed != fee paid"))};
}
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index c612ed105e..5ffdd11813 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -76,6 +76,8 @@ struct CoinFilterParams {
bool only_spendable{true};
// By default, do not include immature coinbase outputs
bool include_immature_coinbase{false};
+ // By default, skip locked UTXOs
+ bool skip_locked{true};
};
/**
@@ -119,9 +121,11 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C
* param@[in] coin_selection_params Parameters for the coin selection
* param@[in] allow_mixed_output_types Relax restriction that SelectionResults must be of the same OutputType
* returns If successful, a SelectionResult containing the input set
- * If failed, a nullopt
+ * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds")
+ * or (2) an specific error message if there was something particularly wrong (e.g. a selection
+ * result that surpassed the tx max weight size).
*/
-std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins,
+util::Result<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins,
const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types);
/**
@@ -135,9 +139,11 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm
* param@[in] available_coins The struct of coins, organized by OutputType, available for selection prior to filtering
* param@[in] coin_selection_params Parameters for the coin selection
* returns If successful, a SelectionResult containing the input set
- * If failed, a nullopt
+ * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds")
+ * or (2) an specific error message if there was something particularly wrong (e.g. a selection
+ * result that surpassed the tx max weight size).
*/
-std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins,
+util::Result<SelectionResult> ChooseSelectionResult(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const std::vector<COutput>& available_coins,
const CoinSelectionParams& coin_selection_params);
// User manually selected inputs that must be part of the transaction
@@ -175,18 +181,20 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const
* param@[in] coin_selection_params Parameters for this coin selection such as feerates, whether to avoid partial spends,
* and whether to subtract the fee from the outputs.
* returns If successful, a SelectionResult containing the selected coins
- * If failed, a nullopt.
+ * If failed, returns (1) an empty error message if the target was not reached (general "Insufficient funds")
+ * or (2) an specific error message if there was something particularly wrong (e.g. a selection
+ * result that surpassed the tx max weight size).
*/
-std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control,
+util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& nTargetValue, const CCoinControl& coin_control,
const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
/**
* Select all coins from coin_control, and if coin_control 'm_allow_other_inputs=true', call 'AutomaticCoinSelection' to
* select a set of coins such that nTargetValue - pre_set_inputs.total_amount is met.
*/
-std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
- const CAmount& nTargetValue, const CCoinControl& coin_control,
- const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
+util::Result<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& available_coins, const PreSelectedInputs& pre_set_inputs,
+ const CAmount& nTargetValue, const CCoinControl& coin_control,
+ const CoinSelectionParams& coin_selection_params) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
struct CreatedTransactionResult
{
diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp
deleted file mode 100644
index 2427a343d5..0000000000
--- a/src/wallet/test/availablecoins_tests.cpp
+++ /dev/null
@@ -1,107 +0,0 @@
-// Copyright (c) 2022 The Bitcoin Core developers
-// Distributed under the MIT software license, see the accompanying
-// file COPYING or https://www.opensource.org/licenses/mit-license.php.
-
-#include <validation.h>
-#include <wallet/coincontrol.h>
-#include <wallet/spend.h>
-#include <wallet/test/util.h>
-#include <wallet/test/wallet_test_fixture.h>
-
-#include <boost/test/unit_test.hpp>
-
-namespace wallet {
-BOOST_FIXTURE_TEST_SUITE(availablecoins_tests, WalletTestingSetup)
-class AvailableCoinsTestingSetup : public TestChain100Setup
-{
-public:
- AvailableCoinsTestingSetup()
- {
- CreateAndProcessBlock({}, {});
- wallet = CreateSyncedWallet(*m_node.chain, m_node.chainman->ActiveChain(), m_args, coinbaseKey);
- }
-
- ~AvailableCoinsTestingSetup()
- {
- wallet.reset();
- }
- CWalletTx& AddTx(CRecipient recipient)
- {
- CTransactionRef tx;
- CCoinControl dummy;
- {
- constexpr int RANDOM_CHANGE_POSITION = -1;
- auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy);
- BOOST_CHECK(res);
- tx = res->tx;
- }
- wallet->CommitTransaction(tx, {}, {});
- CMutableTransaction blocktx;
- {
- LOCK(wallet->cs_wallet);
- blocktx = CMutableTransaction(*wallet->mapWallet.at(tx->GetHash()).tx);
- }
- CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));
-
- LOCK(wallet->cs_wallet);
- LOCK(m_node.chainman->GetMutex());
- wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, m_node.chainman->ActiveChain().Tip()->GetBlockHash());
- auto it = wallet->mapWallet.find(tx->GetHash());
- BOOST_CHECK(it != wallet->mapWallet.end());
- it->second.m_state = TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1};
- return it->second;
- }
-
- std::unique_ptr<CWallet> wallet;
-};
-
-BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup)
-{
- CoinsResult available_coins;
- util::Result<CTxDestination> dest{util::Error{}};
- LOCK(wallet->cs_wallet);
-
- // Verify our wallet has one usable coinbase UTXO before starting
- // This UTXO is a P2PK, so it should show up in the Other bucket
- available_coins = AvailableCoins(*wallet);
- BOOST_CHECK_EQUAL(available_coins.Size(), 1U);
- BOOST_CHECK_EQUAL(available_coins.coins[OutputType::UNKNOWN].size(), 1U);
-
- // We will create a self transfer for each of the OutputTypes and
- // verify it is put in the correct bucket after running GetAvailablecoins
- //
- // For each OutputType, We expect 2 UTXOs in our wallet following the self transfer:
- // 1. One UTXO as the recipient
- // 2. One UTXO from the change, due to payment address matching logic
-
- // Bech32m
- dest = wallet->GetNewDestination(OutputType::BECH32M, "");
- BOOST_ASSERT(dest);
- AddTx(CRecipient{{GetScriptForDestination(*dest)}, 1 * COIN, /*fSubtractFeeFromAmount=*/true});
- available_coins = AvailableCoins(*wallet);
- BOOST_CHECK_EQUAL(available_coins.coins[OutputType::BECH32M].size(), 2U);
-
- // Bech32
- dest = wallet->GetNewDestination(OutputType::BECH32, "");
- BOOST_ASSERT(dest);
- AddTx(CRecipient{{GetScriptForDestination(*dest)}, 2 * COIN, /*fSubtractFeeFromAmount=*/true});
- available_coins = AvailableCoins(*wallet);
- BOOST_CHECK_EQUAL(available_coins.coins[OutputType::BECH32].size(), 2U);
-
- // P2SH-SEGWIT
- dest = wallet->GetNewDestination(OutputType::P2SH_SEGWIT, "");
- BOOST_ASSERT(dest);
- AddTx(CRecipient{{GetScriptForDestination(*dest)}, 3 * COIN, /*fSubtractFeeFromAmount=*/true});
- available_coins = AvailableCoins(*wallet);
- BOOST_CHECK_EQUAL(available_coins.coins[OutputType::P2SH_SEGWIT].size(), 2U);
-
- // Legacy (P2PKH)
- dest = wallet->GetNewDestination(OutputType::LEGACY, "");
- BOOST_ASSERT(dest);
- AddTx(CRecipient{{GetScriptForDestination(*dest)}, 4 * COIN, /*fSubtractFeeFromAmount=*/true});
- available_coins = AvailableCoins(*wallet);
- BOOST_CHECK_EQUAL(available_coins.coins[OutputType::LEGACY].size(), 2U);
-}
-
-BOOST_AUTO_TEST_SUITE_END()
-} // namespace wallet
diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp
index fdd56101b4..2e12b5b1d4 100644
--- a/src/wallet/test/coinselector_tests.cpp
+++ b/src/wallet/test/coinselector_tests.cpp
@@ -121,9 +121,9 @@ static CAmount make_hard_case(int utxos, std::vector<COutput>& utxo_pool)
utxo_pool.clear();
CAmount target = 0;
for (int i = 0; i < utxos; ++i) {
- target += (CAmount)1 << (utxos+i);
- add_coin((CAmount)1 << (utxos+i), 2*i, utxo_pool);
- add_coin(((CAmount)1 << (utxos+i)) + ((CAmount)1 << (utxos-1-i)), 2*i + 1, utxo_pool);
+ target += CAmount{1} << (utxos+i);
+ add_coin(CAmount{1} << (utxos+i), 2*i, utxo_pool);
+ add_coin((CAmount{1} << (utxos+i)) + (CAmount{1} << (utxos-1-i)), 2*i + 1, utxo_pool);
}
return target;
}
@@ -302,6 +302,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
coin_selection_params_bnb.m_change_fee = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_output_size);
coin_selection_params_bnb.m_cost_of_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size) + coin_selection_params_bnb.m_change_fee;
coin_selection_params_bnb.min_viable_change = coin_selection_params_bnb.m_effective_feerate.GetFee(coin_selection_params_bnb.change_spend_size);
+ coin_selection_params_bnb.m_subtract_fee_outputs = true;
+
{
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase());
wallet->LoadWallet();
@@ -319,7 +321,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test)
available_coins.Clear();
add_coin(available_coins, *wallet, 1 * CENT, coin_selection_params_bnb.m_effective_feerate);
available_coins.All().at(0).input_bytes = 40;
- coin_selection_params_bnb.m_subtract_fee_outputs = true;
const auto result9 = SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change);
BOOST_CHECK(result9);
BOOST_CHECK_EQUAL(result9->GetSelectedValue(), 1 * CENT);
@@ -931,7 +932,7 @@ BOOST_AUTO_TEST_CASE(effective_value_test)
BOOST_CHECK_EQUAL(output5.GetEffectiveValue(), nValue); // The effective value should be equal to the absolute value if input_bytes is -1
}
-static std::optional<SelectionResult> select_coins(const CAmount& target, const CoinSelectionParams& cs_params, const CCoinControl& cc, std::function<CoinsResult(CWallet&)> coin_setup, interfaces::Chain* chain, const ArgsManager& args)
+static util::Result<SelectionResult> select_coins(const CAmount& target, const CoinSelectionParams& cs_params, const CCoinControl& cc, std::function<CoinsResult(CWallet&)> coin_setup, interfaces::Chain* chain, const ArgsManager& args)
{
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(chain, "", args, CreateMockWalletDatabase());
wallet->LoadWallet();
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 7ae10c23ca..b6e50e961a 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -622,6 +622,46 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup)
BOOST_CHECK_EQUAL(list.begin()->second.size(), 2U);
}
+void TestCoinsResult(ListCoinsTest& context, OutputType out_type, CAmount amount,
+ std::map<OutputType, size_t>& expected_coins_sizes)
+{
+ LOCK(context.wallet->cs_wallet);
+ util::Result<CTxDestination> dest = Assert(context.wallet->GetNewDestination(out_type, ""));
+ CWalletTx& wtx = context.AddTx(CRecipient{{GetScriptForDestination(*dest)}, amount, /*fSubtractFeeFromAmount=*/true});
+ CoinFilterParams filter;
+ filter.skip_locked = false;
+ CoinsResult available_coins = AvailableCoins(*context.wallet, nullptr, std::nullopt, filter);
+ // Lock outputs so they are not spent in follow-up transactions
+ for (uint32_t i = 0; i < wtx.tx->vout.size(); i++) context.wallet->LockCoin({wtx.GetHash(), i});
+ for (const auto& [type, size] : expected_coins_sizes) BOOST_CHECK_EQUAL(size, available_coins.coins[type].size());
+}
+
+BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, ListCoinsTest)
+{
+ std::map<OutputType, size_t> expected_coins_sizes;
+ for (const auto& out_type : OUTPUT_TYPES) { expected_coins_sizes[out_type] = 0U; }
+
+ // Verify our wallet has one usable coinbase UTXO before starting
+ // This UTXO is a P2PK, so it should show up in the Other bucket
+ expected_coins_sizes[OutputType::UNKNOWN] = 1U;
+ CoinsResult available_coins = WITH_LOCK(wallet->cs_wallet, return AvailableCoins(*wallet));
+ BOOST_CHECK_EQUAL(available_coins.Size(), expected_coins_sizes[OutputType::UNKNOWN]);
+ BOOST_CHECK_EQUAL(available_coins.coins[OutputType::UNKNOWN].size(), expected_coins_sizes[OutputType::UNKNOWN]);
+
+ // We will create a self transfer for each of the OutputTypes and
+ // verify it is put in the correct bucket after running GetAvailablecoins
+ //
+ // For each OutputType, We expect 2 UTXOs in our wallet following the self transfer:
+ // 1. One UTXO as the recipient
+ // 2. One UTXO from the change, due to payment address matching logic
+
+ for (const auto& out_type : OUTPUT_TYPES) {
+ if (out_type == OutputType::UNKNOWN) continue;
+ expected_coins_sizes[out_type] = 2U;
+ TestCoinsResult(*this, out_type, 1 * COIN, expected_coins_sizes);
+ }
+}
+
BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup)
{
{
@@ -696,10 +736,10 @@ BOOST_FIXTURE_TEST_CASE(wallet_descriptor_test, BasicTestingSetup)
std::vector<unsigned char> malformed_record;
CVectorWriter vw(0, 0, malformed_record, 0);
vw << std::string("notadescriptor");
- vw << (uint64_t)0;
- vw << (int32_t)0;
- vw << (int32_t)0;
- vw << (int32_t)1;
+ vw << uint64_t{0};
+ vw << int32_t{0};
+ vw << int32_t{0};
+ vw << int32_t{1};
SpanReader vr{0, 0, malformed_record};
WalletDescriptor w_desc;
@@ -946,7 +986,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup)
mtx.vin.clear();
mtx.vin.push_back(CTxIn(tx_id_to_spend, 0));
- wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0);
+ wallet.transactionAddedToMempool(MakeTransactionRef(mtx));
const uint256& good_tx_id = mtx.GetHash();
{
@@ -967,7 +1007,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup)
static_cast<FailDatabase&>(wallet.GetDatabase()).m_pass = false;
mtx.vin.clear();
mtx.vin.push_back(CTxIn(good_tx_id, 0));
- BOOST_CHECK_EXCEPTION(wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0),
+ BOOST_CHECK_EXCEPTION(wallet.transactionAddedToMempool(MakeTransactionRef(mtx)),
std::runtime_error,
HasReason("DB error adding transaction to wallet, write failed"));
}
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 13b92431a6..0f761fb01a 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -472,8 +472,7 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& b
error += strprintf(Untranslated("Unexpected exception: %s"), e.what());
}
if (!wallet) {
- fs::remove(wallet_file);
- fs::remove(wallet_path);
+ fs::remove_all(wallet_path);
}
return wallet;
@@ -1355,7 +1354,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const SyncTxState& sta
MarkInputsDirty(ptx);
}
-void CWallet::transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) {
+void CWallet::transactionAddedToMempool(const CTransactionRef& tx) {
LOCK(cs_wallet);
SyncTransaction(tx, TxStateInMempool{});
@@ -1365,7 +1364,7 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& tx, uint64_t memp
}
}
-void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) {
+void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {
LOCK(cs_wallet);
auto it = mapWallet.find(tx->GetHash());
if (it != mapWallet.end()) {
@@ -1411,7 +1410,7 @@ void CWallet::blockConnected(const interfaces::BlockInfo& block)
m_last_block_processed = block.hash;
for (size_t index = 0; index < block.data->vtx.size(); index++) {
SyncTransaction(block.data->vtx[index], TxStateConfirmed{block.hash, block.height, static_cast<int>(index)});
- transactionRemovedFromMempool(block.data->vtx[index], MemPoolRemovalReason::BLOCK, /*mempool_sequence=*/0);
+ transactionRemovedFromMempool(block.data->vtx[index], MemPoolRemovalReason::BLOCK);
}
}
@@ -3184,6 +3183,24 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
if (tip_height && *tip_height != rescan_height)
{
+ // No need to read and scan block if block was created before
+ // our wallet birthday (as adjusted for block time variability)
+ std::optional<int64_t> time_first_key;
+ for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) {
+ int64_t time = spk_man->GetTimeFirstKey();
+ if (!time_first_key || time < *time_first_key) time_first_key = time;
+ }
+ if (time_first_key) {
+ FoundBlock found = FoundBlock().height(rescan_height);
+ chain.findFirstBlockWithTimeAndHeight(*time_first_key - TIMESTAMP_WINDOW, rescan_height, found);
+ if (!found.found) {
+ // We were unable to find a block that had a time more recent than our earliest timestamp
+ // or a height higher than the wallet was synced to, indicating that the wallet is newer than the
+ // current chain tip. Skip rescanning in this case.
+ rescan_height = *tip_height;
+ }
+ }
+
// Technically we could execute the code below in any case, but performing the
// `while` loop below can make startup very slow, so only check blocks on disk
// if necessary.
@@ -3218,17 +3235,6 @@ bool CWallet::AttachChain(const std::shared_ptr<CWallet>& walletInstance, interf
chain.initMessage(_("Rescanning…").translated);
walletInstance->WalletLogPrintf("Rescanning last %i blocks (from block %i)...\n", *tip_height - rescan_height, rescan_height);
- // No need to read and scan block if block was created before
- // our wallet birthday (as adjusted for block time variability)
- std::optional<int64_t> time_first_key;
- for (auto spk_man : walletInstance->GetAllScriptPubKeyMans()) {
- int64_t time = spk_man->GetTimeFirstKey();
- if (!time_first_key || time < *time_first_key) time_first_key = time;
- }
- if (time_first_key) {
- chain.findFirstBlockWithTimeAndHeight(*time_first_key - TIMESTAMP_WINDOW, rescan_height, FoundBlock().height(rescan_height));
- }
-
{
WalletRescanReserver reserver(*walletInstance);
if (!reserver.reserve() || (ScanResult::SUCCESS != walletInstance->ScanForWalletTransactions(chain.getBlockHash(rescan_height), rescan_height, /*max_height=*/{}, reserver, /*fUpdate=*/true, /*save_progress=*/true).status)) {
@@ -4002,6 +4008,23 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
}
}
}
+
+ // Persist added address book entries (labels, purpose) for watchonly and solvable wallets
+ auto persist_address_book = [](const CWallet& wallet) {
+ LOCK(wallet.cs_wallet);
+ 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 (!label.empty()) batch.WriteName(address, label);
+ }
+ };
+ if (data.watchonly_wallet) persist_address_book(*data.watchonly_wallet);
+ if (data.solvable_wallet) persist_address_book(*data.solvable_wallet);
+
// Remove the things to delete
if (dests_to_delete.size() > 0) {
for (const auto& dest : dests_to_delete) {
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 57b5f7fc31..8b7e6dd526 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -516,7 +516,7 @@ public:
*/
CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true, bool rescanning_old_block = false);
bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
- void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override;
+ void transactionAddedToMempool(const CTransactionRef& tx) override;
void blockConnected(const interfaces::BlockInfo& block) override;
void blockDisconnected(const interfaces::BlockInfo& block) override;
void updatedBlockTip() override;
@@ -538,7 +538,7 @@ public:
uint256 last_failed_block;
};
ScanResult ScanForWalletTransactions(const uint256& start_block, int start_height, std::optional<int> max_height, const WalletRescanReserver& reserver, bool fUpdate, const bool save_progress);
- void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override;
+ void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override;
/** Set the next time this wallet should resend transactions to 12-36 hours from now, ~1 day on average. */
void SetNextResend() { m_next_resend = GetDefaultNextResend(); }
/** Return true if all conditions for periodically resending transactions are met. */
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index d560de863a..b393c35112 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -974,7 +974,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
return result;
}
-DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWalletTx>& vWtx)
+DBErrors WalletBatch::FindWalletTxHashes(std::vector<uint256>& tx_hashes)
{
DBErrors result = DBErrors::LOAD_OK;
@@ -1012,9 +1012,7 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWal
if (strType == DBKeys::TX) {
uint256 hash;
ssKey >> hash;
- vTxHash.push_back(hash);
- vWtx.emplace_back(/*tx=*/nullptr, TxStateInactive{});
- ssValue >> vWtx.back();
+ tx_hashes.push_back(hash);
}
}
} catch (...) {
@@ -1027,10 +1025,9 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWal
DBErrors WalletBatch::ZapSelectTx(std::vector<uint256>& vTxHashIn, std::vector<uint256>& vTxHashOut)
{
- // build list of wallet TXs and hashes
+ // build list of wallet TX hashes
std::vector<uint256> vTxHash;
- std::list<CWalletTx> vWtx;
- DBErrors err = FindWalletTx(vTxHash, vWtx);
+ DBErrors err = FindWalletTxHashes(vTxHash);
if (err != DBErrors::LOAD_OK) {
return err;
}
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index c0d9de8bb3..97e8fad278 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -273,7 +273,7 @@ public:
bool EraseActiveScriptPubKeyMan(uint8_t type, bool internal);
DBErrors LoadWallet(CWallet* pwallet);
- DBErrors FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWalletTx>& vWtx);
+ DBErrors FindWalletTxHashes(std::vector<uint256>& tx_hashes);
DBErrors ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut);
/* Function to determine if a certain KV/key-type is a key (cryptographical key) type */
static bool IsKeyType(const std::string& strType);
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-06 08:26:02 +0000