diff options
Diffstat (limited to 'src/wallet')
45 files changed, 1556 insertions, 1012 deletions
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 06650a73cc..4d3285333f 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -8,6 +8,7 @@ #include <wallet/bdb.h> #include <wallet/db.h> +#include <util/check.h> #include <util/strencodings.h> #include <util/translation.h> @@ -100,7 +101,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); @@ -220,17 +221,17 @@ BerkeleyEnvironment::BerkeleyEnvironment() : m_use_shared_memory(false) fMockDb = true; } -BerkeleyBatch::SafeDbt::SafeDbt() +SafeDbt::SafeDbt() { m_dbt.set_flags(DB_DBT_MALLOC); } -BerkeleyBatch::SafeDbt::SafeDbt(void* data, size_t size) +SafeDbt::SafeDbt(void* data, size_t size) : m_dbt(data, size) { } -BerkeleyBatch::SafeDbt::~SafeDbt() +SafeDbt::~SafeDbt() { if (m_dbt.get_data() != nullptr) { // Clear memory, e.g. in case it was a private key @@ -244,17 +245,17 @@ BerkeleyBatch::SafeDbt::~SafeDbt() } } -const void* BerkeleyBatch::SafeDbt::get_data() const +const void* SafeDbt::get_data() const { return m_dbt.get_data(); } -uint32_t BerkeleyBatch::SafeDbt::get_size() const +uint32_t SafeDbt::get_size() const { return m_dbt.get_size(); } -BerkeleyBatch::SafeDbt::operator Dbt*() +SafeDbt::operator Dbt*() { return &m_dbt; } @@ -307,7 +308,7 @@ BerkeleyDatabase::~BerkeleyDatabase() } } -BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_cursor(nullptr), m_database(database) +BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, bool fFlushOnCloseIn) : m_database(database) { database.AddRef(); database.Open(); @@ -398,7 +399,6 @@ void BerkeleyBatch::Close() activeTxn->abort(); activeTxn = nullptr; pdb = nullptr; - CloseCursor(); if (fFlushOnClose) Flush(); @@ -432,6 +432,7 @@ void BerkeleyEnvironment::ReloadDbEnv() }); std::vector<fs::path> filenames; + filenames.reserve(m_databases.size()); for (const auto& it : m_databases) { filenames.push_back(it.first); } @@ -476,15 +477,15 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) fSuccess = false; } - if (db.StartCursor()) { + std::unique_ptr<DatabaseCursor> cursor = db.GetNewCursor(); + if (cursor) { while (fSuccess) { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret1 = db.ReadAtCursor(ssKey, ssValue, complete); - if (complete) { + DataStream ssKey{}; + DataStream ssValue{}; + DatabaseCursor::Status ret1 = cursor->Next(ssKey, ssValue); + if (ret1 == DatabaseCursor::Status::DONE) { break; - } else if (!ret1) { + } else if (ret1 == DatabaseCursor::Status::FAIL) { fSuccess = false; break; } @@ -502,7 +503,7 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) if (ret2 > 0) fSuccess = false; } - db.CloseCursor(); + cursor.reset(); } if (fSuccess) { db.Close(); @@ -656,48 +657,52 @@ void BerkeleyDatabase::ReloadDbEnv() env->ReloadDbEnv(); } -bool BerkeleyBatch::StartCursor() +BerkeleyCursor::BerkeleyCursor(BerkeleyDatabase& database) { - assert(!m_cursor); - if (!pdb) - return false; - int ret = pdb->cursor(nullptr, &m_cursor, 0); - return ret == 0; + 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); + if (ret != 0) { + throw std::runtime_error(STR_INTERNAL_BUG(strprintf("BDB Cursor could not be created. Returned %d", ret))); + } } -bool BerkeleyBatch::ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) +DatabaseCursor::Status BerkeleyCursor::Next(DataStream& ssKey, DataStream& ssValue) { - complete = false; - if (m_cursor == nullptr) return false; + if (m_cursor == nullptr) return Status::FAIL; // Read at cursor SafeDbt datKey; SafeDbt datValue; int ret = m_cursor->get(datKey, datValue, DB_NEXT); if (ret == DB_NOTFOUND) { - complete = true; + return Status::DONE; + } + if (ret != 0 || datKey.get_data() == nullptr || datValue.get_data() == nullptr) { + return Status::FAIL; } - if (ret != 0) - return false; - else if (datKey.get_data() == nullptr || datValue.get_data() == nullptr) - return false; // Convert to streams - ssKey.SetType(SER_DISK); ssKey.clear(); ssKey.write({AsBytePtr(datKey.get_data()), datKey.get_size()}); - ssValue.SetType(SER_DISK); ssValue.clear(); ssValue.write({AsBytePtr(datValue.get_data()), datValue.get_size()}); - return true; + return Status::MORE; } -void BerkeleyBatch::CloseCursor() +BerkeleyCursor::~BerkeleyCursor() { if (!m_cursor) return; m_cursor->close(); m_cursor = nullptr; } +std::unique_ptr<DatabaseCursor> BerkeleyBatch::GetNewCursor() +{ + if (!pdb) return nullptr; + return std::make_unique<BerkeleyCursor>(m_database); +} + bool BerkeleyBatch::TxnBegin() { if (!pdb || activeTxn) @@ -749,7 +754,7 @@ std::string BerkeleyDatabaseVersion() return DbEnv::version(nullptr, nullptr, nullptr); } -bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value) +bool BerkeleyBatch::ReadKey(DataStream&& key, DataStream& value) { if (!pdb) return false; @@ -765,7 +770,7 @@ bool BerkeleyBatch::ReadKey(CDataStream&& key, CDataStream& value) return false; } -bool BerkeleyBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite) +bool BerkeleyBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite) { if (!pdb) return false; @@ -780,7 +785,7 @@ bool BerkeleyBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwr return (ret == 0); } -bool BerkeleyBatch::EraseKey(CDataStream&& key) +bool BerkeleyBatch::EraseKey(DataStream&& key) { if (!pdb) return false; @@ -793,7 +798,7 @@ bool BerkeleyBatch::EraseKey(CDataStream&& key) return (ret == 0 || ret == DB_NOTFOUND); } -bool BerkeleyBatch::HasKey(CDataStream&& key) +bool BerkeleyBatch::HasKey(DataStream&& key) { if (!pdb) return false; diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index 40a1031c8e..06c98972b0 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -165,40 +165,51 @@ public: std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override; }; -/** RAII class that provides access to a Berkeley database */ -class BerkeleyBatch : public DatabaseBatch +/** RAII class that automatically cleanses its data on destruction */ +class SafeDbt final { - /** RAII class that automatically cleanses its data on destruction */ - class SafeDbt final - { - Dbt m_dbt; + Dbt m_dbt; - public: - // construct Dbt with internally-managed data - SafeDbt(); - // construct Dbt with provided data - SafeDbt(void* data, size_t size); - ~SafeDbt(); +public: + // construct Dbt with internally-managed data + SafeDbt(); + // construct Dbt with provided data + SafeDbt(void* data, size_t size); + ~SafeDbt(); + + // delegate to Dbt + const void* get_data() const; + uint32_t get_size() const; + + // conversion operator to access the underlying Dbt + operator Dbt*(); +}; - // delegate to Dbt - const void* get_data() const; - uint32_t get_size() const; +class BerkeleyCursor : public DatabaseCursor +{ +private: + Dbc* m_cursor; - // conversion operator to access the underlying Dbt - operator Dbt*(); - }; +public: + explicit BerkeleyCursor(BerkeleyDatabase& database); + ~BerkeleyCursor() override; + + Status Next(DataStream& key, DataStream& value) override; +}; +/** RAII class that provides access to a Berkeley database */ +class BerkeleyBatch : public DatabaseBatch +{ private: - bool ReadKey(CDataStream&& key, CDataStream& value) override; - bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true) override; - bool EraseKey(CDataStream&& key) override; - bool HasKey(CDataStream&& key) override; + 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; protected: - Db* pdb; + Db* pdb{nullptr}; std::string strFile; - DbTxn* activeTxn; - Dbc* m_cursor; + DbTxn* activeTxn{nullptr}; bool fReadOnly; bool fFlushOnClose; BerkeleyEnvironment *env; @@ -214,9 +225,7 @@ public: void Flush() override; void Close() override; - bool StartCursor() override; - bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override; - void CloseCursor() override; + std::unique_ptr<DatabaseCursor> GetNewCursor() override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 4fba567b7f..9cf61dbe51 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 @@ -331,12 +333,9 @@ std::optional<SelectionResult> KnapsackSolver(std::vector<OutputGroup>& groups, ******************************************************************************/ -void OutputGroup::Insert(const COutput& output, size_t ancestors, size_t descendants, bool positive_only) { - // Filter for positive only here before adding the coin - if (positive_only && output.GetEffectiveValue() <= 0) return; - +void OutputGroup::Insert(const std::shared_ptr<COutput>& output, size_t ancestors, size_t descendants) { m_outputs.push_back(output); - COutput& coin = m_outputs.back(); + auto& coin = *m_outputs.back(); fee += coin.GetFee(); @@ -356,8 +355,8 @@ void OutputGroup::Insert(const COutput& output, size_t ancestors, size_t descend // coin itself; thus, this value is counted as the max, not the sum m_descendants = std::max(m_descendants, descendants); - if (output.input_bytes > 0) { - m_weight += output.input_bytes * WITNESS_SCALE_FACTOR; + if (output->input_bytes > 0) { + m_weight += output->input_bytes * WITNESS_SCALE_FACTOR; } } @@ -373,7 +372,22 @@ CAmount OutputGroup::GetSelectionAmount() const return m_subtract_fee_outputs ? m_value : effective_value; } -CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, CAmount target, bool use_effective_value) +void OutputGroupTypeMap::Push(const OutputGroup& group, OutputType type, bool insert_positive, bool insert_mixed) +{ + if (group.m_outputs.empty()) return; + + Groups& groups = groups_by_type[type]; + if (insert_positive && group.GetSelectionAmount() > 0) { + groups.positive_group.emplace_back(group); + all_groups.positive_group.emplace_back(group); + } + if (insert_mixed) { + groups.mixed_group.emplace_back(group); + all_groups.mixed_group.emplace_back(group); + } +} + +CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value) { // This function should not be called with empty inputs as that would mean the selection failed assert(!inputs.empty()); @@ -381,7 +395,8 @@ CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, // Always consider the cost of spending an input now vs in the future. CAmount waste = 0; CAmount selected_effective_value = 0; - for (const COutput& coin : inputs) { + for (const auto& coin_ptr : inputs) { + const COutput& coin = *coin_ptr; waste += coin.GetFee() - coin.long_term_fee; selected_effective_value += use_effective_value ? coin.GetEffectiveValue() : coin.txout.nValue; } @@ -429,12 +444,12 @@ CAmount SelectionResult::GetWaste() const CAmount SelectionResult::GetSelectedValue() const { - return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin.txout.nValue; }); + return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin->txout.nValue; }); } CAmount SelectionResult::GetSelectedEffectiveValue() const { - return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin.GetEffectiveValue(); }); + return std::accumulate(m_selected_inputs.cbegin(), m_selected_inputs.cend(), CAmount{0}, [](CAmount sum, const auto& coin) { return sum + coin->GetEffectiveValue(); }); } void SelectionResult::Clear() @@ -446,46 +461,46 @@ 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; } -void SelectionResult::AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs) +void SelectionResult::AddInputs(const std::set<std::shared_ptr<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) { - return sum + std::max(coin.input_bytes, 0) * WITNESS_SCALE_FACTOR; + return sum + std::max(coin->input_bytes, 0) * WITNESS_SCALE_FACTOR; }); } 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; } -const std::set<COutput>& SelectionResult::GetInputSet() const +const std::set<std::shared_ptr<COutput>>& SelectionResult::GetInputSet() const { return m_selected_inputs; } -std::vector<COutput> SelectionResult::GetShuffledInputVector() const +std::vector<std::shared_ptr<COutput>> SelectionResult::GetShuffledInputVector() const { - std::vector<COutput> coins(m_selected_inputs.begin(), m_selected_inputs.end()); + std::vector<std::shared_ptr<COutput>> coins(m_selected_inputs.begin(), m_selected_inputs.end()); Shuffle(coins.begin(), coins.end(), FastRandomContext()); return coins; } diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 2efeb8476d..5a7b748be1 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -7,9 +7,12 @@ #include <consensus/amount.h> #include <consensus/consensus.h> +#include <outputtype.h> #include <policy/feerate.h> #include <primitives/transaction.h> #include <random.h> +#include <util/system.h> +#include <util/check.h> #include <optional> @@ -150,6 +153,11 @@ struct CoinSelectionParams { * associated with the same address. This helps reduce privacy leaks resulting from address * reuse. Dust outputs are not eligible to be added to output groups and thus not considered. */ bool m_avoid_partial_spends = false; + /** + * When true, allow unsafe coins to be selected during Coin Selection. This may spend unconfirmed outputs: + * 1) Received from other wallets, 2) replacing other txs, 3) that have been replaced. + */ + bool m_include_unsafe_inputs = false; CoinSelectionParams(FastRandomContext& rng_fast, size_t change_output_size, size_t change_spend_size, CAmount min_change_target, CFeeRate effective_feerate, @@ -186,16 +194,22 @@ 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) {} + + bool operator<(const CoinEligibilityFilter& other) const { + return std::tie(conf_mine, conf_theirs, max_ancestors, max_descendants, m_include_partial_groups) + < std::tie(other.conf_mine, other.conf_theirs, other.max_ancestors, other.max_descendants, other.m_include_partial_groups); + } }; /** A group of UTXOs paid to the same output script. */ struct OutputGroup { /** The list of UTXOs contained in this output group. */ - std::vector<COutput> m_outputs; + std::vector<std::shared_ptr<COutput>> m_outputs; /** Whether the UTXOs were sent by the wallet to itself. This is relevant because we may want at * least a certain number of confirmations on UTXOs received from outside wallets while trusting * our own UTXOs more. */ @@ -213,8 +227,6 @@ struct OutputGroup CAmount effective_value{0}; /** The fee to spend these UTXOs at the effective feerate. */ CAmount fee{0}; - /** The target feerate of the transaction we're trying to build. */ - CFeeRate m_effective_feerate{0}; /** The fee to spend these UTXOs at the long term feerate. */ CAmount long_term_fee{0}; /** The feerate for spending a created change output eventually (i.e. not urgently, and thus at @@ -229,16 +241,39 @@ struct OutputGroup OutputGroup() {} OutputGroup(const CoinSelectionParams& params) : - m_effective_feerate(params.m_effective_feerate), m_long_term_feerate(params.m_long_term_feerate), m_subtract_fee_outputs(params.m_subtract_fee_outputs) {} - void Insert(const COutput& output, size_t ancestors, size_t descendants, bool positive_only); + void Insert(const std::shared_ptr<COutput>& output, size_t ancestors, size_t descendants); bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; CAmount GetSelectionAmount() const; }; +struct Groups { + // Stores 'OutputGroup' containing only positive UTXOs (value > 0). + std::vector<OutputGroup> positive_group; + // Stores 'OutputGroup' which may contain both positive and negative UTXOs. + std::vector<OutputGroup> mixed_group; +}; + +/** Stores several 'Groups' whose were mapped by output type. */ +struct OutputGroupTypeMap +{ + // Maps output type to output groups. + std::map<OutputType, Groups> groups_by_type; + // All inserted groups, no type distinction. + Groups all_groups; + + // Based on the insert flag; appends group to the 'mixed_group' and, if value > 0, to the 'positive_group'. + // This affects both; the groups filtered by type and the overall groups container. + void Push(const OutputGroup& group, OutputType type, bool insert_positive, bool insert_mixed); + // Different output types count + size_t TypesCount() { return groups_by_type.size(); } +}; + +typedef std::map<CoinEligibilityFilter, OutputGroupTypeMap> FilteredOutputGroups; + /** Compute the waste for this result given the cost of change * and the opportunity cost of spending these inputs now vs in the future. * If change exists, waste = change_cost + inputs * (effective_feerate - long_term_feerate) @@ -256,7 +291,7 @@ struct OutputGroup * @param[in] use_effective_value Whether to use the input's effective value (when true) or the real value (when false). * @return The waste */ -[[nodiscard]] CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); +[[nodiscard]] CAmount GetSelectionWaste(const std::set<std::shared_ptr<COutput>>& inputs, CAmount change_cost, CAmount target, bool use_effective_value = true); /** Choose a random change target for each transaction to make it harder to fingerprint the Core @@ -289,7 +324,7 @@ struct SelectionResult { private: /** Set of inputs selected by the algorithm to use in the transaction */ - std::set<COutput> m_selected_inputs; + std::set<std::shared_ptr<COutput>> m_selected_inputs; /** The target the algorithm selected for. Equal to the recipient amount plus non-input fees */ CAmount m_target; /** The algorithm used to produce this result */ @@ -301,6 +336,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) {} @@ -315,7 +361,7 @@ public: void Clear(); void AddInput(const OutputGroup& group); - void AddInputs(const std::set<COutput>& inputs, bool subtract_fee_outputs); + void AddInputs(const std::set<std::shared_ptr<COutput>>& inputs, bool subtract_fee_outputs); /** Calculates and stores the waste for this selection via GetSelectionWaste */ void ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee); @@ -330,9 +376,9 @@ public: void Merge(const SelectionResult& other); /** Get m_selected_inputs */ - const std::set<COutput>& GetInputSet() const; + const std::set<std::shared_ptr<COutput>>& GetInputSet() const; /** Get the vector of COutputs that will be used to fill in a CTransaction's vin */ - std::vector<COutput> GetShuffledInputVector() const; + std::vector<std::shared_ptr<COutput>> GetShuffledInputVector() const; bool operator<(SelectionResult other) const; diff --git a/src/wallet/db.h b/src/wallet/db.h index f09844c37e..d4c590fac7 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -22,14 +22,33 @@ struct bilingual_str; namespace wallet { void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename); +class DatabaseCursor +{ +public: + explicit DatabaseCursor() {} + virtual ~DatabaseCursor() {} + + DatabaseCursor(const DatabaseCursor&) = delete; + DatabaseCursor& operator=(const DatabaseCursor&) = delete; + + enum class Status + { + FAIL, + MORE, + DONE, + }; + + virtual Status Next(DataStream& key, DataStream& value) { return Status::FAIL; } +}; + /** RAII class that provides access to a WalletDatabase */ class DatabaseBatch { private: - virtual bool ReadKey(CDataStream&& key, CDataStream& value) = 0; - virtual bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite=true) = 0; - virtual bool EraseKey(CDataStream&& key) = 0; - virtual bool HasKey(CDataStream&& key) = 0; + virtual bool ReadKey(DataStream&& key, DataStream& value) = 0; + virtual bool WriteKey(DataStream&& key, DataStream&& value, bool overwrite = true) = 0; + virtual bool EraseKey(DataStream&& key) = 0; + virtual bool HasKey(DataStream&& key) = 0; public: explicit DatabaseBatch() {} @@ -44,7 +63,7 @@ public: template <typename K, typename T> bool Read(const K& key, T& value) { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); + DataStream ssKey{}; ssKey.reserve(1000); ssKey << key; @@ -61,7 +80,7 @@ public: template <typename K, typename T> bool Write(const K& key, const T& value, bool fOverwrite = true) { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); + DataStream ssKey{}; ssKey.reserve(1000); ssKey << key; @@ -75,7 +94,7 @@ public: template <typename K> bool Erase(const K& key) { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); + DataStream ssKey{}; ssKey.reserve(1000); ssKey << key; @@ -85,16 +104,14 @@ public: template <typename K> bool Exists(const K& key) { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); + DataStream ssKey{}; ssKey.reserve(1000); ssKey << key; return HasKey(std::move(ssKey)); } - virtual bool StartCursor() = 0; - virtual bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) = 0; - virtual void CloseCursor() = 0; + virtual std::unique_ptr<DatabaseCursor> GetNewCursor() = 0; virtual bool TxnBegin() = 0; virtual bool TxnCommit() = 0; virtual bool TxnAbort() = 0; @@ -106,7 +123,7 @@ class WalletDatabase { public: /** Create dummy DB handle */ - WalletDatabase() : nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0) {} + WalletDatabase() : nUpdateCounter(0) {} virtual ~WalletDatabase() {}; /** Open the database if it is not already opened. */ @@ -148,30 +165,33 @@ public: virtual std::string Format() = 0; std::atomic<unsigned int> nUpdateCounter; - unsigned int nLastSeen; - unsigned int nLastFlushed; - int64_t nLastWalletUpdate; + unsigned int nLastSeen{0}; + unsigned int nLastFlushed{0}; + int64_t nLastWalletUpdate{0}; /** Make a DatabaseBatch connected to this database */ virtual std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) = 0; }; +class DummyCursor : public DatabaseCursor +{ + Status Next(DataStream& key, DataStream& value) override { return Status::FAIL; } +}; + /** RAII class that provides access to a DummyDatabase. Never fails. */ class DummyBatch : public DatabaseBatch { private: - bool ReadKey(CDataStream&& key, CDataStream& value) override { return true; } - bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite=true) override { return true; } - bool EraseKey(CDataStream&& key) override { return true; } - bool HasKey(CDataStream&& key) override { return true; } + bool ReadKey(DataStream&& key, DataStream& value) override { return true; } + 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; } public: void Flush() override {} void Close() override {} - bool StartCursor() override { return true; } - bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return true; } - void CloseCursor() override {} + std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<DummyCursor>(); } bool TxnBegin() override { return true; } bool TxnCommit() override { return true; } bool TxnAbort() override { return true; } diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index efa548ad91..69208c19dc 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -5,6 +5,7 @@ #include <wallet/dump.h> #include <fs.h> +#include <util/system.h> #include <util/translation.h> #include <wallet/wallet.h> @@ -47,7 +48,8 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) std::unique_ptr<DatabaseBatch> batch = db.MakeBatch(); bool ret = true; - if (!batch->StartCursor()) { + std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor(); + if (!cursor) { error = _("Error: Couldn't create cursor into database"); ret = false; } @@ -66,15 +68,15 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) // Read the records while (true) { - CDataStream ss_key(SER_DISK, CLIENT_VERSION); - CDataStream ss_value(SER_DISK, CLIENT_VERSION); - bool complete; - ret = batch->ReadAtCursor(ss_key, ss_value, complete); - if (complete) { + DataStream ss_key{}; + DataStream ss_value{}; + DatabaseCursor::Status status = cursor->Next(ss_key, ss_value); + if (status == DatabaseCursor::Status::DONE) { ret = true; break; - } else if (!ret) { + } else if (status == DatabaseCursor::Status::FAIL) { error = _("Error reading next record from wallet database"); + ret = false; break; } std::string key_str = HexStr(ss_key); @@ -85,7 +87,7 @@ bool DumpWallet(const ArgsManager& args, CWallet& wallet, bilingual_str& error) } } - batch->CloseCursor(); + cursor.reset(); batch.reset(); // Close the wallet after we're done with it. The caller won't be doing this @@ -201,7 +203,7 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: // dummy chain interface bool ret = true; - std::shared_ptr<CWallet> wallet(new CWallet(/*chain=*/nullptr, name, gArgs, std::move(database)), WalletToolReleaseWallet); + std::shared_ptr<CWallet> wallet(new CWallet(/*chain=*/nullptr, name, std::move(database)), WalletToolReleaseWallet); { LOCK(wallet->cs_wallet); DBErrors load_wallet_ret = wallet->LoadWallet(); @@ -254,8 +256,8 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: std::vector<unsigned char> k = ParseHex(key); std::vector<unsigned char> v = ParseHex(value); - CDataStream ss_key(k, SER_DISK, CLIENT_VERSION); - CDataStream ss_value(v, SER_DISK, CLIENT_VERSION); + DataStream ss_key{k}; + DataStream ss_value{v}; if (!batch->Write(ss_key, ss_value)) { error = strprintf(_("Error: Unable to write record to new wallet")); diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 9918979a81..01dc80b1ca 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -13,11 +13,11 @@ namespace wallet { class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan { public: - ExternalSignerScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor) - : DescriptorScriptPubKeyMan(storage, descriptor) + ExternalSignerScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size) + : DescriptorScriptPubKeyMan(storage, descriptor, keypool_size) {} - ExternalSignerScriptPubKeyMan(WalletStorage& storage) - : DescriptorScriptPubKeyMan(storage) + ExternalSignerScriptPubKeyMan(WalletStorage& storage, int64_t keypool_size) + : DescriptorScriptPubKeyMan(storage, keypool_size) {} /** Provide a descriptor at setup time diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index bd158b5985..37a704bfa4 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -155,7 +155,7 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) } Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors, - CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine) + CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine, const std::vector<CTxOut>& outputs) { // We are going to modify coin control later, copy to re-use CCoinControl new_coin_control(coin_control); @@ -222,11 +222,19 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo return result; } - // Fill in recipients(and preserve a single change key if there is one) - // While we're here, calculate the output amount - std::vector<CRecipient> recipients; + // Calculate the old output amount. CAmount output_value = 0; - for (const auto& output : wtx.tx->vout) { + for (const auto& old_output : wtx.tx->vout) { + output_value += old_output.nValue; + } + + old_fee = input_value - output_value; + + // Fill in recipients (and preserve a single change key if there + // is one). If outputs vector is non-empty, replace original + // outputs with its contents, otherwise use original outputs. + std::vector<CRecipient> recipients; + for (const auto& output : outputs.empty() ? wtx.tx->vout : outputs) { if (!OutputIsChange(wallet, output)) { CRecipient recipient = {output.scriptPubKey, output.nValue, false}; recipients.push_back(recipient); @@ -235,11 +243,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo ExtractDestination(output.scriptPubKey, change_dest); new_coin_control.destChange = change_dest; } - output_value += output.nValue; } - old_fee = input_value - output_value; - if (coin_control.m_feerate) { // The user provided a feeRate argument. // We calculate this here to avoid compiler warning on the cs_wallet lock diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index a96871b26f..53cf16e0f1 100644 --- a/src/wallet/feebumper.h +++ b/src/wallet/feebumper.h @@ -51,7 +51,8 @@ Result CreateRateBumpTransaction(CWallet& wallet, CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, - bool require_mine); + bool require_mine, + const std::vector<CTxOut>& outputs); //! Sign the new transaction, //! @return false if the tx couldn't be found or if it was diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 773f094274..5403e38950 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -122,9 +122,6 @@ bool WalletInit::ParameterInteraction() const return InitError(Untranslated("-zapwallettxes has been removed. If you are attempting to remove a stuck transaction from your wallet, please use abandontransaction instead.")); } - if (gArgs.GetBoolArg("-sysperms", false)) - return InitError(Untranslated("-sysperms is not allowed in combination with enabled wallet functionality")); - return true; } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 68dd3da9b5..1a76e46c54 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -291,7 +291,8 @@ public: CAmount& new_fee, CMutableTransaction& mtx) override { - return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true) == feebumper::Result::OK; + std::vector<CTxOut> outputs; // just an empty list of new recipients for now + return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true, outputs) == feebumper::Result::OK; } bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); } bool commitBumpTransaction(const uint256& txid, diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 7b49bad3f5..da63d45d11 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"); @@ -228,7 +226,7 @@ RPCHelpMan addmultisigaddress() {"key", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "bitcoin address or hex-encoded public key"}, }, }, - {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A label to assign the addresses to."}, + {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A label to assign the addresses to."}, {"address_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -addresstype"}, "The address type to use. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, }, RPCResult{ @@ -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); @@ -700,7 +696,7 @@ RPCHelpMan listlabels() return RPCHelpMan{"listlabels", "\nReturns the list of all labels, or labels that are assigned to addresses with a specific purpose.\n", { - {"purpose", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument."}, + {"purpose", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Address purpose to list labels for ('send','receive'). An empty string is the same as not providing this argument."}, }, RPCResult{ RPCResult::Type::ARR, "", "", diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 95117b6c19..9b6b3d629c 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -6,6 +6,7 @@ #include <clientversion.h> #include <core_io.h> #include <fs.h> +#include <hash.h> #include <interfaces/chain.h> #include <key_io.h> #include <merkleblock.h> @@ -14,8 +15,8 @@ #include <script/script.h> #include <script/standard.h> #include <sync.h> +#include <uint256.h> #include <util/bip32.h> -#include <util/system.h> #include <util/time.h> #include <util/translation.h> #include <wallet/rpc/util.h> @@ -156,9 +157,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 +248,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; @@ -338,7 +335,7 @@ RPCHelpMan importprunedfunds() } uint256 hashTx = tx.GetHash(); - CDataStream ssMB(ParseHexV(request.params[1], "proof"), SER_NETWORK, PROTOCOL_VERSION); + DataStream ssMB{ParseHexV(request.params[1], "proof")}; CMerkleBlock merkleBlock; ssMB >> merkleBlock; @@ -442,9 +439,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; @@ -757,6 +752,7 @@ RPCHelpMan dumpwallet() // sort time/key pairs std::vector<std::pair<int64_t, CKeyID> > vKeyBirth; + vKeyBirth.reserve(mapKeyBirth.size()); for (const auto& entry : mapKeyBirth) { vKeyBirth.push_back(std::make_pair(entry.second, entry.first)); } @@ -892,9 +888,7 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d } case TxoutType::WITNESS_V0_SCRIPTHASH: { if (script_ctx == ScriptContext::WITNESS_V0) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Trying to nest P2WSH inside another P2WSH"); - uint256 fullid(solverdata[0]); - CScriptID id; - CRIPEMD160().Write(fullid.begin(), fullid.size()).Finalize(id.begin()); + CScriptID id{RIPEMD160(solverdata[0])}; auto subscript = std::move(import_data.witnessscript); // Remove redeemscript from import_data to check for superfluous script later. if (!subscript) return "missing witnessscript"; if (CScriptID(*subscript) != id) return "witnessScript does not match the scriptPubKey or redeemScript"; @@ -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 @@ -1298,7 +1292,7 @@ RPCHelpMan importmulti() }, }, RPCArgOptions{.oneline_description="\"requests\""}}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, @@ -1336,8 +1330,6 @@ RPCHelpMan importmulti() // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); - RPCTypeCheck(mainRequest.params, {UniValue::VARR, UniValue::VOBJ}); - EnsureLegacyScriptPubKeyMan(*pwallet, true); const UniValue& requests = mainRequest.params[0]; @@ -1464,7 +1456,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; @@ -1486,7 +1478,7 @@ static UniValue ProcessDescriptorImport(CWallet& wallet, const UniValue& data, c } else { warnings.push_back("Range not given, using default keypool range"); range_start = 0; - range_end = gArgs.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE); + range_end = wallet.m_keypool_size; } next_index = range_start; @@ -1658,13 +1650,15 @@ RPCHelpMan importdescriptors() throw JSONRPCError(RPC_WALLET_ERROR, "importdescriptors is not available for non-descriptor wallets"); } - RPCTypeCheck(main_request.params, {UniValue::VARR, UniValue::VOBJ}); - WalletRescanReserver reserver(*pwallet); - if (!reserver.reserve()) { + if (!reserver.reserve(/*with_passphrase=*/true)) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } + // Ensure that the wallet is not locked for the remainder of this RPC, as + // the passphrase is used to top up the keypool. + LOCK(pwallet->m_relock_mutex); + const UniValue& requests = main_request.params[0]; const int64_t minimum_timestamp = 1; int64_t now = 0; @@ -1767,7 +1761,8 @@ RPCHelpMan listdescriptors() {RPCResult::Type::NUM, "", "Range start inclusive"}, {RPCResult::Type::NUM, "", "Range end inclusive"}, }}, - {RPCResult::Type::NUM, "next", /*optional=*/true, "The next index to generate addresses from; defined only for ranged descriptors"}, + {RPCResult::Type::NUM, "next", /*optional=*/true, "Same as next_index field. Kept for compatibility reason."}, + {RPCResult::Type::NUM, "next_index", /*optional=*/true, "The next index to generate addresses from; defined only for ranged descriptors"}, }}, }} }}, @@ -1844,6 +1839,7 @@ RPCHelpMan listdescriptors() range.push_back(info.range->second - 1); spk.pushKV("range", range); spk.pushKV("next", info.next_index); + spk.pushKV("next_index", info.next_index); } descriptors.push_back(spk); } @@ -1901,7 +1897,7 @@ RPCHelpMan restorewallet() { {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name that will be applied to the restored wallet"}, {"backup_file", RPCArg::Type::STR, RPCArg::Optional::NO, "The backup file that will be used to restore the wallet."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index 82642194c2..4c386789f1 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <core_io.h> +#include <hash.h> #include <key_io.h> #include <rpc/util.h> #include <util/moneystr.h> @@ -165,7 +166,7 @@ RPCHelpMan getbalance() "The available balance is what the wallet considers currently spendable, and is\n" "thus affected by options which limit spendability such as -spendzeroconfchange.\n", { - {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Remains for backward compatibility. Must be excluded or set to \"*\"."}, + {"dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Remains for backward compatibility. Must be excluded or set to \"*\"."}, {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "Only include transactions confirmed at least this many times."}, {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also include balance in watch-only addresses (see 'importaddress')"}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{true}, "(only available if avoid_reuse wallet flag is set) Do not include balance in dirty outputs; addresses are considered dirty if they have previously been used in a transaction."}, @@ -509,7 +510,7 @@ RPCHelpMan listunspent() }, {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{true}, "Include outputs that are not safe to spend\n" "See description of \"safe\" attribute below."}, - {"query_options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "JSON with query options", + {"query_options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "JSON with query options", { {"minimumAmount", RPCArg::Type::AMOUNT, RPCArg::Default{FormatMoney(0)}, "Minimum value of each UTXO in " + CURRENCY_UNIT + ""}, {"maximumAmount", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"unlimited"}, "Maximum value of each UTXO in " + CURRENCY_UNIT + ""}, @@ -679,8 +680,7 @@ RPCHelpMan listunspent() CHECK_NONFATAL(extracted); // Also return the witness script const WitnessV0ScriptHash& whash = std::get<WitnessV0ScriptHash>(witness_destination); - CScriptID id; - CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin()); + CScriptID id{RIPEMD160(whash)}; CScript witnessScript; if (provider->GetCScript(id, witnessScript)) { entry.pushKV("witnessScript", HexStr(witnessScript)); @@ -689,8 +689,7 @@ RPCHelpMan listunspent() } } else if (scriptPubKey.IsPayToWitnessScriptHash()) { const WitnessV0ScriptHash& whash = std::get<WitnessV0ScriptHash>(address); - CScriptID id; - CRIPEMD160().Write(whash.begin(), whash.size()).Finalize(id.begin()); + CScriptID id{RIPEMD160(whash)}; CScript witnessScript; if (provider->GetCScript(id, witnessScript)) { entry.pushKV("witnessScript", HexStr(witnessScript)); diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp index fcf25e01d6..0226d15698 100644 --- a/src/wallet/rpc/encrypt.cpp +++ b/src/wallet/rpc/encrypt.cpp @@ -49,9 +49,7 @@ RPCHelpMan walletpassphrase() // Note that the walletpassphrase is stored in request.params[0] which is not mlock()ed SecureString strWalletPass; strWalletPass.reserve(100); - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. - strWalletPass = request.params[0].get_str().c_str(); + strWalletPass = std::string_view{request.params[0].get_str()}; // Get the timeout nSleepTime = request.params[1].getInt<int64_t>(); @@ -70,7 +68,17 @@ RPCHelpMan walletpassphrase() } if (!pwallet->Unlock(strWalletPass)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + // Check if the passphrase has a null character (see #27067 for details) + if (strWalletPass.find('\0') == std::string::npos) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + } else { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered is incorrect. " + "It contains a null character (ie - a zero byte). " + "If the passphrase was set with a version of this software prior to 25.0, " + "please try again with only the characters up to — but not including — " + "the first null character. If this is successful, please set a new " + "passphrase to avoid this issue in the future."); + } } pwallet->TopUpKeyPool(); @@ -90,7 +98,7 @@ RPCHelpMan walletpassphrase() std::weak_ptr<CWallet> weak_wallet = wallet; pwallet->chain().rpcRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), [weak_wallet, relock_time] { if (auto shared_wallet = weak_wallet.lock()) { - LOCK(shared_wallet->cs_wallet); + LOCK2(shared_wallet->m_relock_mutex, shared_wallet->cs_wallet); // Skip if this is not the most recent rpcRunLater callback. if (shared_wallet->nRelockTime != relock_time) return; shared_wallet->Lock(); @@ -122,28 +130,39 @@ RPCHelpMan walletpassphrasechange() std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - LOCK(pwallet->cs_wallet); - if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletpassphrasechange was called."); } - // TODO: get rid of these .c_str() calls by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. + if (pwallet->IsScanningWithPassphrase()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before changing the passphrase."); + } + + LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet); + SecureString strOldWalletPass; strOldWalletPass.reserve(100); - strOldWalletPass = request.params[0].get_str().c_str(); + strOldWalletPass = std::string_view{request.params[0].get_str()}; SecureString strNewWalletPass; strNewWalletPass.reserve(100); - strNewWalletPass = request.params[1].get_str().c_str(); + strNewWalletPass = std::string_view{request.params[1].get_str()}; if (strOldWalletPass.empty() || strNewWalletPass.empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty"); } if (!pwallet->ChangeWalletPassphrase(strOldWalletPass, strNewWalletPass)) { - throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + // Check if the old passphrase had a null character (see #27067 for details) + if (strOldWalletPass.find('\0') == std::string::npos) { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The wallet passphrase entered was incorrect."); + } else { + throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "Error: The old wallet passphrase entered is incorrect. " + "It contains a null character (ie - a zero byte). " + "If the old passphrase was set with a version of this software prior to 25.0, " + "please try again with only the characters up to — but not including — " + "the first null character."); + } } return UniValue::VNULL; @@ -175,12 +194,16 @@ RPCHelpMan walletlock() std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - LOCK(pwallet->cs_wallet); - if (!pwallet->IsCrypted()) { throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an unencrypted wallet, but walletlock was called."); } + if (pwallet->IsScanningWithPassphrase()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before locking the wallet."); + } + + LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet); + pwallet->Lock(); pwallet->nRelockTime = 0; @@ -219,8 +242,6 @@ RPCHelpMan encryptwallet() std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - LOCK(pwallet->cs_wallet); - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: wallet does not contain private keys, nothing to encrypt."); } @@ -229,11 +250,15 @@ RPCHelpMan encryptwallet() throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: running with an encrypted wallet, but encryptwallet was called."); } - // TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string) - // Alternately, find a way to make request.params[0] mlock()'d to begin with. + if (pwallet->IsScanningWithPassphrase()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Error: the wallet is currently being used to rescan the blockchain for related transactions. Please call `abortrescan` before encrypting the wallet."); + } + + LOCK2(pwallet->m_relock_mutex, pwallet->cs_wallet); + SecureString strWalletPass; strWalletPass.reserve(100); - strWalletPass = request.params[0].get_str().c_str(); + strWalletPass = std::string_view{request.params[0].get_str()}; if (strWalletPass.empty()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "passphrase cannot be empty"); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 5f31c1d72c..88ee6e96b0 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -217,9 +217,9 @@ RPCHelpMan sendtoaddress() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to send to."}, {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The amount in " + CURRENCY_UNIT + " to send. eg 0.1"}, - {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment used to store what the transaction is for.\n" + {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A comment used to store what the transaction is for.\n" "This is not part of the transaction, just kept in your wallet."}, - {"comment_to", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment to store the name of the person or organization\n" + {"comment_to", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A comment to store the name of the person or organization\n" "to which you're sending the transaction. This is not part of the \n" "transaction, just kept in your wallet."}, {"subtractfeefromamount", RPCArg::Type::BOOL, RPCArg::Default{false}, "The fee will be deducted from the amount being sent.\n" @@ -314,18 +314,21 @@ RPCHelpMan sendtoaddress() RPCHelpMan sendmany() { return RPCHelpMan{"sendmany", - "\nSend multiple times. Amounts are double-precision floating point numbers." + + "Send multiple times. Amounts are double-precision floating point numbers." + HELP_REQUIRING_PASSPHRASE, { - {"dummy", RPCArg::Type::STR, RPCArg::Optional::NO, "Must be set to \"\" for backwards compatibility.", RPCArgOptions{.oneline_description="\"\""}}, + {"dummy", RPCArg::Type::STR, RPCArg::Default{"\"\""}, "Must be set to \"\" for backwards compatibility.", + RPCArgOptions{ + .oneline_description = "\"\"", + }}, {"amounts", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::NO, "The addresses and amounts", { {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The bitcoin address is the key, the numeric amount (can be string) in " + CURRENCY_UNIT + " is the value"}, }, }, - {"minconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "Ignored dummy value"}, - {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment"}, - {"subtractfeefrom", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The addresses.\n" + {"minconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Ignored dummy value"}, + {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "A comment"}, + {"subtractfeefrom", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The addresses.\n" "The fee will be equally deducted from the amount of each selected address.\n" "Those recipients will receive less bitcoins than you enter in their corresponding amount field.\n" "If no addresses are specified here, the sender pays the fee.", @@ -459,7 +462,7 @@ static std::vector<RPCArg> FundTxDoc(bool solving_data = true) }, }; if (solving_data) { - args.push_back({"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" + args.push_back({"solving_data", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "Keys and scripts needed for producing a final transaction with a dummy signature.\n" "Used for fee estimation during coin selection.", { { @@ -528,6 +531,8 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, {"replaceable", UniValueType(UniValue::VBOOL)}, {"conf_target", UniValueType(UniValue::VNUM)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, + {"minconf", UniValueType(UniValue::VNUM)}, + {"maxconf", UniValueType(UniValue::VNUM)}, {"input_weights", UniValueType(UniValue::VARR)}, }, true, true); @@ -593,6 +598,22 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, if (options.exists("replaceable")) { coinControl.m_signal_bip125_rbf = options["replaceable"].get_bool(); } + + if (options.exists("minconf")) { + coinControl.m_min_depth = options["minconf"].getInt<int>(); + + if (coinControl.m_min_depth < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Negative minconf"); + } + } + + if (options.exists("maxconf")) { + coinControl.m_max_depth = options["maxconf"].getInt<int>(); + + if (coinControl.m_max_depth < coinControl.m_min_depth) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("maxconf can't be lower than minconf: %d < %d", coinControl.m_max_depth, coinControl.m_min_depth)); + } + } SetFeeEstimateMode(wallet, coinControl, options["conf_target"], options["estimate_mode"], options["fee_rate"], override_min_fee); } } else { @@ -737,13 +758,15 @@ RPCHelpMan fundrawtransaction() "Only pay-to-pubkey, multisig, and P2SH versions thereof are currently supported for watch-only\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The hex string of the raw transaction"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "for backward compatibility: passing in a true instead of an object will result in {\"includeWatching\":true}", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{true}, "For a transaction with existing inputs, automatically include more if they are not enough."}, {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n" "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n" "If that happens, you will need to fund the transaction with different inputs and republish it."}, + {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "If add_inputs is specified, require inputs with at least this many confirmations."}, + {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If add_inputs is specified, require inputs with at most this many confirmations."}, {"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."}, @@ -761,20 +784,27 @@ RPCHelpMan fundrawtransaction() {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, }, - {"input_weights", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Inputs and their corresponding weights", + {"input_weights", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "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."}, + }, + }, }, }, }, FundTxDoc()), - RPCArgOptions{.oneline_description="options"}}, + RPCArgOptions{ + .skip_type_check = true, + .oneline_description = "options", + }}, {"iswitness", RPCArg::Type::BOOL, RPCArg::DefaultHint{"depends on heuristic tests"}, "Whether the transaction hex is a serialized witness transaction.\n" "If iswitness is not present, heuristic tests will be used in decoding.\n" "If true, only witness deserialization will be tried.\n" @@ -806,8 +836,6 @@ RPCHelpMan fundrawtransaction() std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - RPCTypeCheck(request.params, {UniValue::VSTR, UniValueType(), UniValue::VBOOL}); - // parse hex string from parameter CMutableTransaction tx; bool try_witness = request.params[2].isNull() ? true : request.params[2].get_bool(); @@ -842,7 +870,7 @@ RPCHelpMan signrawtransactionwithwallet() HELP_REQUIRING_PASSPHRASE, { {"hexstring", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction hex string"}, - {"prevtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The previous dependent transaction outputs", + {"prevtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "The previous dependent transaction outputs", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -896,8 +924,6 @@ RPCHelpMan signrawtransactionwithwallet() const std::shared_ptr<const CWallet> pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VARR, UniValue::VSTR}, true); - CMutableTransaction mtx; if (!DecodeHexTx(mtx, request.params[0].get_str())) { throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed. Make sure the tx has at least one input."); @@ -930,6 +956,26 @@ RPCHelpMan signrawtransactionwithwallet() }; } +// Definition of allowed formats of specifying transaction outputs in +// `bumpfee`, `psbtbumpfee`, `send` and `walletcreatefundedpsbt` RPCs. +static std::vector<RPCArg> OutputsDoc() +{ + return + { + {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "", + { + {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address,\n" + "the value (float or string) is the amount in " + CURRENCY_UNIT + ""}, + }, + }, + {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", + { + {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"}, + }, + }, + }; +} + static RPCHelpMan bumpfee_helper(std::string method_name) { const bool want_psbt = method_name == "psbtbumpfee"; @@ -951,7 +997,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) "* WARNING: before version 0.21, fee_rate was in " + CURRENCY_UNIT + "/kvB. As of 0.21, fee_rate is in " + CURRENCY_ATOM + "/vB. *\n", { {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The txid to be bumped"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks\n"}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, @@ -966,7 +1012,12 @@ static RPCHelpMan bumpfee_helper(std::string method_name) "still be replaceable in practice, for example if it has unconfirmed ancestors which\n" "are replaceable).\n"}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" - "\"" + FeeModes("\"\n\"") + "\""}, + "\"" + FeeModes("\"\n\"") + "\""}, + {"outputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "New outputs (key-value pairs) which will replace\n" + "the original ones, if provided. Each address can only appear once and there can\n" + "only be one \"data\" object.\n", + OutputsDoc(), + RPCArgOptions{.skip_type_check = true}}, }, RPCArgOptions{.oneline_description="options"}}, }, @@ -997,13 +1048,13 @@ static RPCHelpMan bumpfee_helper(std::string method_name) throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead."); } - RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ}); uint256 hash(ParseHashV(request.params[0], "txid")); CCoinControl coin_control; coin_control.fAllowWatchOnly = pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); // optional parameters coin_control.m_signal_bip125_rbf = true; + std::vector<CTxOut> outputs; if (!request.params[1].isNull()) { UniValue options = request.params[1]; @@ -1014,6 +1065,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) {"fee_rate", UniValueType()}, // will be checked by AmountFromValue() in SetFeeEstimateMode() {"replaceable", UniValueType(UniValue::VBOOL)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, + {"outputs", UniValueType()}, // will be checked by AddOutputs() }, true, true); @@ -1027,6 +1079,16 @@ static RPCHelpMan bumpfee_helper(std::string method_name) coin_control.m_signal_bip125_rbf = options["replaceable"].get_bool(); } SetFeeEstimateMode(*pwallet, coin_control, conf_target, options["estimate_mode"], options["fee_rate"], /*override_min_fee=*/false); + + // Prepare new outputs by creating a temporary tx and calling AddOutputs(). + if (!options["outputs"].isNull()) { + if (options["outputs"].isArray() && options["outputs"].empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, output argument cannot be an empty array"); + } + CMutableTransaction tempTx; + AddOutputs(tempTx, options["outputs"]); + outputs = tempTx.vout; + } } // Make sure the results are valid at least up to the most recent block @@ -1044,7 +1106,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) CMutableTransaction mtx; feebumper::Result res; // Targeting feerate bump. - res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt); + res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt, outputs); if (res != feebumper::Result::OK) { switch(res) { case feebumper::Result::INVALID_ADDRESS_OR_KEY: @@ -1119,30 +1181,21 @@ RPCHelpMan send() {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs (key-value pairs), where none of the keys are duplicated.\n" "That is, each address can only appear once and there can only be one 'data' object.\n" "For convenience, a dictionary, which holds the key-value pairs directly, is also accepted.", - { - {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "", - { - {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""}, - }, - }, - {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", - { - {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"}, - }, - }, - }, - }, + OutputsDoc(), + RPCArgOptions{.skip_type_check = true}}, {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"},"Automatically include coins from the wallet to cover the target amount.\n"}, {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n" "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n" "If that happens, you will need to fund the transaction with different inputs and republish it."}, + {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "If add_inputs is specified, require inputs with at least this many confirmations."}, + {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If add_inputs is specified, require inputs with at most this many confirmations."}, {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns a serialized transaction which will not be added to the wallet or broadcast"}, {"change_address", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"change_position", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, @@ -1201,15 +1254,6 @@ RPCHelpMan send() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - RPCTypeCheck(request.params, { - UniValueType(), // outputs (ARR or OBJ, checked later) - UniValue::VNUM, // conf_target - UniValue::VSTR, // estimate_mode - UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode() - UniValue::VOBJ, // options - }, true - ); - std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; @@ -1258,7 +1302,7 @@ RPCHelpMan sendall() "\"" + FeeModes("\"\n\"") + "\""}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, { - "options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + "options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_to_wallet", RPCArg::Type::BOOL, RPCArg::Default{true}, "When false, returns the serialized transaction without broadcasting or adding it to the wallet"}, @@ -1266,7 +1310,7 @@ RPCHelpMan sendall() {"include_watching", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Also select inputs which are watch-only.\n" "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, - {"inputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Use exactly the specified inputs to build the transaction. Specifying inputs is incompatible with send_max.", + {"inputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Use exactly the specified inputs to build the transaction. Specifying inputs is incompatible with the send_max, minconf, and maxconf options.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -1281,6 +1325,8 @@ RPCHelpMan sendall() {"lock_unspents", RPCArg::Type::BOOL, RPCArg::Default{false}, "Lock selected unspent outputs"}, {"psbt", RPCArg::Type::BOOL, RPCArg::DefaultHint{"automatic"}, "Always return a PSBT, implies add_to_wallet=false."}, {"send_max", RPCArg::Type::BOOL, RPCArg::Default{false}, "When true, only use UTXOs that can pay for their own fees to maximize the output amount. When 'false' (default), no UTXO is left behind. send_max is incompatible with providing specific inputs."}, + {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "Require inputs with at least this many confirmations."}, + {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Require inputs with at most this many confirmations."}, }, FundTxDoc() ), @@ -1310,15 +1356,6 @@ RPCHelpMan sendall() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - RPCTypeCheck(request.params, { - UniValue::VARR, // recipients - UniValue::VNUM, // conf_target - UniValue::VSTR, // estimate_mode - UniValueType(), // fee_rate, will be checked by AmountFromValue() in SetFeeEstimateMode() - UniValue::VOBJ, // options - }, true - ); - std::shared_ptr<CWallet> const pwallet{GetWalletForJSONRPCRequest(request)}; if (!pwallet) return UniValue::VNULL; // Make sure the results are valid at least up to the most recent block @@ -1355,6 +1392,23 @@ RPCHelpMan sendall() coin_control.fAllowWatchOnly = ParseIncludeWatchonly(options["include_watching"], *pwallet); + if (options.exists("minconf")) { + if (options["minconf"].getInt<int>() < 0) + { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid minconf (minconf cannot be negative): %s", options["minconf"].getInt<int>())); + } + + coin_control.m_min_depth = options["minconf"].getInt<int>(); + } + + if (options.exists("maxconf")) { + coin_control.m_max_depth = options["maxconf"].getInt<int>(); + + if (coin_control.m_max_depth < coin_control.m_min_depth) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("maxconf can't be lower than minconf: %d < %d", coin_control.m_max_depth, coin_control.m_min_depth)); + } + } + const bool rbf{options.exists("replaceable") ? options["replaceable"].get_bool() : pwallet->m_signal_rbf}; FeeCalculation fee_calc_out; @@ -1376,6 +1430,8 @@ RPCHelpMan sendall() bool send_max{options.exists("send_max") ? options["send_max"].get_bool() : false}; if (options.exists("inputs") && options.exists("send_max")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot combine send_max with specific inputs."); + } else if (options.exists("inputs") && (options.exists("minconf") || options.exists("maxconf"))) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot combine minconf or maxconf with specific inputs."); } else if (options.exists("inputs")) { for (const CTxIn& input : rawTx.vin) { if (pwallet->IsSpent(input.prevout)) { @@ -1514,8 +1570,6 @@ RPCHelpMan walletprocesspsbt() // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); - RPCTypeCheck(request.params, {UniValue::VSTR}); - // Unserialize the transaction PartiallySignedTransaction psbtx; std::string error; @@ -1558,7 +1612,7 @@ RPCHelpMan walletcreatefundedpsbt() "All existing inputs must either have their previous output transaction be in the wallet\n" "or be in the UTXO set. Solving data must be provided for non-wallet inputs.\n", { - {"inputs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "Leave empty to add inputs automatically. See add_inputs option.", + {"inputs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Leave empty to add inputs automatically. See add_inputs option.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -1578,27 +1632,18 @@ RPCHelpMan walletcreatefundedpsbt() "That is, each address can only appear once and there can only be one 'data' object.\n" "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" "accepted as second parameter.", - { - {"", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "", - { - {"address", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "A key-value pair. The key (string) is the bitcoin address, the value (float or string) is the amount in " + CURRENCY_UNIT + ""}, - }, - }, - {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", - { - {"data", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "A key-value pair. The key must be \"data\", the value is hex-encoded data"}, - }, - }, - }, - }, + OutputsDoc(), + RPCArgOptions{.skip_type_check = true}}, {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"}, - {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", + {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", Cat<std::vector<RPCArg>>( { {"add_inputs", RPCArg::Type::BOOL, RPCArg::DefaultHint{"false when \"inputs\" are specified, true otherwise"}, "Automatically include coins from the wallet to cover the target amount.\n"}, {"include_unsafe", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include inputs that are not safe to spend (unconfirmed transactions from outside keys and unconfirmed replacement transactions).\n" "Warning: the resulting transaction may become invalid if one of the unsafe inputs disappears.\n" "If that happens, you will need to fund the transaction with different inputs and republish it."}, + {"minconf", RPCArg::Type::NUM, RPCArg::Default{0}, "If add_inputs is specified, require inputs with at least this many confirmations."}, + {"maxconf", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "If add_inputs is specified, require inputs with at most this many confirmations."}, {"changeAddress", RPCArg::Type::STR, RPCArg::DefaultHint{"automatic"}, "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, RPCArg::DefaultHint{"random"}, "The index of the change output"}, {"change_type", RPCArg::Type::STR, RPCArg::DefaultHint{"set by -changetype"}, "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", \"bech32\", and \"bech32m\"."}, @@ -1641,15 +1686,6 @@ RPCHelpMan walletcreatefundedpsbt() // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); - RPCTypeCheck(request.params, { - UniValue::VARR, - UniValueType(), // ARR or OBJ, checked later - UniValue::VNUM, - UniValue::VOBJ, - UniValue::VBOOL - }, true - ); - UniValue options{request.params[3].isNull() ? UniValue::VOBJ : request.params[3]}; CAmount fee; diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index cff371c55b..3bfe296d90 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -206,7 +206,7 @@ RPCHelpMan listreceivedbyaddress() {"minconf", RPCArg::Type::NUM, RPCArg::Default{1}, "The minimum number of confirmations before payments are included."}, {"include_empty", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether to include addresses that haven't received any payments."}, {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see 'importaddress')"}, - {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If present and non-empty, only return information on this address."}, + {"address_filter", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If present and non-empty, only return information on this address."}, {"include_immature_coinbase", RPCArg::Type::BOOL, RPCArg::Default{false}, "Include immature coinbase transactions."}, }, RPCResult{ @@ -397,7 +397,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM } -static const std::vector<RPCResult> TransactionDescriptionString() +static std::vector<RPCResult> TransactionDescriptionString() { return{{RPCResult::Type::NUM, "confirmations", "The number of confirmations for the transaction. Negative confirmations means the\n" "transaction conflicted that many blocks ago."}, @@ -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 + "."}, @@ -435,7 +434,7 @@ RPCHelpMan listtransactions() "\nIf a label name is provided, this will return only incoming transactions paying to addresses with the specified label.\n" "\nReturns up to 'count' most recent transactions skipping the first 'from' transactions.\n", { - {"label|dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If set, should be a valid label name to return only incoming transactions\n" + {"label|dummy", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If set, should be a valid label name to return only incoming transactions\n" "with the specified label, or \"*\" to disable filtering and return all transactions."}, {"count", RPCArg::Type::NUM, RPCArg::Default{10}, "The number of transactions to return"}, {"skip", RPCArg::Type::NUM, RPCArg::Default{0}, "The number of transactions to skip"}, @@ -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 \"*\"."); } @@ -546,13 +545,13 @@ RPCHelpMan listsinceblock() "If \"blockhash\" is no longer a part of the main chain, transactions from the fork point onward are included.\n" "Additionally, if include_removed is set, transactions affecting the wallet which were removed are returned in the \"removed\" array.\n", { - {"blockhash", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "If set, the block hash to list transactions since, otherwise list all transactions."}, + {"blockhash", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "If set, the block hash to list transactions since, otherwise list all transactions."}, {"target_confirmations", RPCArg::Type::NUM, RPCArg::Default{1}, "Return the nth block hash from the main chain. e.g. 1 would mean the best block hash. Note: this is not used as a filter, but only affects [lastblock] in the return value"}, {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Include transactions to watch-only addresses (see 'importaddress')"}, {"include_removed", RPCArg::Type::BOOL, RPCArg::Default{true}, "Show transactions that were removed due to a reorg in the \"removed\" array\n" "(not guaranteed to work on pruned nodes)"}, {"include_change", RPCArg::Type::BOOL, RPCArg::Default{false}, "Also add entries for change outputs.\n"}, - {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Return only incoming transactions paying to addresses with the specified label.\n"}, + {"label", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Return only incoming transactions paying to addresses with the specified label.\n"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -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; @@ -850,7 +848,7 @@ RPCHelpMan rescanblockchain() "and block filters are available (using startup option \"-blockfilterindex=1\").\n", { {"start_height", RPCArg::Type::NUM, RPCArg::Default{0}, "block height where the rescan should start"}, - {"stop_height", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "the last block height that should be scanned. If none is provided it will rescan up to the tip at return time of this call."}, + {"stop_height", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "the last block height that should be scanned. If none is provided it will rescan up to the tip at return time of this call."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -874,15 +872,18 @@ RPCHelpMan rescanblockchain() wallet.BlockUntilSyncedToCurrentChain(); WalletRescanReserver reserver(*pwallet); - if (!reserver.reserve()) { + if (!reserver.reserve(/*with_passphrase=*/true)) { throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } int start_height = 0; std::optional<int> stop_height; uint256 start_block; + + LOCK(pwallet->m_relock_mutex); { LOCK(pwallet->cs_wallet); + EnsureWalletIsUnlocked(*pwallet); int tip_height = pwallet->GetLastBlockHeight(); if (!request.params[0].isNull()) { diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index e9607791a9..4d82e0a41f 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -6,6 +6,7 @@ #include <common/url.h> #include <rpc/util.h> +#include <util/system.h> #include <util/translation.h> #include <wallet/context.h> #include <wallet/wallet.h> @@ -75,7 +76,7 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques std::string wallet_name; if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { - const std::shared_ptr<CWallet> pwallet = GetWallet(context, wallet_name); + std::shared_ptr<CWallet> pwallet = GetWallet(context, wallet_name); if (!pwallet) throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Requested wallet does not exist or is not loaded"); return pwallet; } @@ -132,7 +133,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..16595267b4 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -201,7 +201,7 @@ static RPCHelpMan loadwallet() "\napplied to the new wallet.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -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); @@ -315,12 +323,12 @@ static RPCHelpMan createwallet() {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name for the new wallet. If this is a path, the wallet will be created at the path location."}, {"disable_private_keys", RPCArg::Type::BOOL, RPCArg::Default{false}, "Disable the possibility of private keys (only watchonlys are possible in this mode)."}, {"blank", RPCArg::Type::BOOL, RPCArg::Default{false}, "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed."}, - {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "Encrypt the wallet with this passphrase."}, + {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Encrypt the wallet with this passphrase."}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{false}, "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."}, {"descriptors", RPCArg::Type::BOOL, RPCArg::Default{true}, "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation." " Setting to \"false\" will create a legacy wallet; however, the legacy wallet type is being deprecated and" " support for creating and opening legacy wallets will be removed in the future."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, {"external_signer", RPCArg::Type::BOOL, RPCArg::Default{false}, "Use an external signer such as a hardware wallet. Requires -signer to be configured. Wallet creation will fail if keys cannot be fetched. Requires disable_private_keys and descriptors set to true."}, }, RPCResult{ @@ -351,7 +359,7 @@ static RPCHelpMan createwallet() passphrase.reserve(100); std::vector<bilingual_str> warnings; if (!request.params[3].isNull()) { - passphrase = request.params[3].get_str().c_str(); + passphrase = std::string_view{request.params[3].get_str()}; if (passphrase.empty()) { // Empty string means unencrypted warnings.emplace_back(Untranslated("Empty string given as passphrase, wallet will not be encrypted.")); @@ -411,7 +419,7 @@ static RPCHelpMan unloadwallet() "Specifying the wallet name on a wallet endpoint is invalid.", { {"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to unload. If provided both here and in the RPC endpoint, the two must be identical."}, - {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, + {"load_on_startup", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{RPCResult::Type::OBJ, "", "", { {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."}, @@ -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)); @@ -553,8 +568,6 @@ static RPCHelpMan upgradewallet() std::shared_ptr<CWallet> const pwallet = GetWalletForJSONRPCRequest(request); if (!pwallet) return UniValue::VNULL; - RPCTypeCheck(request.params, {UniValue::VNUM}, true); - EnsureWalletIsUnlocked(*pwallet); int version = 0; @@ -595,12 +608,12 @@ RPCHelpMan simulaterawtransaction() return RPCHelpMan{"simulaterawtransaction", "\nCalculate the balance change resulting in the signing and broadcasting of the given transaction(s).\n", { - {"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "An array of hex strings of raw transactions.\n", + {"rawtxs", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "An array of hex strings of raw transactions.\n", { {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, }, }, - {"options", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED_NAMED_ARG, "Options", + {"options", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED, "Options", { {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see RPC importaddress)"}, }, @@ -622,8 +635,6 @@ RPCHelpMan simulaterawtransaction() if (!rpc_wallet) return UniValue::VNULL; const CWallet& wallet = *rpc_wallet; - RPCTypeCheck(request.params, {UniValue::VARR, UniValue::VOBJ}, true); - LOCK(wallet.cs_wallet); UniValue include_watchonly(UniValue::VNULL); @@ -709,9 +720,12 @@ static RPCHelpMan migratewallet() "A new wallet backup will need to be made.\n" "\nThe migration process will create a backup of the wallet before migrating. This backup\n" "file will be named <wallet name>-<timestamp>.legacy.bak and can be found in the directory\n" - "for this wallet. In the event of an incorrect migration, the backup can be restored using restorewallet." + - HELP_REQUIRING_PASSPHRASE, - {}, + "for this wallet. In the event of an incorrect migration, the backup can be restored using restorewallet." + "\nEncrypted wallets must have the passphrase provided as an argument to this call.", + { + {"wallet_name", RPCArg::Type::STR, RPCArg::DefaultHint{"the wallet name from the RPC endpoint"}, "The name of the wallet to migrate. If provided both here and in the RPC endpoint, the two must be identical."}, + {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "The wallet passphrase"}, + }, RPCResult{ RPCResult::Type::OBJ, "", "", { @@ -727,16 +741,26 @@ static RPCHelpMan migratewallet() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - std::shared_ptr<CWallet> wallet = GetWalletForJSONRPCRequest(request); - if (!wallet) return NullUniValue; + std::string wallet_name; + if (GetWalletNameFromJSONRPCRequest(request, wallet_name)) { + if (!(request.params[0].isNull() || request.params[0].get_str() == wallet_name)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "RPC endpoint wallet and wallet_name parameter specify different wallets"); + } + } else { + if (request.params[0].isNull()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Either RPC endpoint wallet or wallet_name parameter must be provided"); + } + wallet_name = request.params[0].get_str(); + } - if (wallet->IsCrypted()) { - throw JSONRPCError(RPC_WALLET_WRONG_ENC_STATE, "Error: migratewallet on encrypted wallets is currently unsupported."); + SecureString wallet_pass; + wallet_pass.reserve(100); + if (!request.params[1].isNull()) { + wallet_pass = std::string_view{request.params[1].get_str()}; } WalletContext& context = EnsureWalletContext(request.context); - - util::Result<MigrationResult> res = MigrateLegacyToDescriptor(std::move(wallet), context); + util::Result<MigrationResult> res = MigrateLegacyToDescriptor(wallet_name, wallet_pass, context); if (!res) { throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(res).original); } diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index 9ba3c7fd2c..e2b4dbf4c2 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -135,11 +135,11 @@ bool RecoverDatabaseFile(const ArgsManager& args, const fs::path& file_path, bil } DbTxn* ptxn = env->TxnBegin(); - CWallet dummyWallet(nullptr, "", gArgs, CreateDummyWalletDatabase()); + CWallet dummyWallet(nullptr, "", CreateDummyWalletDatabase()); for (KeyValPair& row : salvagedData) { /* Filter for only private key type KV pairs to be added to the salvaged wallet */ - CDataStream ssKey(row.first, SER_DISK, CLIENT_VERSION); + DataStream ssKey{row.first}; CDataStream ssValue(row.second, SER_DISK, CLIENT_VERSION); std::string strType, strErr; bool fReadOK; diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index fbb52ea621..a1956049e2 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <hash.h> #include <key_io.h> #include <logging.h> #include <outputtype.h> @@ -10,7 +11,6 @@ #include <util/bip32.h> #include <util/strencodings.h> #include <util/string.h> -#include <util/system.h> #include <util/time.h> #include <util/translation.h> #include <wallet/scriptpubkeyman.h> @@ -166,9 +166,7 @@ IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& s if (sigversion == IsMineSigVersion::TOP && !keystore.HaveCScript(CScriptID(CScript() << OP_0 << vSolutions[0]))) { break; } - uint160 hash; - CRIPEMD160().Write(vSolutions[0].data(), vSolutions[0].size()).Finalize(hash.begin()); - CScriptID scriptID = CScriptID(hash); + CScriptID scriptID{RIPEMD160(vSolutions[0])}; CScript subscript; if (keystore.GetCScript(scriptID, subscript)) { ret = std::max(ret, recurse_scripthash ? IsMineInner(keystore, subscript, IsMineSigVersion::WITNESS_V0) : IsMineResult::SPENDABLE); @@ -1295,7 +1293,7 @@ bool LegacyScriptPubKeyMan::TopUpChain(CHDChain& chain, unsigned int kpSize) if (kpSize > 0) { nTargetSize = kpSize; } else { - nTargetSize = std::max(gArgs.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{0}); + nTargetSize = m_keypool_size; } int64_t target = std::max((int64_t) nTargetSize, int64_t{1}); @@ -1509,6 +1507,7 @@ std::vector<CKeyID> GetAffectedKeys(const CScript& spk, const SigningProvider& p FlatSigningProvider out; InferDescriptor(spk, provider)->Expand(0, DUMMY_SIGNING_PROVIDER, dummy, out); std::vector<CKeyID> ret; + ret.reserve(out.pubkeys.size()); for (const auto& entry : out.pubkeys) { ret.push_back(entry.first); } @@ -1665,7 +1664,7 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const return set_address; } -const std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPubKeys() const +std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPubKeys() const { LOCK(cs_KeyStore); std::unordered_set<CScript, SaltedSipHasher> spks; @@ -1785,7 +1784,7 @@ std::optional<MigrationData> LegacyScriptPubKeyMan::MigrateToDescriptor() WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0); // Make the DescriptorScriptPubKeyMan and get the scriptPubKeys - auto desc_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(m_storage, w_desc)); + auto desc_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(m_storage, w_desc, m_keypool_size)); desc_spk_man->AddDescriptorKey(key, key.GetPubKey()); desc_spk_man->TopUp(); auto desc_spks = desc_spk_man->GetScriptPubKeys(); @@ -1830,7 +1829,7 @@ std::optional<MigrationData> LegacyScriptPubKeyMan::MigrateToDescriptor() WalletDescriptor w_desc(std::move(desc), 0, 0, chain_counter, 0); // Make the DescriptorScriptPubKeyMan and get the scriptPubKeys - auto desc_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(m_storage, w_desc)); + auto desc_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(m_storage, w_desc, m_keypool_size)); desc_spk_man->AddDescriptorKey(master_key.key, master_key.key.GetPubKey()); desc_spk_man->TopUp(); auto desc_spks = desc_spk_man->GetScriptPubKeys(); @@ -1892,7 +1891,7 @@ std::optional<MigrationData> LegacyScriptPubKeyMan::MigrateToDescriptor() } else { // Make the DescriptorScriptPubKeyMan and get the scriptPubKeys WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0); - auto desc_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(m_storage, w_desc)); + auto desc_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(m_storage, w_desc, m_keypool_size)); for (const auto& keyid : privkeyids) { CKey key; if (!GetKey(keyid, key)) { @@ -2123,7 +2122,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 = m_keypool_size; } // Calculate the new range_end @@ -2497,12 +2496,13 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& input.FillSignatureData(sigdata); std::unique_ptr<FlatSigningProvider> keys = std::make_unique<FlatSigningProvider>(); - std::unique_ptr<FlatSigningProvider> script_keys = GetSigningProvider(script, sign); + std::unique_ptr<FlatSigningProvider> script_keys = GetSigningProvider(script, /*include_private=*/sign); if (script_keys) { keys->Merge(std::move(*script_keys)); } else { // Maybe there are pubkeys listed that we can sign for std::vector<CPubKey> pubkeys; + pubkeys.reserve(input.hd_keypaths.size() + 2); // ECDSA Pubkeys for (const auto& [pk, _] : input.hd_keypaths) { @@ -2538,7 +2538,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& } } - SignPSBTInput(HidingSigningProvider(keys.get(), !sign, !bip32derivs), psbtx, i, &txdata, sighash_type, nullptr, finalize); + SignPSBTInput(HidingSigningProvider(keys.get(), /*hide_secret=*/!sign, /*hide_origin=*/!bip32derivs), psbtx, i, &txdata, sighash_type, nullptr, finalize); bool signed_one = PSBTInputSigned(input); if (n_signed && (signed_one || !sign)) { @@ -2555,7 +2555,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& if (!keys) { continue; } - UpdatePSBTOutput(HidingSigningProvider(keys.get(), true, !bip32derivs), psbtx, i); + UpdatePSBTOutput(HidingSigningProvider(keys.get(), /*hide_secret=*/true, /*hide_origin=*/!bip32derivs), psbtx, i); } return TransactionError::OK; @@ -2651,17 +2651,17 @@ void DescriptorScriptPubKeyMan::WriteDescriptor() } } -const WalletDescriptor DescriptorScriptPubKeyMan::GetWalletDescriptor() const +WalletDescriptor DescriptorScriptPubKeyMan::GetWalletDescriptor() const { return m_wallet_descriptor; } -const std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys() const +std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys() const { return GetScriptPubKeys(0); } -const std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys(int32_t minimum_index) const +std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys(int32_t minimum_index) const { LOCK(cs_desc_man); std::unordered_set<CScript, SaltedSipHasher> script_pub_keys; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index d74388b3e8..4d14325241 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -36,7 +36,7 @@ class WalletStorage { public: virtual ~WalletStorage() = default; - virtual const std::string GetDisplayName() const = 0; + virtual std::string GetDisplayName() const = 0; virtual WalletDatabase& GetDatabase() const = 0; virtual bool IsWalletFlagSet(uint64_t) const = 0; virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; @@ -243,7 +243,7 @@ public: virtual uint256 GetID() const { return uint256(); } /** Returns a set of all the scriptPubKeys that this ScriptPubKeyMan watches */ - virtual const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const { return {}; }; + virtual std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const { return {}; }; /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ template<typename... Params> @@ -286,6 +286,9 @@ private: int64_t nTimeFirstKey GUARDED_BY(cs_KeyStore) = 0; + //! Number of pre-generated keys/scripts (part of the look-ahead process, used to detect payments) + int64_t m_keypool_size GUARDED_BY(cs_KeyStore){DEFAULT_KEYPOOL_SIZE}; + bool AddKeyPubKeyInner(const CKey& key, const CPubKey &pubkey); bool AddCryptedKeyInner(const CPubKey &vchPubKey, const std::vector<unsigned char> &vchCryptedSecret); @@ -363,7 +366,7 @@ private: bool TopUpChain(CHDChain& chain, unsigned int size); public: - using ScriptPubKeyMan::ScriptPubKeyMan; + LegacyScriptPubKeyMan(WalletStorage& storage, int64_t keypool_size) : ScriptPubKeyMan(storage), m_keypool_size(keypool_size) {} util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; @@ -512,7 +515,7 @@ public: const std::map<CKeyID, int64_t>& GetAllReserveKeys() const { return m_pool_key_to_index; } std::set<CKeyID> GetKeys() const override; - const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; + std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; /** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan. * Does not modify this ScriptPubKeyMan. */ @@ -555,6 +558,9 @@ private: //! keeps track of whether Unlock has run a thorough check before bool m_decryption_thoroughly_checked = false; + //! Number of pre-generated keys/scripts (part of the look-ahead process, used to detect payments) + int64_t m_keypool_size GUARDED_BY(cs_desc_man){DEFAULT_KEYPOOL_SIZE}; + bool AddDescriptorKeyWithDB(WalletBatch& batch, const CKey& key, const CPubKey &pubkey) EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); @@ -572,12 +578,14 @@ protected: WalletDescriptor m_wallet_descriptor GUARDED_BY(cs_desc_man); public: - DescriptorScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor) + DescriptorScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size) : ScriptPubKeyMan(storage), + m_keypool_size(keypool_size), m_wallet_descriptor(descriptor) {} - DescriptorScriptPubKeyMan(WalletStorage& storage) - : ScriptPubKeyMan(storage) + DescriptorScriptPubKeyMan(WalletStorage& storage, int64_t keypool_size) + : ScriptPubKeyMan(storage), + m_keypool_size(keypool_size) {} mutable RecursiveMutex cs_desc_man; @@ -641,9 +649,9 @@ public: void AddDescriptorKey(const CKey& key, const CPubKey &pubkey); void WriteDescriptor(); - const WalletDescriptor GetWalletDescriptor() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); - const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; - const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys(int32_t minimum_index) const; + WalletDescriptor GetWalletDescriptor() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); + std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; + std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys(int32_t minimum_index) const; int32_t GetEndRange() const; bool GetDescriptorString(std::string& out, const bool priv) const; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index 3bb3c17119..4548d5f813 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -30,11 +30,11 @@ using interfaces::FoundBlock; namespace wallet { static constexpr size_t OUTPUT_GROUP_MAX_ENTRIES{100}; -int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* provider, const CCoinControl* coin_control) +int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* provider, bool can_grind_r, const CCoinControl* coin_control) { CMutableTransaction txn; txn.vin.push_back(CTxIn(outpoint)); - if (!provider || !DummySignInput(*provider, txn.vin[0], txout, coin_control)) { + if (!provider || !DummySignInput(*provider, txn.vin[0], txout, can_grind_r, coin_control)) { return -1; } return GetVirtualTransactionInputSize(txn.vin[0]); @@ -43,7 +43,7 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoin int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, const CCoinControl* coin_control) { const std::unique_ptr<SigningProvider> provider = wallet->GetSolvingProvider(txout.scriptPubKey); - return CalculateMaximumSignedInputSize(txout, COutPoint(), provider.get(), coin_control); + return CalculateMaximumSignedInputSize(txout, COutPoint(), provider.get(), wallet->CanGrindR(), coin_control); } // txouts needs to be in the order of tx.vin @@ -163,6 +163,7 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const PreSelectedInputs result; std::vector<COutPoint> vPresetInputs; coin_control.ListSelected(vPresetInputs); + const bool can_grind_r = wallet.CanGrindR(); for (const COutPoint& outpoint : vPresetInputs) { int input_bytes = -1; CTxOut txout; @@ -181,7 +182,7 @@ util::Result<PreSelectedInputs> FetchSelectedInputs(const CWallet& wallet, const } if (input_bytes == -1) { - input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); + input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, can_grind_r, &coin_control); } // If available, override calculated size with coin control specified size @@ -214,6 +215,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, const int min_depth = {coinControl ? coinControl->m_min_depth : DEFAULT_MIN_DEPTH}; const int max_depth = {coinControl ? coinControl->m_max_depth : DEFAULT_MAX_DEPTH}; const bool only_safe = {coinControl ? !coinControl->m_include_unsafe_inputs : true}; + const bool can_grind_r = wallet.CanGrindR(); std::set<uint256> trusted_parents; for (const auto& entry : wallet.mapWallet) @@ -287,7 +289,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)) @@ -305,10 +307,8 @@ CoinsResult AvailableCoins(const CWallet& wallet, std::unique_ptr<SigningProvider> provider = wallet.GetSolvingProvider(output.scriptPubKey); - int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), coinControl); - // Because CalculateMaximumSignedInputSize just uses ProduceSignature and makes a dummy signature, - // it is safe to assume that this input is solvable if input_bytes is greater -1. - bool solvable = input_bytes > -1; + int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), can_grind_r, coinControl); + bool solvable = provider ? InferDescriptor(output.scriptPubKey, *provider)->IsSolvable() : false; bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)); // Filter by spendable outputs only @@ -362,92 +362,80 @@ CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinContr return AvailableCoins(wallet, coinControl).GetTotalAmount(); } -const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransaction& tx, int output) +const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint) { AssertLockHeld(wallet.cs_wallet); - const CTransaction* ptx = &tx; - int n = output; + const CWalletTx* wtx{Assert(wallet.GetWalletTx(outpoint.hash))}; + + const CTransaction* ptx = wtx->tx.get(); + int n = outpoint.n; while (OutputIsChange(wallet, ptx->vout[n]) && ptx->vin.size() > 0) { const COutPoint& prevout = ptx->vin[0].prevout; - auto it = wallet.mapWallet.find(prevout.hash); - if (it == wallet.mapWallet.end() || it->second.tx->vout.size() <= prevout.n || - !wallet.IsMine(it->second.tx->vout[prevout.n])) { + const CWalletTx* it = wallet.GetWalletTx(prevout.hash); + if (!it || it->tx->vout.size() <= prevout.n || + !wallet.IsMine(it->tx->vout[prevout.n])) { break; } - ptx = it->second.tx.get(); + ptx = it->tx.get(); n = prevout.n; } return ptx->vout[n]; } -const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint) -{ - AssertLockHeld(wallet.cs_wallet); - return FindNonChangeParentOutput(wallet, *wallet.GetWalletTx(outpoint.hash)->tx, outpoint.n); -} - std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) { AssertLockHeld(wallet.cs_wallet); std::map<CTxDestination, std::vector<COutput>> result; - for (COutput& coin : AvailableCoinsListUnspent(wallet).All()) { + CCoinControl coin_control; + // Include watch-only for LegacyScriptPubKeyMan wallets without private keys + coin_control.fAllowWatchOnly = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); + CoinFilterParams coins_params; + coins_params.only_spendable = false; + coins_params.skip_locked = false; + for (const COutput& coin : AvailableCoins(wallet, &coin_control, /*feerate=*/std::nullopt, coins_params).All()) { CTxDestination address; if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) && ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) { - result[address].emplace_back(std::move(coin)); + result[address].emplace_back(coin); } } - - std::vector<COutPoint> lockedCoins; - wallet.ListLockedCoins(lockedCoins); - // Include watch-only for LegacyScriptPubKeyMan wallets without private keys - const bool include_watch_only = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); - const isminetype is_mine_filter = include_watch_only ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - for (const COutPoint& output : lockedCoins) { - auto it = wallet.mapWallet.find(output.hash); - if (it != wallet.mapWallet.end()) { - const auto& wtx = it->second; - int depth = wallet.GetTxDepthInMainChain(wtx); - if (depth >= 0 && output.n < wtx.tx->vout.size() && - wallet.IsMine(wtx.tx->vout[output.n]) == is_mine_filter - ) { - CTxDestination address; - if (ExtractDestination(FindNonChangeParentOutput(wallet, *wtx.tx, output.n).scriptPubKey, address)) { - const auto out = wtx.tx->vout.at(output.n); - result[address].emplace_back( - COutPoint(wtx.GetHash(), output.n), out, depth, CalculateMaximumSignedInputSize(out, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ false, wtx.GetTxTime(), CachedTxIsFromMe(wallet, wtx, ISMINE_ALL)); - } - } - } - } - return result; } -std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<COutput>& outputs, const CoinSelectionParams& coin_sel_params, const CoinEligibilityFilter& filter, bool positive_only) +FilteredOutputGroups GroupOutputs(const CWallet& wallet, + const CoinsResult& coins, + const CoinSelectionParams& coin_sel_params, + const std::vector<SelectionFilter>& filters, + std::vector<OutputGroup>& ret_discarded_groups) { - std::vector<OutputGroup> groups_out; + FilteredOutputGroups filtered_groups; if (!coin_sel_params.m_avoid_partial_spends) { - // Allowing partial spends means no grouping. Each COutput gets its own OutputGroup. - for (const COutput& output : outputs) { - // Skip outputs we cannot spend - if (!output.spendable) continue; - - size_t ancestors, descendants; - wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); - - // Make an OutputGroup containing just this output - OutputGroup group{coin_sel_params}; - group.Insert(output, ancestors, descendants, positive_only); - - // Check the OutputGroup's eligibility. Only add the eligible ones. - if (positive_only && group.GetSelectionAmount() <= 0) continue; - if (group.m_outputs.size() > 0 && group.EligibleForSpending(filter)) groups_out.push_back(group); + // Allowing partial spends means no grouping. Each COutput gets its own OutputGroup + for (const auto& [type, outputs] : coins.coins) { + for (const COutput& output : outputs) { + // Get mempool info + size_t ancestors, descendants; + wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); + + // Create a new group per output and add it to the all groups vector + OutputGroup group(coin_sel_params); + group.Insert(std::make_shared<COutput>(output), ancestors, descendants); + + // Each filter maps to a different set of groups + bool accepted = false; + for (const auto& sel_filter : filters) { + const auto& filter = sel_filter.filter; + if (!group.EligibleForSpending(filter)) continue; + filtered_groups[filter].Push(group, type, /*insert_positive=*/true, /*insert_mixed=*/true); + accepted = true; + } + if (!accepted) ret_discarded_groups.emplace_back(group); + } } - return groups_out; + return filtered_groups; } // We want to combine COutputs that have the same scriptPubKey into single OutputGroups @@ -456,16 +444,11 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C // For each COutput, we check if the scriptPubKey is in the map, and if it is, the COutput is added // to the last OutputGroup in the vector for the scriptPubKey. When the last OutputGroup has // OUTPUT_GROUP_MAX_ENTRIES COutputs, a new OutputGroup is added to the end of the vector. - std::map<CScript, std::vector<OutputGroup>> spk_to_groups_map; - for (const auto& output : outputs) { - // Skip outputs we cannot spend - if (!output.spendable) continue; - - size_t ancestors, descendants; - wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); - CScript spk = output.txout.scriptPubKey; - - std::vector<OutputGroup>& groups = spk_to_groups_map[spk]; + typedef std::map<std::pair<CScript, OutputType>, std::vector<OutputGroup>> ScriptPubKeyToOutgroup; + const auto& insert_output = [&]( + const std::shared_ptr<COutput>& output, OutputType type, size_t ancestors, size_t descendants, + ScriptPubKeyToOutgroup& groups_map) { + std::vector<OutputGroup>& groups = groups_map[std::make_pair(output->txout.scriptPubKey,type)]; if (groups.size() == 0) { // No OutputGroups for this scriptPubKey yet, add one @@ -484,41 +467,84 @@ std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<C group = &groups.back(); } - // Add the output to group - group->Insert(output, ancestors, descendants, positive_only); - } - - // Now we go through the entire map and pull out the OutputGroups - for (const auto& spk_and_groups_pair: spk_to_groups_map) { - const std::vector<OutputGroup>& groups_per_spk= spk_and_groups_pair.second; + group->Insert(output, ancestors, descendants); + }; - // Go through the vector backwards. This allows for the first item we deal with being the partial group. - for (auto group_it = groups_per_spk.rbegin(); group_it != groups_per_spk.rend(); group_it++) { - const OutputGroup& group = *group_it; + ScriptPubKeyToOutgroup spk_to_groups_map; + ScriptPubKeyToOutgroup spk_to_positive_groups_map; + for (const auto& [type, outs] : coins.coins) { + for (const COutput& output : outs) { + size_t ancestors, descendants; + wallet.chain().getTransactionAncestry(output.outpoint.hash, ancestors, descendants); - // Don't include partial groups if there are full groups too and we don't want partial groups - if (group_it == groups_per_spk.rbegin() && groups_per_spk.size() > 1 && !filter.m_include_partial_groups) { - continue; + const auto& shared_output = std::make_shared<COutput>(output); + // Filter for positive only before adding the output + if (output.GetEffectiveValue() > 0) { + insert_output(shared_output, type, ancestors, descendants, spk_to_positive_groups_map); } - // Check the OutputGroup's eligibility. Only add the eligible ones. - if (positive_only && group.GetSelectionAmount() <= 0) continue; - if (group.m_outputs.size() > 0 && group.EligibleForSpending(filter)) groups_out.push_back(group); + // 'All' groups + insert_output(shared_output, type, ancestors, descendants, spk_to_groups_map); } } - return groups_out; + // Now we go through the entire maps and pull out the OutputGroups + const auto& push_output_groups = [&](const ScriptPubKeyToOutgroup& groups_map, bool positive_only) { + for (const auto& [script, groups] : groups_map) { + // Go through the vector backwards. This allows for the first item we deal with being the partial group. + for (auto group_it = groups.rbegin(); group_it != groups.rend(); group_it++) { + const OutputGroup& group = *group_it; + + // Each filter maps to a different set of groups + bool accepted = false; + for (const auto& sel_filter : filters) { + const auto& filter = sel_filter.filter; + if (!group.EligibleForSpending(filter)) continue; + + // Don't include partial groups if there are full groups too and we don't want partial groups + if (group_it == groups.rbegin() && groups.size() > 1 && !filter.m_include_partial_groups) { + continue; + } + + OutputType type = script.second; + // Either insert the group into the positive-only groups or the mixed ones. + filtered_groups[filter].Push(group, type, positive_only, /*insert_mixed=*/!positive_only); + accepted = true; + } + if (!accepted) ret_discarded_groups.emplace_back(group); + } + } + }; + + push_output_groups(spk_to_groups_map, /*positive_only=*/ false); + push_output_groups(spk_to_positive_groups_map, /*positive_only=*/ true); + + return filtered_groups; } -std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, const CoinsResult& available_coins, +FilteredOutputGroups GroupOutputs(const CWallet& wallet, + const CoinsResult& coins, + const CoinSelectionParams& params, + const std::vector<SelectionFilter>& filters) +{ + std::vector<OutputGroup> unused; + return GroupOutputs(wallet, coins, params, filters, unused); +} + +// 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 CAmount& nTargetValue, OutputGroupTypeMap& groups, 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); - } + for (auto& [type, group] : groups.groups_by_type) { + auto result{ChooseSelectionResult(nTargetValue, group, 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 @@ -527,41 +553,37 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm // If we can't fund the transaction from any individual OutputType, run coin selection one last time // 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; - } + if (allow_mixed_output_types && groups.TypesCount() > 1) { + return ChooseSelectionResult(nTargetValue, groups.all_groups, 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 CAmount& nTargetValue, Groups& groups, const CoinSelectionParams& coin_selection_params) { // Vector of results. We will choose the best one based on waste. std::vector<SelectionResult> results; - std::vector<OutputGroup> positive_groups = GroupOutputs(wallet, available_coins, coin_selection_params, eligibility_filter, /*positive_only=*/true); - if (auto bnb_result{SelectCoinsBnB(positive_groups, nTargetValue, coin_selection_params.m_cost_of_change)}) { + if (auto bnb_result{SelectCoinsBnB(groups.positive_group, nTargetValue, coin_selection_params.m_cost_of_change)}) { results.push_back(*bnb_result); } // The knapsack solver has some legacy behavior where it will spend dust outputs. We retain this behavior, so don't filter for positive only here. - std::vector<OutputGroup> all_groups = GroupOutputs(wallet, available_coins, coin_selection_params, eligibility_filter, /*positive_only=*/false); - if (auto knapsack_result{KnapsackSolver(all_groups, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) { + if (auto knapsack_result{KnapsackSolver(groups.mixed_group, nTargetValue, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) { knapsack_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*knapsack_result); } - if (auto srd_result{SelectCoinsSRD(positive_groups, nTargetValue, coin_selection_params.rng_fast)}) { + if (auto srd_result{SelectCoinsSRD(groups.positive_group, nTargetValue, coin_selection_params.rng_fast)}) { srd_result->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); results.push_back(*srd_result); } if (results.empty()) { // No solution found - return std::nullopt; + return util::Error(); } std::vector<SelectionResult> eligible_results; @@ -571,7 +593,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 +603,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,11 +629,11 @@ 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 - auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_control, coin_selection_params); + auto op_selection_result = AutomaticCoinSelection(wallet, available_coins, selection_target, coin_selection_params); if (!op_selection_result) return op_selection_result; // If needed, add preset inputs to the automatic coin selection result @@ -622,7 +648,7 @@ 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) +util::Result<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, CoinsResult& available_coins, const CAmount& value_to_select, const CoinSelectionParams& coin_selection_params) { unsigned int limit_ancestor_count = 0; unsigned int limit_descendant_count = 0; @@ -631,66 +657,93 @@ std::optional<SelectionResult> AutomaticCoinSelection(const CWallet& wallet, Coi const size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count); const bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); - // form groups from remaining coins; note that preset coins will not - // automatically have their associated (same address) coins included - if (coin_control.m_avoid_partial_spends && available_coins.Size() > OUTPUT_GROUP_MAX_ENTRIES) { - // Cases where we have 101+ outputs all pointing to the same destination may result in - // privacy leaks as they will potentially be deterministically sorted. We solve that by - // explicitly shuffling the outputs before processing + // Cases where we have 101+ outputs all pointing to the same destination may result in + // privacy leaks as they will potentially be deterministically sorted. We solve that by + // explicitly shuffling the outputs before processing + if (coin_selection_params.m_avoid_partial_spends && available_coins.Size() > OUTPUT_GROUP_MAX_ENTRIES) { available_coins.Shuffle(coin_selection_params.rng_fast); } // 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; - } + if (coin_selection_params.m_include_unsafe_inputs) { + 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)}); } } - // Coin Selection failed. - return std::optional<SelectionResult>(); + + // Group outputs and map them by coin eligibility filter + std::vector<OutputGroup> discarded_groups; + FilteredOutputGroups filtered_groups = GroupOutputs(wallet, available_coins, coin_selection_params, ordered_filters, discarded_groups); + + // Check if we still have enough balance after applying filters (some coins might be discarded) + CAmount total_discarded = 0; + CAmount total_unconf_long_chain = 0; + for (const auto& group : discarded_groups) { + total_discarded += group.GetSelectionAmount(); + if (group.m_ancestors >= max_ancestors || group.m_descendants >= max_descendants) total_unconf_long_chain += group.GetSelectionAmount(); + } + + if (CAmount total_amount = available_coins.GetTotalAmount() - total_discarded < value_to_select) { + // Special case, too-long-mempool cluster. + if (total_amount + total_unconf_long_chain > value_to_select) { + return util::Result<SelectionResult>({_("Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool")}); + } + return util::Result<SelectionResult>(util::Error()); // General "Insufficient Funds" + } + + // 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) { + auto it = filtered_groups.find(select_filter.filter); + if (it == filtered_groups.end()) continue; + if (auto res{AttemptSelection(value_to_select, it->second, + 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); + } + } + + // Return right away if we have a detailed error + if (!res_detailed_errors.empty()) return res_detailed_errors.front(); + + + // General "Insufficient Funds" + return util::Result<SelectionResult>(util::Error()); }(); return res; @@ -787,6 +840,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( CoinSelectionParams coin_selection_params{rng_fast}; // Parameters for coin selection, init with dummy coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends; + coin_selection_params.m_include_unsafe_inputs = coin_control.m_include_unsafe_inputs; // Set the long term feerate estimate to the wallet's consolidate feerate coin_selection_params.m_long_term_feerate = wallet.m_consolidate_feerate; @@ -838,7 +892,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( coin_selection_params.change_output_size = GetSerializeSize(change_prototype_txout); // Get size of spending the change output - int change_spend_size = CalculateMaximumSignedInputSize(change_prototype_txout, &wallet); + int change_spend_size = CalculateMaximumSignedInputSize(change_prototype_txout, &wallet, /*coin_control=*/nullptr); // If the wallet doesn't know how to sign change output, assume p2sh-p2wpkh // as lower-bound to allow BnB to do it's thing if (change_spend_size == -1) { @@ -860,7 +914,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( } if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) { // eventually allow a fallback fee - return util::Error{_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.")}; + return util::Error{strprintf(_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable %s."), "-fallbackfee")}; } // Calculate the cost of change @@ -901,6 +955,12 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( const CAmount not_input_fees = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.m_subtract_fee_outputs ? 0 : coin_selection_params.tx_noinputs_size); CAmount selection_target = recipients_sum + not_input_fees; + // This can only happen if feerate is 0, and requested destinations are value of 0 (e.g. OP_RETURN) + // and no pre-selected inputs. This will result in 0-input transaction, which is consensus-invalid anyways + if (selection_target == 0 && !coin_control.HasSelected()) { + return util::Error{_("Transaction requires one destination of non-0 value, a non-0 feerate, or a pre-selected input")}; + } + // Fetch manually selected coins PreSelectedInputs preset_inputs; if (coin_control.HasSelected()) { @@ -917,13 +977,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 +1001,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( } // Shuffle selected coins and fill in final vin - std::vector<COutput> selected_coins = result->GetShuffledInputVector(); + std::vector<std::shared_ptr<COutput>> selected_coins = result.GetShuffledInputVector(); // The sequence number is set to non-maxint so that DiscourageFeeSniping // works. @@ -950,7 +1013,7 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( // behavior." const uint32_t nSequence{coin_control.m_signal_bip125_rbf.value_or(wallet.m_signal_rbf) ? MAX_BIP125_RBF_SEQUENCE : CTxIn::MAX_SEQUENCE_NONFINAL}; for (const auto& coin : selected_coins) { - txNew.vin.push_back(CTxIn(coin.outpoint, CScript(), nSequence)); + txNew.vin.push_back(CTxIn(coin->outpoint, CScript(), nSequence)); } DiscourageFeeSniping(txNew, rng_fast, wallet.chain(), wallet.GetLastBlockHash(), wallet.GetLastBlockHeight()); @@ -963,7 +1026,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 +1037,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 +1076,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"))}; } @@ -1097,6 +1160,13 @@ util::Result<CreatedTransactionResult> CreateTransaction( TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str()); CCoinControl tmp_cc = coin_control; tmp_cc.m_avoid_partial_spends = true; + + // Re-use the change destination from the first creation attempt to avoid skipping BIP44 indexes + const int ungrouped_change_pos = txr_ungrouped.change_pos; + if (ungrouped_change_pos != -1) { + ExtractDestination(txr_ungrouped.tx->vout[ungrouped_change_pos].scriptPubKey, tmp_cc.destChange); + } + auto txr_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); // if fee of this alternative one is within the range of the max fee, we use this one const bool use_aps{txr_grouped.has_value() ? (txr_grouped->fee <= txr_ungrouped.fee + wallet.m_max_aps_fee) : false}; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index c612ed105e..78c2c5f22b 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -17,8 +17,8 @@ namespace wallet { /** Get the marginal bytes if spending the specified output from this transaction. * Use CoinControl to determine whether to expect signature grinding when calculating the size of the input spend. */ -int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, const CCoinControl* coin_control = nullptr); -int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* pwallet, const CCoinControl* coin_control = nullptr); +int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, const CCoinControl* coin_control); +int CalculateMaximumSignedInputSize(const CTxOut& txout, const COutPoint outpoint, const SigningProvider* pwallet, bool can_grind_r, const CCoinControl* coin_control); struct TxSize { int64_t vsize{-1}; int64_t weight{-1}; @@ -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}; }; /** @@ -97,7 +99,6 @@ CAmount GetAvailableBalance(const CWallet& wallet, const CCoinControl* coinContr /** * Find non-change parent output. */ -const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const CTransaction& tx, int output) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& outpoint) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); /** @@ -105,23 +106,35 @@ const CTxOut& FindNonChangeParentOutput(const CWallet& wallet, const COutPoint& */ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); -std::vector<OutputGroup> GroupOutputs(const CWallet& wallet, const std::vector<COutput>& outputs, const CoinSelectionParams& coin_sel_params, const CoinEligibilityFilter& filter, bool positive_only); +struct SelectionFilter { + CoinEligibilityFilter filter; + bool allow_mixed_output_types{true}; +}; + +/** +* Group coins by the provided filters. +*/ +FilteredOutputGroups GroupOutputs(const CWallet& wallet, + const CoinsResult& coins, + const CoinSelectionParams& coin_sel_params, + const std::vector<SelectionFilter>& filters); + /** * Attempt to find a valid input set that preserves privacy by not mixing OutputTypes. * `ChooseSelectionResult()` will be called on each OutputType individually and the best * the solution (according to the waste metric) will be chosen. If a valid input cannot be found from any * single OutputType, fallback to running `ChooseSelectionResult()` over all available coins. * - * param@[in] wallet The wallet which provides solving data for the coins * param@[in] nTargetValue The target value - * param@[in] eligilibity_filter A filter containing rules for which coins are allowed to be included in this selection - * param@[in] available_coins The struct of coins, organized by OutputType, available for selection prior to filtering + * param@[in] groups The grouped outputs mapped by coin eligibility filters * 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 CAmount& nTargetValue, OutputGroupTypeMap& groups, const CoinSelectionParams& coin_selection_params, bool allow_mixed_output_types); /** @@ -129,21 +142,20 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm * Multiple coin selection algorithms will be run and the input set that produces the least waste * (according to the waste metric) will be chosen. * - * param@[in] wallet The wallet which provides solving data for the coins * param@[in] nTargetValue The target value - * param@[in] eligilibity_filter A filter containing rules for which coins are allowed to be included in this selection - * param@[in] available_coins The struct of coins, organized by OutputType, available for selection prior to filtering + * param@[in] groups The struct containing the outputs grouped by script and divided by (1) positive only outputs and (2) all outputs (positive + negative). * 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, - const CoinSelectionParams& coin_selection_params); +util::Result<SelectionResult> ChooseSelectionResult(const CAmount& nTargetValue, Groups& groups, const CoinSelectionParams& coin_selection_params); // User manually selected inputs that must be part of the transaction struct PreSelectedInputs { - std::set<COutput> coins; + std::set<std::shared_ptr<COutput>> coins; // If subtract fee from outputs is disabled, the 'total_amount' // will be the sum of each output effective value // instead of the sum of the outputs amount @@ -156,7 +168,7 @@ struct PreSelectedInputs } else { total_amount += output.GetEffectiveValue(); } - coins.insert(output); + coins.insert(std::make_shared<COutput>(output)); } }; @@ -175,18 +187,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 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/sqlite.cpp b/src/wallet/sqlite.cpp index f2b9909851..8d8a7ab2a2 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -23,9 +23,6 @@ namespace wallet { static constexpr int32_t WALLET_SCHEMA_VERSION = 0; -static GlobalMutex g_sqlite_mutex; -static int g_sqlite_count GUARDED_BY(g_sqlite_mutex) = 0; - static void ErrorLogCallback(void* arg, int code, const char* msg) { // From sqlite3_config() documentation for the SQLITE_CONFIG_LOG option: @@ -83,6 +80,9 @@ static void SetPragma(sqlite3* db, const std::string& key, const std::string& va } } +Mutex SQLiteDatabase::g_sqlite_mutex; +int SQLiteDatabase::g_sqlite_count = 0; + SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock) : WalletDatabase(), m_mock(mock), m_dir_path(fs::PathToString(dir_path)), m_file_path(fs::PathToString(file_path)), m_use_unsafe_sync(options.use_unsafe_sync) { @@ -125,7 +125,6 @@ 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_cursor_stmt, "SELECT key, value FROM main"}, }; for (const auto& [stmt_prepared, stmt_text] : statements) { @@ -146,6 +145,8 @@ SQLiteDatabase::~SQLiteDatabase() void SQLiteDatabase::Cleanup() noexcept { + AssertLockNotHeld(g_sqlite_mutex); + Close(); LOCK(g_sqlite_mutex); @@ -374,7 +375,6 @@ void SQLiteBatch::Close() {&m_insert_stmt, "insert"}, {&m_overwrite_stmt, "overwrite"}, {&m_delete_stmt, "delete"}, - {&m_cursor_stmt, "cursor"}, }; for (const auto& [stmt_prepared, stmt_description] : statements) { @@ -387,7 +387,7 @@ void SQLiteBatch::Close() } } -bool SQLiteBatch::ReadKey(CDataStream&& key, CDataStream& value) +bool SQLiteBatch::ReadKey(DataStream&& key, DataStream& value) { if (!m_database.m_db) return false; assert(m_read_stmt); @@ -414,7 +414,7 @@ bool SQLiteBatch::ReadKey(CDataStream&& key, CDataStream& value) return true; } -bool SQLiteBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite) +bool SQLiteBatch::WriteKey(DataStream&& key, DataStream&& value, bool overwrite) { if (!m_database.m_db) return false; assert(m_insert_stmt && m_overwrite_stmt); @@ -441,7 +441,7 @@ bool SQLiteBatch::WriteKey(CDataStream&& key, CDataStream&& value, bool overwrit return res == SQLITE_DONE; } -bool SQLiteBatch::EraseKey(CDataStream&& key) +bool SQLiteBatch::EraseKey(DataStream&& key) { if (!m_database.m_db) return false; assert(m_delete_stmt); @@ -459,7 +459,7 @@ bool SQLiteBatch::EraseKey(CDataStream&& key) return res == SQLITE_DONE; } -bool SQLiteBatch::HasKey(CDataStream&& key) +bool SQLiteBatch::HasKey(DataStream&& key) { if (!m_database.m_db) return false; assert(m_read_stmt); @@ -472,28 +472,15 @@ bool SQLiteBatch::HasKey(CDataStream&& key) return res == SQLITE_ROW; } -bool SQLiteBatch::StartCursor() +DatabaseCursor::Status SQLiteCursor::Next(DataStream& key, DataStream& value) { - assert(!m_cursor_init); - if (!m_database.m_db) return false; - m_cursor_init = true; - return true; -} - -bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete) -{ - complete = false; - - if (!m_cursor_init) return false; - int res = sqlite3_step(m_cursor_stmt); if (res == SQLITE_DONE) { - complete = true; - return true; + return Status::DONE; } if (res != SQLITE_ROW) { - LogPrintf("SQLiteBatch::ReadAtCursor: Unable to execute cursor step: %s\n", sqlite3_errstr(res)); - return false; + LogPrintf("%s: Unable to execute cursor step: %s\n", __func__, sqlite3_errstr(res)); + return Status::FAIL; } // Leftmost column in result is index 0 @@ -503,13 +490,32 @@ bool SQLiteBatch::ReadAtCursor(CDataStream& key, CDataStream& value, bool& compl const std::byte* value_data{AsBytePtr(sqlite3_column_blob(m_cursor_stmt, 1))}; size_t value_data_size(sqlite3_column_bytes(m_cursor_stmt, 1)); value.write({value_data, value_data_size}); - return true; + return Status::MORE; } -void SQLiteBatch::CloseCursor() +SQLiteCursor::~SQLiteCursor() { sqlite3_reset(m_cursor_stmt); - m_cursor_init = false; + int res = sqlite3_finalize(m_cursor_stmt); + if (res != SQLITE_OK) { + LogPrintf("%s: cursor closed but could not finalize cursor statement: %s\n", + __func__, sqlite3_errstr(res)); + } +} + +std::unique_ptr<DatabaseCursor> SQLiteBatch::GetNewCursor() +{ + if (!m_database.m_db) return nullptr; + auto cursor = std::make_unique<SQLiteCursor>(); + + const char* stmt_text = "SELECT key, value FROM main"; + int res = sqlite3_prepare_v2(m_database.m_db, stmt_text, -1, &cursor->m_cursor_stmt, nullptr); + if (res != SQLITE_OK) { + throw std::runtime_error(strprintf( + "%s: Failed to setup cursor SQL statement: %s\n", __func__, sqlite3_errstr(res))); + } + + return cursor; } bool SQLiteBatch::TxnBegin() diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h index 7680bdd07b..5745a1d4cf 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_WALLET_SQLITE_H #define BITCOIN_WALLET_SQLITE_H +#include <sync.h> #include <wallet/db.h> #include <sqlite3.h> @@ -14,26 +15,34 @@ struct bilingual_str; namespace wallet { class SQLiteDatabase; +class SQLiteCursor : public DatabaseCursor +{ +public: + sqlite3_stmt* m_cursor_stmt{nullptr}; + + explicit SQLiteCursor() {} + ~SQLiteCursor() override; + + Status Next(DataStream& key, DataStream& value) override; +}; + /** RAII class that provides access to a WalletDatabase */ class SQLiteBatch : public DatabaseBatch { private: SQLiteDatabase& m_database; - bool m_cursor_init = false; - sqlite3_stmt* m_read_stmt{nullptr}; sqlite3_stmt* m_insert_stmt{nullptr}; sqlite3_stmt* m_overwrite_stmt{nullptr}; sqlite3_stmt* m_delete_stmt{nullptr}; - sqlite3_stmt* m_cursor_stmt{nullptr}; void SetupSQLStatements(); - bool ReadKey(CDataStream&& key, CDataStream& value) override; - bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite = true) override; - bool EraseKey(CDataStream&& key) override; - bool HasKey(CDataStream&& key) override; + 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; public: explicit SQLiteBatch(SQLiteDatabase& database); @@ -44,9 +53,7 @@ public: void Close() override; - bool StartCursor() override; - bool ReadAtCursor(CDataStream& key, CDataStream& value, bool& complete) override; - void CloseCursor() override; + std::unique_ptr<DatabaseCursor> GetNewCursor() override; bool TxnBegin() override; bool TxnCommit() override; bool TxnAbort() override; @@ -63,7 +70,16 @@ private: const std::string m_file_path; - void Cleanup() noexcept; + /** + * This mutex protects SQLite initialization and shutdown. + * sqlite3_config() and sqlite3_shutdown() are not thread-safe (sqlite3_initialize() is). + * Concurrent threads that execute SQLiteDatabase::SQLiteDatabase() should have just one + * of them do the init and the rest wait for it to complete before all can proceed. + */ + static Mutex g_sqlite_mutex; + static int g_sqlite_count GUARDED_BY(g_sqlite_mutex); + + void Cleanup() noexcept EXCLUSIVE_LOCKS_REQUIRED(!g_sqlite_mutex); public: SQLiteDatabase() = delete; 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 c30209002d..8f626addde 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -29,7 +29,7 @@ BOOST_FIXTURE_TEST_SUITE(coinselector_tests, WalletTestingSetup) // we repeat those tests this many times and only complain if all iterations of the test fail #define RANDOM_REPEATS 5 -typedef std::set<COutput> CoinSet; +typedef std::set<std::shared_ptr<COutput>> CoinSet; static const CoinEligibilityFilter filter_standard(1, 6, 0); static const CoinEligibilityFilter filter_confirmed(1, 1, 0); @@ -53,7 +53,7 @@ static void add_coin(const CAmount& nValue, int nInput, SelectionResult& result) tx.nLockTime = nextLockTime++; // so all transactions get different hashes COutput output(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ -1, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, /*fees=*/ 0); OutputGroup group; - group.Insert(output, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ true); + group.Insert(std::make_shared<COutput>(output), /*ancestors=*/ 0, /*descendants=*/ 0); result.AddInput(group); } @@ -65,7 +65,7 @@ static void add_coin(const CAmount& nValue, int nInput, CoinSet& set, CAmount fe tx.nLockTime = nextLockTime++; // so all transactions get different hashes COutput coin(COutPoint(tx.GetHash(), nInput), tx.vout.at(nInput), /*depth=*/ 1, /*input_bytes=*/ 148, /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, /*time=*/ 0, /*from_me=*/ false, fee); coin.long_term_fee = long_term_fee; - set.insert(coin); + set.insert(std::make_shared<COutput>(coin)); } static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmount& nValue, CFeeRate feerate = CFeeRate(0), int nAge = 6*24, bool fIsFromMe = false, int nInput =0, bool spendable = false) @@ -94,10 +94,10 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b) std::vector<CAmount> a_amts; std::vector<CAmount> b_amts; for (const auto& coin : a.GetInputSet()) { - a_amts.push_back(coin.txout.nValue); + a_amts.push_back(coin->txout.nValue); } for (const auto& coin : b.GetInputSet()) { - b_amts.push_back(coin.txout.nValue); + b_amts.push_back(coin->txout.nValue); } std::sort(a_amts.begin(), a_amts.end()); std::sort(b_amts.begin(), b_amts.end()); @@ -110,8 +110,8 @@ static bool EquivalentResult(const SelectionResult& a, const SelectionResult& b) static bool EqualResult(const SelectionResult& a, const SelectionResult& b) { std::pair<CoinSet::iterator, CoinSet::iterator> ret = std::mismatch(a.GetInputSet().begin(), a.GetInputSet().end(), b.GetInputSet().begin(), - [](const COutput& a, const COutput& b) { - return a.outpoint == b.outpoint; + [](const std::shared_ptr<COutput>& a, const std::shared_ptr<COutput>& b) { + return a->outpoint == b->outpoint; }); return ret.first == a.GetInputSet().end() && ret.second == b.GetInputSet().end(); } @@ -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; } @@ -134,12 +134,12 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& availabl static_groups.clear(); for (auto& coin : available_coins) { static_groups.emplace_back(); - static_groups.back().Insert(coin, /*ancestors=*/ 0, /*descendants=*/ 0, /*positive_only=*/ false); + static_groups.back().Insert(std::make_shared<COutput>(coin), /*ancestors=*/ 0, /*descendants=*/ 0); } return static_groups; } -inline std::vector<OutputGroup>& KnapsackGroupOutputs(const std::vector<COutput>& available_coins, CWallet& wallet, const CoinEligibilityFilter& filter) +inline std::vector<OutputGroup>& KnapsackGroupOutputs(const CoinsResult& available_coins, CWallet& wallet, const CoinEligibilityFilter& filter) { FastRandomContext rand{}; CoinSelectionParams coin_selection_params{ @@ -153,9 +153,9 @@ inline std::vector<OutputGroup>& KnapsackGroupOutputs(const std::vector<COutput> /*tx_noinputs_size=*/ 0, /*avoid_partial=*/ false, }; - static std::vector<OutputGroup> static_groups; - static_groups = GroupOutputs(wallet, available_coins, coin_selection_params, filter, /*positive_only=*/false); - return static_groups; + static OutputGroupTypeMap static_groups; + static_groups = GroupOutputs(wallet, available_coins, coin_selection_params, {{filter}})[filter]; + return static_groups.all_groups.mixed_group; } // Branch and bound coin selection tests @@ -232,17 +232,6 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 10 * CENT); expected_result.Clear(); - // Negative effective value - // Select 10 Cent but have 1 Cent not be possible because too small - add_coin(5 * CENT, 5, expected_result); - add_coin(3 * CENT, 3, expected_result); - add_coin(2 * CENT, 2, expected_result); - const auto result6 = SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 5000); - BOOST_CHECK(result6); - BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 10 * CENT); - // FIXME: this test is redundant with the above, because 1 Cent is selected, not "too small" - // BOOST_CHECK(EquivalentResult(expected_result, *result)); - // Select 0.25 Cent, not possible BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), 0.25 * CENT, 0.5 * CENT)); expected_result.Clear(); @@ -302,8 +291,10 @@ 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()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -319,14 +310,13 @@ 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); } { - std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -349,7 +339,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) BOOST_CHECK(result10); } { - std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -414,7 +404,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) FastRandomContext rand{}; const auto temp1{[&rand](std::vector<OutputGroup>& g, const CAmount& v, CAmount c) { return KnapsackSolver(g, v, c, rand); }}; const auto KnapsackSolver{temp1}; - std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -428,25 +418,25 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) available_coins.Clear(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 1 * CENT, CENT)); add_coin(available_coins, *wallet, 1*CENT, CFeeRate(0), 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 1 * CENT, CENT)); // but we can find a new 1 cent - const auto result1 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result1 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result1); BOOST_CHECK_EQUAL(result1->GetSelectedValue(), 1 * CENT); add_coin(available_coins, *wallet, 2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 3 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 3 * CENT, CENT)); // we can make 3 cents of new coins - const auto result2 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 3 * CENT, CENT); + const auto result2 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 3 * CENT, CENT); BOOST_CHECK(result2); BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 3 * CENT); @@ -457,38 +447,38 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // now we have new: 1+10=11 (of which 10 was self-sent), and mature: 2+5+20=27. total = 38 // we can't make 38 cents only if we disallow new coins: - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 38 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 38 * CENT, CENT)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard_extra), 38 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard_extra), 38 * CENT, CENT)); // but we can make 37 cents if we accept new coins from ourself - const auto result3 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 37 * CENT, CENT); + const auto result3 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 37 * CENT, CENT); BOOST_CHECK(result3); BOOST_CHECK_EQUAL(result3->GetSelectedValue(), 37 * CENT); // and we can make 38 cents if we accept all new coins - const auto result4 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 38 * CENT, CENT); + const auto result4 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 38 * CENT, CENT); BOOST_CHECK(result4); BOOST_CHECK_EQUAL(result4->GetSelectedValue(), 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - const auto result5 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 34 * CENT, CENT); + const auto result5 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 34 * CENT, CENT); BOOST_CHECK(result5); BOOST_CHECK_EQUAL(result5->GetSelectedValue(), 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(result5->GetInputSet().size(), 3U); // the best should be 20+10+5. it's incredibly unlikely the 1 or 2 got included (but possible) // when we try making 7 cents, the smaller coins (1,2,5) are enough. We should see just 2+5 - const auto result6 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 7 * CENT, CENT); + const auto result6 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 7 * CENT, CENT); BOOST_CHECK(result6); BOOST_CHECK_EQUAL(result6->GetSelectedValue(), 7 * CENT); BOOST_CHECK_EQUAL(result6->GetInputSet().size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - const auto result7 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 8 * CENT, CENT); + const auto result7 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 8 * CENT, CENT); BOOST_CHECK(result7); BOOST_CHECK(result7->GetSelectedValue() == 8 * CENT); BOOST_CHECK_EQUAL(result7->GetInputSet().size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - const auto result8 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 9 * CENT, CENT); + const auto result8 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 9 * CENT, CENT); BOOST_CHECK(result8); BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 10 * CENT); BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U); @@ -503,12 +493,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - const auto result9 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 71 * CENT, CENT); + const auto result9 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 71 * CENT, CENT); BOOST_CHECK(result9); - BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 72 * CENT, CENT)); + BOOST_CHECK(!KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 72 * CENT, CENT)); // now try making 16 cents. the best smaller coins can do is 6+7+8 = 21; not as good at the next biggest coin, 20 - const auto result10 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT); + const auto result10 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result10); BOOST_CHECK_EQUAL(result10->GetSelectedValue(), 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(result10->GetInputSet().size(), 1U); @@ -516,7 +506,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 5*CENT); // now we have 5+6+7+8+20+30 = 75 cents total // now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, better than the next biggest coin, 20 - const auto result11 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT); + const auto result11 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result11); BOOST_CHECK_EQUAL(result11->GetSelectedValue(), 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(result11->GetInputSet().size(), 3U); @@ -524,13 +514,13 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 18*CENT); // now we have 5+6+7+8+18+20+30 // and now if we try making 16 cents again, the smaller coins can make 5+6+7 = 18 cents, the same as the next biggest coin, 18 - const auto result12 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 16 * CENT, CENT); + const auto result12 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 16 * CENT, CENT); BOOST_CHECK(result12); BOOST_CHECK_EQUAL(result12->GetSelectedValue(), 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(result12->GetInputSet().size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - const auto result13 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 11 * CENT, CENT); + const auto result13 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 11 * CENT, CENT); BOOST_CHECK(result13); BOOST_CHECK_EQUAL(result13->GetSelectedValue(), 11 * CENT); BOOST_CHECK_EQUAL(result13->GetInputSet().size(), 2U); @@ -540,12 +530,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 2*COIN); add_coin(available_coins, *wallet, 3*COIN); add_coin(available_coins, *wallet, 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - const auto result14 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 95 * CENT, CENT); + const auto result14 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 95 * CENT, CENT); BOOST_CHECK(result14); BOOST_CHECK_EQUAL(result14->GetSelectedValue(), 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(result14->GetInputSet().size(), 1U); - const auto result15 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 195 * CENT, CENT); + const auto result15 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 195 * CENT, CENT); BOOST_CHECK(result15); BOOST_CHECK_EQUAL(result15->GetSelectedValue(), 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(result15->GetInputSet().size(), 1U); @@ -561,7 +551,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * CENT from the 1.5 * CENT // we'll get change smaller than CENT whatever happens, so can expect CENT exactly - const auto result16 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT, CENT); + const auto result16 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), CENT, CENT); BOOST_CHECK(result16); BOOST_CHECK_EQUAL(result16->GetSelectedValue(), CENT); @@ -569,7 +559,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, 1111*CENT); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - const auto result17 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result17 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result17); BOOST_CHECK_EQUAL(result17->GetSelectedValue(), 1 * CENT); // we should get the exact amount @@ -578,7 +568,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, CENT * 7 / 10); // and try again to make 1.0 * CENT - const auto result18 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result18 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result18); BOOST_CHECK_EQUAL(result18->GetSelectedValue(), 1 * CENT); // we should get the exact amount @@ -588,7 +578,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(available_coins, *wallet, 50000 * COIN); - const auto result19 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 500000 * COIN, CENT); + const auto result19 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 500000 * COIN, CENT); BOOST_CHECK(result19); BOOST_CHECK_EQUAL(result19->GetSelectedValue(), 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(result19->GetInputSet().size(), 10U); // in ten coins @@ -602,7 +592,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 7 / 10); add_coin(available_coins, *wallet, 1111 * CENT); - const auto result20 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 1 * CENT, CENT); + const auto result20 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result20); BOOST_CHECK_EQUAL(result20->GetSelectedValue(), 1111 * CENT); // we get the bigger coin BOOST_CHECK_EQUAL(result20->GetInputSet().size(), 1U); @@ -613,7 +603,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, CENT * 6 / 10); add_coin(available_coins, *wallet, CENT * 8 / 10); add_coin(available_coins, *wallet, 1111 * CENT); - const auto result21 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT, CENT); + const auto result21 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), CENT, CENT); BOOST_CHECK(result21); BOOST_CHECK_EQUAL(result21->GetSelectedValue(), CENT); // we should get the exact amount BOOST_CHECK_EQUAL(result21->GetInputSet().size(), 2U); // in two coins 0.4+0.6 @@ -625,13 +615,13 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(available_coins, *wallet, CENT * 100); // trying to make 100.01 from these three coins - const auto result22 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT * 10001 / 100, CENT); + const auto result22 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), CENT * 10001 / 100, CENT); BOOST_CHECK(result22); BOOST_CHECK_EQUAL(result22->GetSelectedValue(), CENT * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(result22->GetInputSet().size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - const auto result23 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), CENT * 9990 / 100, CENT); + const auto result23 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), CENT * 9990 / 100, CENT); BOOST_CHECK(result23); BOOST_CHECK_EQUAL(result23->GetSelectedValue(), 101 * CENT); BOOST_CHECK_EQUAL(result23->GetInputSet().size(), 2U); @@ -646,7 +636,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. for (int i = 0; i < RUN_TESTS; i++) { - const auto result24 = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_confirmed), 2000, CENT); + const auto result24 = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_confirmed), 2000, CENT); BOOST_CHECK(result24); if (amt - 2000 < CENT) { @@ -724,7 +714,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) BOOST_AUTO_TEST_CASE(ApproximateBestSubset) { FastRandomContext rand{}; - std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -737,7 +727,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(available_coins, *wallet, 1000 * COIN); add_coin(available_coins, *wallet, 3 * COIN); - const auto result = KnapsackSolver(KnapsackGroupOutputs(available_coins.All(), *wallet, filter_standard), 1003 * COIN, CENT, rand); + const auto result = KnapsackSolver(KnapsackGroupOutputs(available_coins, *wallet, filter_standard), 1003 * COIN, CENT, rand); BOOST_CHECK(result); BOOST_CHECK_EQUAL(result->GetSelectedValue(), 1003 * COIN); BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2U); @@ -746,7 +736,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) // Tests that with the ideal conditions, the coin selector will always be able to find a solution that can pay the target value BOOST_AUTO_TEST_CASE(SelectCoins_test) { - std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -931,9 +921,9 @@ 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) { - std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(chain, "", args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(chain, "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -941,7 +931,7 @@ static std::optional<SelectionResult> select_coins(const CAmount& target, const auto available_coins = coin_setup(*wallet); - const auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/ {}, target, cc, cs_params); + auto result = SelectCoins(*wallet, available_coins, /*pre_set_inputs=*/ {}, target, cc, cs_params); if (result) { const auto signedTxSize = 10 + 34 + 68 * result->GetInputSet().size(); // static header size + output size + inputs size (P2WPKH) BOOST_CHECK_LE(signedTxSize * WITNESS_SCALE_FACTOR, MAX_STANDARD_TX_WEIGHT); @@ -953,7 +943,7 @@ static std::optional<SelectionResult> select_coins(const CAmount& target, const static bool has_coin(const CoinSet& set, CAmount amount) { - return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin.GetEffectiveValue() == amount; }); + return std::any_of(set.begin(), set.end(), [&](const auto& coin) { return coin->GetEffectiveValue() == amount; }); } BOOST_AUTO_TEST_CASE(check_max_weight) @@ -994,7 +984,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight) add_coin(available_coins, wallet, CAmount(50 * COIN), CFeeRate(0), 144, false, 0, true); return available_coins; }, - chain, m_args); + chain); BOOST_CHECK(result); BOOST_CHECK(has_coin(result->GetInputSet(), CAmount(50 * COIN))); @@ -1019,7 +1009,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight) } return available_coins; }, - chain, m_args); + chain); BOOST_CHECK(has_coin(result->GetInputSet(), CAmount(0.0625 * COIN))); BOOST_CHECK(has_coin(result->GetInputSet(), CAmount(0.025 * COIN))); @@ -1040,7 +1030,7 @@ BOOST_AUTO_TEST_CASE(check_max_weight) } return available_coins; }, - chain, m_args); + chain); // No results // 1515 inputs * 68 bytes = 103,020 bytes @@ -1055,7 +1045,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test) // This test creates a coin whose value is higher than the target but whose effective value is lower than the target. // The coin is selected using coin control, with m_allow_other_inputs = false. SelectCoins should fail due to insufficient funds. - std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); wallet->LoadWallet(); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -1063,7 +1053,7 @@ BOOST_AUTO_TEST_CASE(SelectCoins_effective_value_test) CoinsResult available_coins; { - std::unique_ptr<CWallet> dummyWallet = std::make_unique<CWallet>(m_node.chain.get(), "dummy", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> dummyWallet = std::make_unique<CWallet>(m_node.chain.get(), "dummy", CreateMockWalletDatabase()); dummyWallet->LoadWallet(); LOCK(dummyWallet->cs_wallet); dummyWallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -1104,7 +1094,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_coinsresult_test, BasicTestingSetup) // Test case to verify CoinsResult object sanity. CoinsResult available_coins; { - std::unique_ptr<CWallet> dummyWallet = std::make_unique<CWallet>(m_node.chain.get(), "dummy", m_args, CreateMockWalletDatabase()); + std::unique_ptr<CWallet> dummyWallet = std::make_unique<CWallet>(m_node.chain.get(), "dummy", CreateMockWalletDatabase()); BOOST_CHECK_EQUAL(dummyWallet->LoadWallet(), DBErrors::LOAD_OK); LOCK(dummyWallet->cs_wallet); dummyWallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 1a0708c43a..304190eec1 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -29,8 +29,10 @@ static void GroupCoins(FuzzedDataProvider& fuzzed_data_provider, const std::vect auto output_group = OutputGroup(coin_params); bool valid_outputgroup{false}; for (auto& coin : coins) { - output_group.Insert(coin, /*ancestors=*/0, /*descendants=*/0, positive_only); - // If positive_only was specified, nothing may have been inserted, leading to an empty output group + if (!positive_only || (positive_only && coin.GetEffectiveValue() > 0)) { + output_group.Insert(std::make_shared<COutput>(coin), /*ancestors=*/0, /*descendants=*/0); + } + // If positive_only was specified, nothing was inserted, leading to an empty output group // that would be invalid for the BnB algorithm valid_outputgroup = !positive_only || output_group.GetSelectionAmount() > 0; if (valid_outputgroup && fuzzed_data_provider.ConsumeBool()) { diff --git a/src/wallet/test/group_outputs_tests.cpp b/src/wallet/test/group_outputs_tests.cpp new file mode 100644 index 0000000000..283e87989c --- /dev/null +++ b/src/wallet/test/group_outputs_tests.cpp @@ -0,0 +1,234 @@ +// 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 <test/util/setup_common.h> + +#include <wallet/coinselection.h> +#include <wallet/spend.h> +#include <wallet/wallet.h> + +#include <boost/test/unit_test.hpp> + +namespace wallet { +BOOST_FIXTURE_TEST_SUITE(group_outputs_tests, TestingSetup) + +static int nextLockTime = 0; + +static std::shared_ptr<CWallet> NewWallet(const node::NodeContext& m_node) +{ + std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockWalletDatabase()); + wallet->LoadWallet(); + LOCK(wallet->cs_wallet); + wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet->SetupDescriptorScriptPubKeyMans(); + return wallet; +} + +static void addCoin(CoinsResult& coins, + CWallet& wallet, + const CTxDestination& dest, + const CAmount& nValue, + bool is_from_me, + CFeeRate fee_rate = CFeeRate(0), + int depth = 6) +{ + CMutableTransaction tx; + tx.nLockTime = nextLockTime++; // so all transactions get different hashes + tx.vout.resize(1); + tx.vout[0].nValue = nValue; + tx.vout[0].scriptPubKey = GetScriptForDestination(dest); + + const uint256& txid = tx.GetHash(); + LOCK(wallet.cs_wallet); + auto ret = wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(txid), std::forward_as_tuple(MakeTransactionRef(std::move(tx)), TxStateInactive{})); + assert(ret.second); + CWalletTx& wtx = (*ret.first).second; + const auto& txout = wtx.tx->vout.at(0); + coins.Add(*Assert(OutputTypeFromDestination(dest)), + {COutPoint(wtx.GetHash(), 0), + txout, + depth, + CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), + /*spendable=*/ true, + /*solvable=*/ true, + /*safe=*/ true, + wtx.GetTxTime(), + is_from_me, + fee_rate}); +} + + CoinSelectionParams makeSelectionParams(FastRandomContext& rand, bool avoid_partial_spends) +{ + return CoinSelectionParams{ + rand, + /*change_output_size=*/ 0, + /*change_spend_size=*/ 0, + /*min_change_target=*/ CENT, + /*effective_feerate=*/ CFeeRate(0), + /*long_term_feerate=*/ CFeeRate(0), + /*discard_feerate=*/ CFeeRate(0), + /*tx_noinputs_size=*/ 0, + /*avoid_partial=*/ avoid_partial_spends, + }; +} + +class GroupVerifier +{ +public: + std::shared_ptr<CWallet> wallet{nullptr}; + CoinsResult coins_pool; + FastRandomContext rand; + + void GroupVerify(const OutputType type, + const CoinEligibilityFilter& filter, + bool avoid_partial_spends, + bool positive_only, + int expected_size) + { + OutputGroupTypeMap groups = GroupOutputs(*wallet, coins_pool, makeSelectionParams(rand, avoid_partial_spends), {{filter}})[filter]; + std::vector<OutputGroup>& groups_out = positive_only ? groups.groups_by_type[type].positive_group : + groups.groups_by_type[type].mixed_group; + BOOST_CHECK_EQUAL(groups_out.size(), expected_size); + } + + void GroupAndVerify(const OutputType type, + const CoinEligibilityFilter& filter, + int expected_with_partial_spends_size, + int expected_without_partial_spends_size, + bool positive_only) + { + // First avoid partial spends + GroupVerify(type, filter, /*avoid_partial_spends=*/false, positive_only, expected_with_partial_spends_size); + // Second don't avoid partial spends + GroupVerify(type, filter, /*avoid_partial_spends=*/true, positive_only, expected_without_partial_spends_size); + } +}; + +BOOST_AUTO_TEST_CASE(outputs_grouping_tests) +{ + const auto& wallet = NewWallet(m_node); + GroupVerifier group_verifier; + group_verifier.wallet = wallet; + + const CoinEligibilityFilter& BASIC_FILTER{1, 6, 0}; + + // ################################################################################# + // 10 outputs from different txs going to the same script + // 1) if partial spends is enabled --> must not be grouped + // 2) if partial spends is not enabled --> must be grouped into a single OutputGroup + // ################################################################################# + + unsigned long GROUP_SIZE = 10; + const CTxDestination dest = *Assert(wallet->GetNewDestination(OutputType::BECH32, "")); + for (unsigned long i = 0; i < GROUP_SIZE; i++) { + addCoin(group_verifier.coins_pool, *wallet, dest, 10 * COIN, /*is_from_me=*/true); + } + + group_verifier.GroupAndVerify(OutputType::BECH32, + BASIC_FILTER, + /*expected_with_partial_spends_size=*/ GROUP_SIZE, + /*expected_without_partial_spends_size=*/ 1, + /*positive_only=*/ true); + + // #################################################################################### + // 3) 10 more UTXO are added with a different script --> must be grouped into a single + // group for avoid partial spends and 10 different output groups for partial spends + // #################################################################################### + + const CTxDestination dest2 = *Assert(wallet->GetNewDestination(OutputType::BECH32, "")); + for (unsigned long i = 0; i < GROUP_SIZE; i++) { + addCoin(group_verifier.coins_pool, *wallet, dest2, 5 * COIN, /*is_from_me=*/true); + } + + group_verifier.GroupAndVerify(OutputType::BECH32, + BASIC_FILTER, + /*expected_with_partial_spends_size=*/ GROUP_SIZE * 2, + /*expected_without_partial_spends_size=*/ 2, + /*positive_only=*/ true); + + // ################################################################################ + // 4) Now add a negative output --> which will be skipped if "positive_only" is set + // ################################################################################ + + const CTxDestination dest3 = *Assert(wallet->GetNewDestination(OutputType::BECH32, "")); + addCoin(group_verifier.coins_pool, *wallet, dest3, 1, true, CFeeRate(100)); + BOOST_CHECK(group_verifier.coins_pool.coins[OutputType::BECH32].back().GetEffectiveValue() <= 0); + + // First expect no changes with "positive_only" enabled + group_verifier.GroupAndVerify(OutputType::BECH32, + BASIC_FILTER, + /*expected_with_partial_spends_size=*/ GROUP_SIZE * 2, + /*expected_without_partial_spends_size=*/ 2, + /*positive_only=*/ true); + + // Then expect changes with "positive_only" disabled + group_verifier.GroupAndVerify(OutputType::BECH32, + BASIC_FILTER, + /*expected_with_partial_spends_size=*/ GROUP_SIZE * 2 + 1, + /*expected_without_partial_spends_size=*/ 3, + /*positive_only=*/ false); + + + // ############################################################################## + // 5) Try to add a non-eligible UTXO (due not fulfilling the min depth target for + // "not mine" UTXOs) --> it must not be added to any group + // ############################################################################## + + const CTxDestination dest4 = *Assert(wallet->GetNewDestination(OutputType::BECH32, "")); + addCoin(group_verifier.coins_pool, *wallet, dest4, 6 * COIN, + /*is_from_me=*/false, CFeeRate(0), /*depth=*/5); + + // Expect no changes from this round and the previous one (point 4) + group_verifier.GroupAndVerify(OutputType::BECH32, + BASIC_FILTER, + /*expected_with_partial_spends_size=*/ GROUP_SIZE * 2 + 1, + /*expected_without_partial_spends_size=*/ 3, + /*positive_only=*/ false); + + + // ############################################################################## + // 6) Try to add a non-eligible UTXO (due not fulfilling the min depth target for + // "mine" UTXOs) --> it must not be added to any group + // ############################################################################## + + const CTxDestination dest5 = *Assert(wallet->GetNewDestination(OutputType::BECH32, "")); + addCoin(group_verifier.coins_pool, *wallet, dest5, 6 * COIN, + /*is_from_me=*/true, CFeeRate(0), /*depth=*/0); + + // Expect no changes from this round and the previous one (point 5) + group_verifier.GroupAndVerify(OutputType::BECH32, + BASIC_FILTER, + /*expected_with_partial_spends_size=*/ GROUP_SIZE * 2 + 1, + /*expected_without_partial_spends_size=*/ 3, + /*positive_only=*/ false); + + // ########################################################################################### + // 7) Surpass the OUTPUT_GROUP_MAX_ENTRIES and verify that a second partial group gets created + // ########################################################################################### + + const CTxDestination dest7 = *Assert(wallet->GetNewDestination(OutputType::BECH32, "")); + uint16_t NUM_SINGLE_ENTRIES = 101; + for (unsigned long i = 0; i < NUM_SINGLE_ENTRIES; i++) { // OUTPUT_GROUP_MAX_ENTRIES{100} + addCoin(group_verifier.coins_pool, *wallet, dest7, 9 * COIN, /*is_from_me=*/true); + } + + // Exclude partial groups only adds one more group to the previous test case (point 6) + int PREVIOUS_ROUND_COUNT = GROUP_SIZE * 2 + 1; + group_verifier.GroupAndVerify(OutputType::BECH32, + BASIC_FILTER, + /*expected_with_partial_spends_size=*/ PREVIOUS_ROUND_COUNT + NUM_SINGLE_ENTRIES, + /*expected_without_partial_spends_size=*/ 4, + /*positive_only=*/ false); + + // Include partial groups should add one more group inside the "avoid partial spends" count + const CoinEligibilityFilter& avoid_partial_groups_filter{1, 6, 0, 0, /*include_partial=*/ true}; + group_verifier.GroupAndVerify(OutputType::BECH32, + avoid_partial_groups_filter, + /*expected_with_partial_spends_size=*/ PREVIOUS_ROUND_COUNT + NUM_SINGLE_ENTRIES, + /*expected_without_partial_spends_size=*/ 5, + /*positive_only=*/ false); +} + +BOOST_AUTO_TEST_SUITE_END() +} // end namespace wallet diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index 151b09d2a6..90f369b22a 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -55,7 +55,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK compressed - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForRawPubKey(pubkeys[0]); @@ -74,7 +74,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK compressed - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "pk(" + EncodeSecret(keys[0]) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -86,7 +86,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK uncompressed - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForRawPubKey(uncompressedPubkey); @@ -105,7 +105,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PK uncompressed - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "pk(" + EncodeSecret(uncompressedKey) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -117,7 +117,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH compressed - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForDestination(PKHash(pubkeys[0])); @@ -136,7 +136,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH compressed - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "pkh(" + EncodeSecret(keys[0]) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -148,7 +148,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH uncompressed - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); scriptPubKey = GetScriptForDestination(PKHash(uncompressedPubkey)); @@ -167,7 +167,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2PKH uncompressed - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "pkh(" + EncodeSecret(uncompressedKey) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -179,7 +179,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -206,7 +206,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "sh(pkh(" + EncodeSecret(keys[0]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -219,7 +219,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2SH (invalid) - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -238,7 +238,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2SH (invalid) - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "sh(sh(" + EncodeSecret(keys[0]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, false); @@ -247,7 +247,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2WSH (invalid) - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -266,7 +266,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2SH inside P2WSH (invalid) - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "wsh(sh(" + EncodeSecret(keys[0]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, false); @@ -275,7 +275,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH inside P2WSH (invalid) - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -292,7 +292,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH inside P2WSH (invalid) - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "wsh(wpkh(" + EncodeSecret(keys[0]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, false); @@ -301,7 +301,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2WSH inside P2WSH (invalid) - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -320,7 +320,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // (P2PKH inside) P2WSH inside P2WSH (invalid) - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "wsh(wsh(" + EncodeSecret(keys[0]) + "))"; auto spk_manager = CreateDescriptor(keystore, desc_str, false); @@ -329,7 +329,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH compressed - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -345,7 +345,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH compressed - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "wpkh(" + EncodeSecret(keys[0]) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -357,7 +357,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH uncompressed - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -378,7 +378,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WPKH uncompressed (invalid) - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "wpkh(" + EncodeSecret(uncompressedKey) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, false); @@ -387,7 +387,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // scriptPubKey multisig - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -422,7 +422,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // scriptPubKey multisig - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + ")"; auto spk_manager = CreateDescriptor(keystore, desc_str, true); @@ -434,7 +434,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH multisig - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -457,7 +457,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2SH multisig - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "sh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))"; @@ -471,7 +471,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with compressed keys - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -500,7 +500,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with compressed keys - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + "))"; @@ -514,7 +514,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with uncompressed key - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); @@ -543,7 +543,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig with uncompressed key (invalid) - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "wsh(multi(2, " + EncodeSecret(uncompressedKey) + ", " + EncodeSecret(keys[1]) + "))"; @@ -553,7 +553,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig wrapped in P2SH - Legacy { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); @@ -583,7 +583,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // P2WSH multisig wrapped in P2SH - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "sh(wsh(multi(2, " + EncodeSecret(keys[0]) + ", " + EncodeSecret(keys[1]) + ")))"; @@ -598,7 +598,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Combo - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "combo(" + EncodeSecret(keys[0]) + ")"; @@ -642,7 +642,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Taproot - Descriptor { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); std::string desc_str = "tr(" + EncodeSecret(keys[0]) + ")"; @@ -660,7 +660,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // OP_RETURN { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -675,7 +675,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // witness unspendable { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -690,7 +690,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // witness unknown { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); @@ -705,7 +705,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Nonstandard { - CWallet keystore(chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet keystore(chain.get(), "", CreateDummyWalletDatabase()); keystore.SetupLegacyScriptPubKeyMan(); LOCK(keystore.GetLegacyScriptPubKeyMan()->cs_KeyStore); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); diff --git a/src/wallet/test/scriptpubkeyman_tests.cpp b/src/wallet/test/scriptpubkeyman_tests.cpp index a524b85ccb..90042f5252 100644 --- a/src/wallet/test/scriptpubkeyman_tests.cpp +++ b/src/wallet/test/scriptpubkeyman_tests.cpp @@ -18,7 +18,7 @@ BOOST_FIXTURE_TEST_SUITE(scriptpubkeyman_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(CanProvide) { // Set up wallet and keyman variables. - CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); LegacyScriptPubKeyMan& keyman = *wallet.GetOrCreateLegacyScriptPubKeyMan(); // Make a 1 of 2 multisig script diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 364cc5c20b..6e87d1cb49 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -18,7 +18,7 @@ BOOST_FIXTURE_TEST_SUITE(spend_tests, WalletTestingSetup) BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - auto wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), m_args, coinbaseKey); + auto wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), coinbaseKey); // Check that a subtract-from-recipient transaction slightly less than the // coinbase input amount does not create a change output (because it would @@ -118,7 +118,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup) // Add 4 spendable UTXO, 50 BTC each, to the wallet (total balance 200 BTC) for (int i = 0; i < 4; i++) CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - auto wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), m_args, coinbaseKey); + auto wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), coinbaseKey); LOCK(wallet->cs_wallet); auto available_coins = AvailableCoins(*wallet); diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index 88597bd320..b7bf312edf 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -14,9 +14,9 @@ #include <memory> namespace wallet { -std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, ArgsManager& args, const CKey& key) +std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key) { - auto wallet = std::make_unique<CWallet>(&chain, "", args, CreateMockWalletDatabase()); + auto wallet = std::make_unique<CWallet>(&chain, "", CreateMockWalletDatabase()); { LOCK2(wallet->cs_wallet, ::cs_main); wallet->SetLastBlockProcessed(cchain.Height(), cchain.Tip()->GetBlockHash()); @@ -50,18 +50,18 @@ std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, // Get a cursor to the original database auto batch = database.MakeBatch(); - batch->StartCursor(); + std::unique_ptr<wallet::DatabaseCursor> cursor = batch->GetNewCursor(); // Get a batch for the new database auto new_batch = new_database->MakeBatch(); // Read all records from the original database and write them to the new one while (true) { - CDataStream key(SER_DISK, CLIENT_VERSION); - CDataStream value(SER_DISK, CLIENT_VERSION); - bool complete; - batch->ReadAtCursor(key, value, complete); - if (complete) break; + DataStream key{}; + DataStream value{}; + DatabaseCursor::Status status = cursor->Next(key, value); + assert(status != DatabaseCursor::Status::FAIL); + if (status == DatabaseCursor::Status::DONE) break; new_batch->Write(key, value); } diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h index 635a5152ec..d726517e21 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -21,7 +21,7 @@ class CWallet; struct DatabaseOptions; class WalletDatabase; -std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, ArgsManager& args, const CKey& key); +std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cchain, const CKey& key); // Creates a copy of the provided database std::unique_ptr<WalletDatabase> DuplicateMockDatabase(WalletDatabase& database, DatabaseOptions& options); diff --git a/src/wallet/test/wallet_crypto_tests.cpp b/src/wallet/test/wallet_crypto_tests.cpp index 6b8542f378..d5e75bb892 100644 --- a/src/wallet/test/wallet_crypto_tests.cpp +++ b/src/wallet/test/wallet_crypto_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <test/util/random.h> #include <test/util/setup_common.h> #include <util/strencodings.h> #include <wallet/crypter.h> diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index c47e56c093..2dd8f9ad33 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -10,7 +10,7 @@ namespace wallet { WalletTestingSetup::WalletTestingSetup(const std::string& chainName) : TestingSetup(chainName), m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args))}, - m_wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()) + m_wallet(m_node.chain.get(), "", CreateMockWalletDatabase()) { m_wallet.LoadWallet(); m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7ae10c23ca..2e95a14807 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -43,7 +43,7 @@ static_assert(WALLET_INCREMENTAL_RELAY_FEE >= DEFAULT_INCREMENTAL_RELAY_FEE, "wa BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) +static std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) { DatabaseOptions options; options.create_flags = WALLET_FLAG_DESCRIPTORS; @@ -98,7 +98,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions fails to read an unknown start block. { - CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); LOCK(Assert(m_node.chainman)->GetMutex()); @@ -119,7 +119,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { - CWallet wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", CreateMockWalletDatabase()); { LOCK(wallet.cs_wallet); LOCK(Assert(m_node.chainman)->GetMutex()); @@ -164,7 +164,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions only picks transactions in the new block // file. { - CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); LOCK(Assert(m_node.chainman)->GetMutex()); @@ -192,7 +192,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) // Verify ScanForWalletTransactions scans no blocks. { - CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); LOCK(Assert(m_node.chainman)->GetMutex()); @@ -232,7 +232,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) // before the missing block, and success for a key whose creation time is // after. { - const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); WITH_LOCK(wallet->cs_wallet, wallet->SetLastBlockProcessed(newTip->nHeight, newTip->GetBlockHash())); WalletContext context; @@ -298,7 +298,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) { WalletContext context; context.args = &m_args; - const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); { auto spk_man = wallet->GetOrCreateLegacyScriptPubKeyMan(); LOCK2(wallet->cs_wallet, spk_man->cs_KeyStore); @@ -321,7 +321,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME // were scanned, and no prior blocks were scanned. { - const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); LOCK(wallet->cs_wallet); wallet->SetupLegacyScriptPubKeyMan(); @@ -355,7 +355,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) // debit functions. BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { - CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); LOCK(wallet.cs_wallet); LOCK(Assert(m_node.chainman)->GetMutex()); @@ -527,7 +527,7 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), m_args, coinbaseKey); + wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), coinbaseKey); } ~ListCoinsTestingSetup() @@ -622,10 +622,50 @@ 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) { { - const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); wallet->SetupLegacyScriptPubKeyMan(); wallet->SetMinVersion(FEATURE_LATEST); wallet->SetWalletFlag(WALLET_FLAG_DISABLE_PRIVATE_KEYS); @@ -633,7 +673,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_disableprivkeys, TestChain100Setup) BOOST_CHECK(!wallet->GetNewDestination(OutputType::BECH32, "")); } { - const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); + const std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>(m_node.chain.get(), "", CreateDummyWalletDatabase()); LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet->SetMinVersion(FEATURE_LATEST); @@ -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; @@ -867,24 +907,28 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) TestUnloadWallet(std::move(wallet)); } +class FailCursor : public DatabaseCursor +{ +public: + Status Next(DataStream& key, DataStream& value) override { return Status::FAIL; } +}; + /** RAII class that provides access to a FailDatabase. Which fails if needed. */ class FailBatch : public DatabaseBatch { private: bool m_pass{true}; - bool ReadKey(CDataStream&& key, CDataStream& value) override { return m_pass; } - bool WriteKey(CDataStream&& key, CDataStream&& value, bool overwrite=true) override { return m_pass; } - bool EraseKey(CDataStream&& key) override { return m_pass; } - bool HasKey(CDataStream&& key) override { return m_pass; } + bool ReadKey(DataStream&& key, DataStream& value) override { return m_pass; } + 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; } public: explicit FailBatch(bool pass) : m_pass(pass) {} void Flush() override {} void Close() override {} - bool StartCursor() override { return true; } - bool ReadAtCursor(CDataStream& ssKey, CDataStream& ssValue, bool& complete) override { return false; } - void CloseCursor() override {} + std::unique_ptr<DatabaseCursor> GetNewCursor() override { return std::make_unique<FailCursor>(); } bool TxnBegin() override { return false; } bool TxnCommit() override { return false; } bool TxnAbort() override { return false; } @@ -917,7 +961,7 @@ public: */ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup) { - CWallet wallet(m_node.chain.get(), "", m_args, std::make_unique<FailDatabase>()); + CWallet wallet(m_node.chain.get(), "", std::make_unique<FailDatabase>()); { LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); @@ -946,7 +990,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 +1011,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/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp index 24d21c2f22..9f5a4b14d3 100644 --- a/src/wallet/test/walletload_tests.cpp +++ b/src/wallet/test/walletload_tests.cpp @@ -46,7 +46,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup) { // Now try to load the wallet and verify the error. - const std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(database))); + const std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(database))); BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::UNKNOWN_DESCRIPTOR); } } @@ -54,13 +54,15 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup) bool HasAnyRecordOfType(WalletDatabase& db, const std::string& key) { std::unique_ptr<DatabaseBatch> batch = db.MakeBatch(false); - BOOST_CHECK(batch->StartCursor()); + BOOST_CHECK(batch); + std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor(); + BOOST_CHECK(cursor); while (true) { - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - BOOST_CHECK(batch->ReadAtCursor(ssKey, ssValue, complete)); - if (complete) break; + DataStream ssKey{}; + DataStream ssValue{}; + DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); + assert(status != DatabaseCursor::Status::FAIL); + if (status == DatabaseCursor::Status::DONE) break; std::string type; ssKey >> type; if (type == key) return true; @@ -82,7 +84,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup) { // Context setup. // Create and encrypt legacy wallet - std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase())); + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", CreateMockWalletDatabase())); LOCK(wallet->cs_wallet); auto legacy_spkm = wallet->GetOrCreateLegacyScriptPubKeyMan(); BOOST_CHECK(legacy_spkm->SetupGeneration(true)); @@ -110,7 +112,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup) // the records every time that 'CWallet::Unlock' gets called, which is not good. // Load the wallet and check that is encrypted - std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, get_db(dbs))); + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", get_db(dbs))); BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK); BOOST_CHECK(wallet->IsCrypted()); BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); @@ -136,7 +138,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup) } // Load the wallet and check that is encrypted - std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(db))); + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(db))); BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::LOAD_OK); BOOST_CHECK(wallet->IsCrypted()); BOOST_CHECK(HasAnyRecordOfType(wallet->GetDatabase(), DBKeys::CRYPTED_KEY)); @@ -164,7 +166,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup) BOOST_CHECK(batch->Write(key, value, /*fOverwrite=*/true)); } - std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(db))); + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(db))); BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT); } @@ -180,7 +182,7 @@ BOOST_FIXTURE_TEST_CASE(wallet_load_verif_crypted_key_checksum, TestingSetup) BOOST_CHECK(db->MakeBatch(false)->Write(key, value, /*fOverwrite=*/true)); } - std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", m_args, std::move(db))); + std::shared_ptr<CWallet> wallet(new CWallet(m_node.chain.get(), "", std::move(db))); BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::CORRUPT); } } diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 6ad222864a..290ef4eaa9 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -293,6 +293,7 @@ public: bool isAbandoned() const { return state<TxStateInactive>() && state<TxStateInactive>()->abandoned; } bool isConflicted() const { return state<TxStateConflicted>(); } + bool isInactive() const { return state<TxStateInactive>(); } bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } bool isConfirmed() const { return state<TxStateConfirmed>(); } const uint256& GetHash() const { return tx->GetHash(); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d8d6c56cd5..ef8fb29e64 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -26,6 +26,7 @@ #include <script/descriptor.h> #include <script/script.h> #include <script/signingprovider.h> +#include <support/cleanse.h> #include <txmempool.h> #include <util/bip32.h> #include <util/check.h> @@ -34,6 +35,7 @@ #include <util/moneystr.h> #include <util/rbf.h> #include <util/string.h> +#include <util/system.h> #include <util/translation.h> #include <wallet/coincontrol.h> #include <wallet/context.h> @@ -241,7 +243,7 @@ std::shared_ptr<CWallet> LoadWalletInternal(WalletContext& context, const std::s } context.chain->initMessage(_("Loading wallet…").translated); - const std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings); + std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), options.create_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet loading failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_LOAD; @@ -381,7 +383,7 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& // Make the wallet context.chain->initMessage(_("Loading wallet…").translated); - const std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), wallet_creation_flags, error, warnings); + std::shared_ptr<CWallet> wallet = CWallet::Create(context, name, std::move(database), wallet_creation_flags, error, warnings); if (!wallet) { error = Untranslated("Wallet creation failed.") + Untranslated(" ") + error; status = DatabaseStatus::FAILED_CREATE; @@ -472,8 +474,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; @@ -551,7 +552,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, bool fWasLocked = IsLocked(); { - LOCK(cs_wallet); + LOCK2(m_relock_mutex, cs_wallet); Lock(); CCrypter crypter; @@ -648,8 +649,7 @@ bool CWallet::HasWalletSpend(const CTransactionRef& tx) const AssertLockHeld(cs_wallet); const uint256& txid = tx->GetHash(); for (unsigned int i = 0; i < tx->vout.size(); ++i) { - auto iter = mapTxSpends.find(COutPoint(txid, i)); - if (iter != mapTxSpends.end()) { + if (IsSpent(COutPoint(txid, i))) { return true; } } @@ -787,7 +787,7 @@ bool CWallet::EncryptWallet(const SecureString& strWalletPassphrase) return false; { - LOCK(cs_wallet); + LOCK2(m_relock_mutex, cs_wallet); mapMasterKeys[++nMasterKeyMaxID] = kMasterKey; WalletBatch* encrypted_batch = new WalletBatch(GetDatabase()); if (!encrypted_batch->TxnBegin()) { @@ -1067,6 +1067,33 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const } } + // Mark inactive coinbase transactions and their descendants as abandoned + if (wtx.IsCoinBase() && wtx.isInactive()) { + std::vector<CWalletTx*> txs{&wtx}; + + TxStateInactive inactive_state = TxStateInactive{/*abandoned=*/true}; + + while (!txs.empty()) { + CWalletTx* desc_tx = txs.back(); + txs.pop_back(); + desc_tx->m_state = inactive_state; + // Break caches since we have changed the state + desc_tx->MarkDirty(); + batch.WriteTx(*desc_tx); + MarkInputsDirty(desc_tx->tx); + for (unsigned int i = 0; i < desc_tx->tx->vout.size(); ++i) { + COutPoint outpoint(desc_tx->GetHash(), i); + std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(outpoint); + for (TxSpends::const_iterator it = range.first; it != range.second; ++it) { + const auto wit = mapWallet.find(it->second); + if (wit != mapWallet.end()) { + txs.push_back(&wit->second); + } + } + } + } + } + //// debug print WalletLogPrintf("AddToWallet %s %s%s\n", hash.ToString(), (fInsertedNew ? "new" : ""), (fUpdated ? "update" : "")); @@ -1083,7 +1110,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const #if HAVE_SYSTEM // notify an external script when a wallet transaction comes in or is updated - std::string strCmd = m_args.GetArg("-walletnotify", ""); + std::string strCmd = m_notify_tx_changed_script; if (!strCmd.empty()) { @@ -1276,7 +1303,11 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) wtx.MarkDirty(); batch.WriteTx(wtx); NotifyTransactionChanged(wtx.GetHash(), CT_UPDATED); - // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too + // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too. + // States are not permanent, so these transactions can become unabandoned if they are re-added to the + // mempool, or confirmed in a block, or conflicted. + // Note: If the reorged coinbase is re-added to the main chain, the descendants that have not had their + // states change will remain abandoned and will require manual broadcast if the user wants them. for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) { std::pair<TxSpends::const_iterator, TxSpends::const_iterator> range = mapTxSpends.equal_range(COutPoint(now, i)); for (TxSpends::const_iterator iter = range.first; iter != range.second; ++iter) { @@ -1355,7 +1386,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 +1396,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 +1442,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); } } @@ -1611,15 +1642,15 @@ void CWallet::InitWalletFlags(uint64_t flags) // Helper for producing a max-sized low-S low-R signature (eg 71 bytes) // or a max-sized low-S signature (e.g. 72 bytes) depending on coin_control -bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, const CCoinControl* coin_control) +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool can_grind_r, const CCoinControl* coin_control) { // Fill in dummy signatures for fee calculation. const CScript& scriptPubKey = txout.scriptPubKey; SignatureData sigdata; - // Use max sig if watch only inputs were used or if this particular input is an external input - // to ensure a sufficient fee is attained for the requested feerate. - const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(tx_in.prevout)); + // Use max sig if watch only inputs were used, if this particular input is an external input, + // or if this wallet uses an external signer, to ensure a sufficient fee is attained for the requested feerate. + const bool use_max_sig = coin_control && (coin_control->fAllowWatchOnly || coin_control->IsExternalSelected(tx_in.prevout) || !can_grind_r); if (!ProduceSignature(provider, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, scriptPubKey, sigdata)) { return false; } @@ -1675,6 +1706,7 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> { // Fill in dummy signatures for fee calculation. int nIn = 0; + const bool can_grind_r = CanGrindR(); for (const auto& txout : txouts) { CTxIn& txin = txNew.vin[nIn]; @@ -1687,8 +1719,8 @@ bool CWallet::DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> continue; } const std::unique_ptr<SigningProvider> provider = GetSolvingProvider(txout.scriptPubKey); - if (!provider || !DummySignInput(*provider, txin, txout, coin_control)) { - if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, coin_control)) { + if (!provider || !DummySignInput(*provider, txin, txout, can_grind_r, coin_control)) { + if (!coin_control || !DummySignInput(coin_control->m_external_provider, txin, txout, can_grind_r, coin_control)) { return false; } } @@ -2884,7 +2916,11 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri const auto start{SteadyClock::now()}; // TODO: Can't use std::make_shared because we need a custom deleter but // should be possible to use std::allocate_shared. - const std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, args, std::move(database)), ReleaseWallet); + std::shared_ptr<CWallet> walletInstance(new CWallet(chain, name, std::move(database)), ReleaseWallet); + walletInstance->m_keypool_size = std::max(args.GetIntArg("-keypool", DEFAULT_KEYPOOL_SIZE), int64_t{1}); + walletInstance->m_notify_tx_changed_script = args.GetArg("-walletnotify", ""); + + // Load wallet bool rescan_required = false; DBErrors nLoadWalletRet = walletInstance->LoadWallet(); if (nLoadWalletRet != DBErrors::LOAD_OK) { @@ -2997,7 +3033,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-mintxfee")) { std::optional<CAmount> min_tx_fee = ParseMoney(args.GetArg("-mintxfee", "")); - if (!min_tx_fee || min_tx_fee.value() == 0) { + if (!min_tx_fee) { error = AmountErrMsg("mintxfee", args.GetArg("-mintxfee", "")); return nullptr; } else if (min_tx_fee.value() > HIGH_TX_FEE_PER_KB) { @@ -3027,7 +3063,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-fallbackfee")) { std::optional<CAmount> fallback_fee = ParseMoney(args.GetArg("-fallbackfee", "")); if (!fallback_fee) { - error = strprintf(_("Invalid amount for -fallbackfee=<amount>: '%s'"), args.GetArg("-fallbackfee", "")); + error = strprintf(_("Invalid amount for %s=<amount>: '%s'"), "-fallbackfee", args.GetArg("-fallbackfee", "")); return nullptr; } else if (fallback_fee.value() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-fallbackfee") + Untranslated(" ") + @@ -3042,7 +3078,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri if (args.IsArgSet("-discardfee")) { std::optional<CAmount> discard_fee = ParseMoney(args.GetArg("-discardfee", "")); if (!discard_fee) { - error = strprintf(_("Invalid amount for -discardfee=<amount>: '%s'"), args.GetArg("-discardfee", "")); + error = strprintf(_("Invalid amount for %s=<amount>: '%s'"), "-discardfee", args.GetArg("-discardfee", "")); return nullptr; } else if (discard_fee.value() > HIGH_TX_FEE_PER_KB) { warnings.push_back(AmountHighWarn("-discardfee") + Untranslated(" ") + @@ -3064,8 +3100,8 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri walletInstance->m_pay_tx_fee = CFeeRate{pay_tx_fee.value(), 1000}; if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for -paytxfee=<amount>: '%s' (must be at least %s)"), - args.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); + error = strprintf(_("Invalid amount for %s=<amount>: '%s' (must be at least %s)"), + "-paytxfee", args.GetArg("-paytxfee", ""), chain->relayMinFee().ToString()); return nullptr; } } @@ -3076,12 +3112,12 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri error = AmountErrMsg("maxtxfee", args.GetArg("-maxtxfee", "")); return nullptr; } else if (max_fee.value() > HIGH_MAX_TX_FEE) { - warnings.push_back(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction.")); + warnings.push_back(strprintf(_("%s is set very high! Fees this large could be paid on a single transaction."), "-maxtxfee")); } if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) { - error = strprintf(_("Invalid amount for -maxtxfee=<amount>: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), - args.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); + error = strprintf(_("Invalid amount for %s=<amount>: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"), + "-maxtxfee", args.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString()); return nullptr; } @@ -3184,6 +3220,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 +3272,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)) { @@ -3370,8 +3413,11 @@ bool CWallet::Lock() return false; { - LOCK(cs_wallet); - vMasterKey.clear(); + LOCK2(m_relock_mutex, cs_wallet); + if (!vMasterKey.empty()) { + memory_cleanse(vMasterKey.data(), vMasterKey.size() * sizeof(decltype(vMasterKey)::value_type)); + vMasterKey.clear(); + } } NotifyStatusChanged(this); @@ -3498,7 +3544,7 @@ void CWallet::SetupLegacyScriptPubKeyMan() return; } - auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new LegacyScriptPubKeyMan(*this)); + auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new LegacyScriptPubKeyMan(*this, m_keypool_size)); for (const auto& type : LEGACY_OUTPUT_TYPES) { m_internal_spk_managers[type] = spk_manager.get(); m_external_spk_managers[type] = spk_manager.get(); @@ -3527,10 +3573,10 @@ void CWallet::ConnectScriptPubKeyManNotifiers() void CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc) { if (IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)) { - auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, desc)); + auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, desc, m_keypool_size)); m_spk_managers[id] = std::move(spk_manager); } else { - auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc)); + auto spk_manager = std::unique_ptr<ScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc, m_keypool_size)); m_spk_managers[id] = std::move(spk_manager); } } @@ -3541,7 +3587,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { - auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this)); + auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, m_keypool_size)); if (IsCrypted()) { if (IsLocked()) { throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); @@ -3597,7 +3643,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() continue; } OutputType t = *desc->GetOutputType(); - auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this)); + auto spk_manager = std::unique_ptr<ExternalSignerScriptPubKeyMan>(new ExternalSignerScriptPubKeyMan(*this, m_keypool_size)); spk_manager->SetupDescriptor(std::move(desc)); uint256 id = spk_manager->GetID(); m_spk_managers[id] = std::move(spk_manager); @@ -3713,7 +3759,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat WalletLogPrintf("Update existing descriptor: %s\n", desc.descriptor->ToString()); spk_man->UpdateWalletDescriptor(desc); } else { - auto new_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc)); + auto new_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc, m_keypool_size)); spk_man = new_spk_man.get(); // Save the descriptor to memory @@ -3770,26 +3816,27 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) // Get all of the records for DB type migration std::unique_ptr<DatabaseBatch> batch = m_database->MakeBatch(); + std::unique_ptr<DatabaseCursor> cursor = batch->GetNewCursor(); std::vector<std::pair<SerializeData, SerializeData>> records; - if (!batch->StartCursor()) { + if (!cursor) { error = _("Error: Unable to begin reading all records in the database"); return false; } - bool complete = false; + DatabaseCursor::Status status = DatabaseCursor::Status::FAIL; while (true) { - CDataStream ss_key(SER_DISK, CLIENT_VERSION); - CDataStream ss_value(SER_DISK, CLIENT_VERSION); - bool ret = batch->ReadAtCursor(ss_key, ss_value, complete); - if (!ret) { + DataStream ss_key{}; + DataStream ss_value{}; + status = cursor->Next(ss_key, ss_value); + if (status != DatabaseCursor::Status::MORE) { break; } SerializeData key(ss_key.begin(), ss_key.end()); SerializeData value(ss_value.begin(), ss_value.end()); records.emplace_back(key, value); } - batch->CloseCursor(); + cursor.reset(); batch.reset(); - if (!complete) { + if (status != DatabaseCursor::Status::DONE) { error = _("Error: Unable to read all records in the database"); return false; } @@ -3815,8 +3862,8 @@ bool CWallet::MigrateToSQLite(bilingual_str& error) bool began = batch->TxnBegin(); assert(began); // This is a critical error, the new db could not be written to. The original db exists as a backup, but we should not continue execution. for (const auto& [key, value] : records) { - CDataStream ss_key(key, SER_DISK, CLIENT_VERSION); - CDataStream ss_value(value, SER_DISK, CLIENT_VERSION); + DataStream ss_key{key}; + DataStream ss_value{value}; if (!batch->Write(ss_key, ss_value)) { batch->TxnAbort(); m_database->Close(); @@ -3834,14 +3881,11 @@ std::optional<MigrationData> CWallet::GetDescriptorsForLegacy(bilingual_str& err AssertLockHeld(cs_wallet); LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); - if (!legacy_spkm) { - error = _("Error: This wallet is already a descriptor wallet"); - return std::nullopt; - } + assert(legacy_spkm); std::optional<MigrationData> res = legacy_spkm->MigrateToDescriptor(); if (res == std::nullopt) { - error = _("Error: Unable to produce descriptors for this legacy wallet. Make sure the wallet is unlocked first"); + error = _("Error: Unable to produce descriptors for this legacy wallet. Make sure to provide the wallet's passphrase if it is encrypted."); return std::nullopt; } return res; @@ -4002,6 +4046,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) { @@ -4021,6 +4082,11 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) return true; } +bool CWallet::CanGrindR() const +{ + return !IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER); +} + bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, MigrationResult& res) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { AssertLockHeld(wallet.cs_wallet); @@ -4114,27 +4180,19 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, return true; } -util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>&& wallet, WalletContext& context) +util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context) { MigrationResult res; bilingual_str error; std::vector<bilingual_str> warnings; - // Make a backup of the DB - std::string wallet_name = wallet->GetName(); - fs::path this_wallet_dir = fs::absolute(fs::PathFromString(wallet->GetDatabase().Filename())).parent_path(); - fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", wallet_name, GetTime())); - fs::path backup_path = this_wallet_dir / backup_filename; - if (!wallet->BackupWallet(fs::PathToString(backup_path))) { - return util::Error{_("Error: Unable to make a backup of your wallet")}; - } - res.backup_path = backup_path; - - // Unload the wallet so that nothing else tries to use it while we're changing it - if (!RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings)) { - return util::Error{_("Unable to unload the wallet before migrating")}; + // If the wallet is still loaded, unload it so that nothing else tries to use it while we're changing it + if (auto wallet = GetWallet(context, wallet_name)) { + if (!RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings)) { + return util::Error{_("Unable to unload the wallet before migrating")}; + } + UnloadWallet(std::move(wallet)); } - UnloadWallet(std::move(wallet)); // Load the wallet but only in the context of this function. // No signals should be connected nor should anything else be aware of this wallet @@ -4148,15 +4206,43 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> return util::Error{Untranslated("Wallet file verification failed.") + Untranslated(" ") + error}; } + // Make the local wallet std::shared_ptr<CWallet> local_wallet = CWallet::Create(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings); if (!local_wallet) { return util::Error{Untranslated("Wallet loading failed.") + Untranslated(" ") + error}; } + // Before anything else, check if there is something to migrate. + if (!local_wallet->GetLegacyScriptPubKeyMan()) { + return util::Error{_("Error: This wallet is already a descriptor wallet")}; + } + + // Make a backup of the DB + fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path(); + fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", wallet_name, GetTime())); + fs::path backup_path = this_wallet_dir / backup_filename; + if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { + return util::Error{_("Error: Unable to make a backup of your wallet")}; + } + res.backup_path = backup_path; + bool success = false; { LOCK(local_wallet->cs_wallet); + // Unlock the wallet if needed + if (local_wallet->IsLocked() && !local_wallet->Unlock(passphrase)) { + if (passphrase.find('\0') == std::string::npos) { + return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect.")}; + } else { + return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase entered was incorrect. " + "The passphrase contains a null character (ie - a zero byte). " + "If this passphrase was set with a version of this software prior to 25.0, " + "please try again with only the characters up to — but not including — " + "the first null character.")}; + } + } + // First change to using SQLite if (!local_wallet->MigrateToSQLite(error)) return util::Error{error}; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 57b5f7fc31..e8c18dbb67 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -10,6 +10,7 @@ #include <fs.h> #include <interfaces/chain.h> #include <interfaces/handler.h> +#include <logging.h> #include <outputtype.h> #include <policy/feerate.h> #include <psbt.h> @@ -19,7 +20,6 @@ #include <util/result.h> #include <util/strencodings.h> #include <util/string.h> -#include <util/system.h> #include <util/time.h> #include <util/ui_change_type.h> #include <validationinterface.h> @@ -243,6 +243,7 @@ private: std::atomic<bool> fAbortRescan{false}; std::atomic<bool> fScanningWallet{false}; // controlled by WalletRescanReserver std::atomic<bool> m_attaching_chain{false}; + std::atomic<bool> m_scanning_with_passphrase{false}; std::atomic<int64_t> m_scanning_start{0}; std::atomic<double> m_scanning_progress{0}; friend class WalletRescanReserver; @@ -307,9 +308,6 @@ private: //! Unset the blank wallet flag and saves it to disk void UnsetBlankWalletFlag(WalletBatch& batch) override; - /** Provider of aplication-wide arguments. */ - const ArgsManager& m_args; - /** Interface for accessing chain state. */ interfaces::Chain* m_chain; @@ -373,9 +371,8 @@ public: unsigned int nMasterKeyMaxID = 0; /** Construct wallet with specified name and database implementation. */ - CWallet(interfaces::Chain* chain, const std::string& name, const ArgsManager& args, std::unique_ptr<WalletDatabase> database) - : m_args(args), - m_chain(chain), + CWallet(interfaces::Chain* chain, const std::string& name, std::unique_ptr<WalletDatabase> database) + : m_chain(chain), m_name(name), m_database(std::move(database)) { @@ -467,6 +464,7 @@ public: void AbortRescan() { fAbortRescan = true; } bool IsAbortingRescan() const { return fAbortRescan; } bool IsScanning() const { return fScanningWallet; } + bool IsScanningWithPassphrase() const { return m_scanning_with_passphrase; } int64_t ScanningDuration() const { return fScanningWallet ? GetTimeMillis() - m_scanning_start : 0; } double ScanningProgress() const { return fScanningWallet ? (double) m_scanning_progress : 0; } @@ -486,6 +484,9 @@ public: // Used to prevent concurrent calls to walletpassphrase RPC. Mutex m_unlock_mutex; + // Used to prevent deleting the passphrase from memory when it is still in use. + RecursiveMutex m_relock_mutex; + bool Unlock(const SecureString& strWalletPassphrase, bool accept_no_keys = false); bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase); @@ -516,7 +517,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 +539,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. */ @@ -591,12 +592,6 @@ public: bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - bool DummySignTx(CMutableTransaction &txNew, const std::set<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const - { - std::vector<CTxOut> v_txouts(txouts.size()); - std::copy(txouts.begin(), txouts.end(), v_txouts.begin()); - return DummySignTx(txNew, v_txouts, coin_control); - } bool DummySignTx(CMutableTransaction &txNew, const std::vector<CTxOut> &txouts, const CCoinControl* coin_control = nullptr) const; bool ImportScripts(const std::set<CScript> scripts, int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -642,6 +637,12 @@ public: /** Absolute maximum transaction fee (in satoshis) used by default for the wallet */ CAmount m_default_max_tx_fee{DEFAULT_TRANSACTION_MAXFEE}; + /** Number of pre-generated keys/scripts by each spkm (part of the look-ahead process, used to detect payments) */ + int64_t m_keypool_size{DEFAULT_KEYPOOL_SIZE}; + + /** Notify external script when a wallet transaction comes in or is updated (handled by -walletnotify) */ + std::string m_notify_tx_changed_script; + size_t KeypoolCountExternalKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool TopUpKeyPool(unsigned int kpSize = 0); @@ -821,7 +822,8 @@ public: bool IsLegacy() const; /** Returns a bracketed wallet name for displaying in logs, will return [default wallet] if the wallet has no name */ - const std::string GetDisplayName() const override { + std::string GetDisplayName() const override + { std::string wallet_name = GetName().length() == 0 ? "default wallet" : GetName(); return strprintf("[%s]", wallet_name); }; @@ -939,6 +941,9 @@ public: //! Adds the ScriptPubKeyMans given in MigrationData to this wallet, removes LegacyScriptPubKeyMan, //! and where needed, moves tx and address book entries to watchonly_wallet or solvable_wallet bool ApplyMigrationData(MigrationData& data, bilingual_str& error) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + //! Whether the (external) signer performs R-value signature grinding + bool CanGrindR() const; }; /** @@ -954,17 +959,18 @@ private: using Clock = std::chrono::steady_clock; using NowFn = std::function<Clock::time_point()>; CWallet& m_wallet; - bool m_could_reserve; + bool m_could_reserve{false}; NowFn m_now; public: - explicit WalletRescanReserver(CWallet& w) : m_wallet(w), m_could_reserve(false) {} + explicit WalletRescanReserver(CWallet& w) : m_wallet(w) {} - bool reserve() + bool reserve(bool with_passphrase = false) { assert(!m_could_reserve); if (m_wallet.fScanningWallet.exchange(true)) { return false; } + m_wallet.m_scanning_with_passphrase.exchange(with_passphrase); m_wallet.m_scanning_start = GetTimeMillis(); m_wallet.m_scanning_progress = 0; m_could_reserve = true; @@ -984,6 +990,7 @@ public: { if (m_could_reserve) { m_wallet.fScanningWallet = false; + m_wallet.m_scanning_with_passphrase = false; } } }; @@ -994,7 +1001,7 @@ bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); //! Remove wallet name from persistent configuration so it will not be loaded on startup. bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); -bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, const CCoinControl* coin_control = nullptr); +bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, bool can_grind_r, const CCoinControl* coin_control); bool FillInputToWeight(CTxIn& txin, int64_t target_weight); @@ -1006,7 +1013,7 @@ struct MigrationResult { }; //! Do all steps to migrate a legacy wallet to a descriptor wallet -util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>&& wallet, WalletContext& context); +util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context); } // namespace wallet #endif // BITCOIN_WALLET_WALLET_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index d560de863a..2cd35ae40e 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -321,7 +321,7 @@ public: }; static bool -ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, +ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, CWalletScanState &wss, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr) EXCLUSIVE_LOCKS_REQUIRED(pwallet->cs_wallet) { try { @@ -759,7 +759,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, return true; } -bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn) +bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn) { CWalletScanState dummy_wss; LOCK(pwallet->cs_wallet); @@ -812,7 +812,8 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) #endif // Get cursor - if (!m_batch->StartCursor()) + std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor(); + if (!cursor) { pwallet->WalletLogPrintf("Error getting wallet database cursor\n"); return DBErrors::CORRUPT; @@ -821,16 +822,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) while (true) { // Read next record - CDataStream ssKey(SER_DISK, CLIENT_VERSION); + DataStream ssKey{}; CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete); - if (complete) { + DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); + if (status == DatabaseCursor::Status::DONE) { break; - } - else if (!ret) - { - m_batch->CloseCursor(); + } else if (status == DatabaseCursor::Status::FAIL) { + cursor.reset(); pwallet->WalletLogPrintf("Error reading next record from wallet database\n"); return DBErrors::CORRUPT; } @@ -878,7 +876,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } catch (...) { result = DBErrors::CORRUPT; } - m_batch->CloseCursor(); // Set the active ScriptPubKeyMans for (auto spk_man_pair : wss.m_active_external_spks) { @@ -974,7 +971,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; @@ -986,7 +983,8 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWal } // Get cursor - if (!m_batch->StartCursor()) + std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor(); + if (!cursor) { LogPrintf("Error getting wallet database cursor\n"); return DBErrors::CORRUPT; @@ -995,14 +993,12 @@ DBErrors WalletBatch::FindWalletTx(std::vector<uint256>& vTxHash, std::list<CWal while (true) { // Read next record - CDataStream ssKey(SER_DISK, CLIENT_VERSION); - CDataStream ssValue(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret = m_batch->ReadAtCursor(ssKey, ssValue, complete); - if (complete) { + DataStream ssKey{}; + DataStream ssValue{}; + DatabaseCursor::Status status = cursor->Next(ssKey, ssValue); + if (status == DatabaseCursor::Status::DONE) { break; - } else if (!ret) { - m_batch->CloseCursor(); + } else if (status == DatabaseCursor::Status::FAIL) { LogPrintf("Error reading next record from wallet database\n"); return DBErrors::CORRUPT; } @@ -1012,25 +1008,21 @@ 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 (...) { result = DBErrors::CORRUPT; } - m_batch->CloseCursor(); return result; } 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; } @@ -1114,7 +1106,8 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags) bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types) { // Get cursor - if (!m_batch->StartCursor()) + std::unique_ptr<DatabaseCursor> cursor = m_batch->GetNewCursor(); + if (!cursor) { return false; } @@ -1123,16 +1116,12 @@ bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types) while (true) { // Read next record - CDataStream key(SER_DISK, CLIENT_VERSION); - CDataStream value(SER_DISK, CLIENT_VERSION); - bool complete; - bool ret = m_batch->ReadAtCursor(key, value, complete); - if (complete) { + DataStream key{}; + DataStream value{}; + DatabaseCursor::Status status = cursor->Next(key, value); + if (status == DatabaseCursor::Status::DONE) { break; - } - else if (!ret) - { - m_batch->CloseCursor(); + } else if (status == DatabaseCursor::Status::FAIL) { return false; } @@ -1146,7 +1135,6 @@ bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types) m_batch->Erase(key_data); } } - m_batch->CloseCursor(); return true; } diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index c0d9de8bb3..c97356a71f 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); @@ -303,7 +303,7 @@ void MaybeCompactWalletDB(WalletContext& context); using KeyFilterFn = std::function<bool(const std::string&)>; //! Unserialize a given Key-Value pair and load it into the wallet -bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr); +bool ReadKeyValue(CWallet* pwallet, DataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr); /** Return object for accessing dummy database with no read/write capabilities. */ std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase(); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index f93b666bd5..f389676204 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -47,7 +47,7 @@ static void WalletCreate(CWallet* wallet_instance, uint64_t wallet_creation_flag wallet_instance->TopUpKeyPool(); } -static const std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, const ArgsManager& args, DatabaseOptions options) +static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, DatabaseOptions options) { DatabaseStatus status; bilingual_str error; @@ -58,7 +58,7 @@ static const std::shared_ptr<CWallet> MakeWallet(const std::string& name, const } // dummy chain interface - std::shared_ptr<CWallet> wallet_instance{new CWallet(/*chain=*/nullptr, name, args, std::move(database)), WalletToolReleaseWallet}; + std::shared_ptr<CWallet> wallet_instance{new CWallet(/*chain=*/nullptr, name, std::move(database)), WalletToolReleaseWallet}; DBErrors load_wallet_ret; try { load_wallet_ret = wallet_instance->LoadWallet(); @@ -159,7 +159,7 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) options.require_format = DatabaseFormat::SQLITE; } - const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, args, options); + const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options); if (wallet_instance) { WalletShowInfo(wallet_instance.get()); wallet_instance->Close(); @@ -168,7 +168,7 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) DatabaseOptions options; ReadDatabaseArgs(args, options); options.require_existing = true; - const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, args, options); + const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options); if (!wallet_instance) return false; WalletShowInfo(wallet_instance.get()); wallet_instance->Close(); @@ -194,7 +194,7 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) DatabaseOptions options; ReadDatabaseArgs(args, options); options.require_existing = true; - const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, args, options); + const std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, options); if (!wallet_instance) return false; bilingual_str error; bool ret = DumpWallet(args, *wallet_instance, error); |