diff options
Diffstat (limited to 'src/wallet')
39 files changed, 2291 insertions, 620 deletions
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index f2b4de860a..3a9d277f65 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -432,7 +432,7 @@ void BerkeleyEnvironment::ReloadDbEnv() }); std::vector<fs::path> filenames; - for (auto it : m_databases) { + for (const auto& it : m_databases) { filenames.push_back(it.first); } // Close the individual Db's diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 49e6bac462..b568e90998 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -156,7 +156,7 @@ std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_poo for (const size_t& i : best_selection) { result.AddInput(utxo_pool.at(i)); } - result.ComputeAndSetWaste(CAmount{0}); + result.ComputeAndSetWaste(cost_of_change, cost_of_change, CAmount{0}); assert(best_waste == result.GetWaste()); return result; @@ -166,6 +166,12 @@ std::optional<SelectionResult> SelectCoinsSRD(const std::vector<OutputGroup>& ut { SelectionResult result(target_value, SelectionAlgorithm::SRD); + // Include change for SRD as we want to avoid making really small change if the selection just + // barely meets the target. Just use the lower bound change target instead of the randomly + // generated one, since SRD will result in a random change amount anyway; avoid making the + // target needlessly large. + target_value += CHANGE_LOWER; + std::vector<size_t> indexes; indexes.resize(utxo_pool.size()); std::iota(indexes.begin(), indexes.end(), 0); @@ -389,20 +395,26 @@ CAmount GetSelectionWaste(const std::set<COutput>& inputs, CAmount change_cost, return waste; } -CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng) +CAmount GenerateChangeTarget(const CAmount payment_value, const CAmount change_fee, FastRandomContext& rng) { if (payment_value <= CHANGE_LOWER / 2) { - return CHANGE_LOWER; + return change_fee + CHANGE_LOWER; } else { // random value between 50ksat and min (payment_value * 2, 1milsat) const auto upper_bound = std::min(payment_value * 2, CHANGE_UPPER); - return rng.randrange(upper_bound - CHANGE_LOWER) + CHANGE_LOWER; + return change_fee + rng.randrange(upper_bound - CHANGE_LOWER) + CHANGE_LOWER; } } -void SelectionResult::ComputeAndSetWaste(CAmount change_cost) +void SelectionResult::ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee) { - m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective); + const CAmount change = GetChange(min_viable_change, change_fee); + + if (change > 0) { + m_waste = GetSelectionWaste(m_selected_inputs, change_cost, m_target, m_use_effective); + } else { + m_waste = GetSelectionWaste(m_selected_inputs, 0, m_target, m_use_effective); + } } CAmount SelectionResult::GetWaste() const @@ -415,6 +427,11 @@ 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; }); } +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(); }); +} + void SelectionResult::Clear() { m_selected_inputs.clear(); @@ -427,6 +444,16 @@ void SelectionResult::AddInput(const OutputGroup& group) m_use_effective = !group.m_subtract_fee_outputs; } +void SelectionResult::Merge(const SelectionResult& other) +{ + 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); +} + const std::set<COutput>& SelectionResult::GetInputSet() const { return m_selected_inputs; @@ -464,4 +491,24 @@ std::string GetAlgorithmName(const SelectionAlgorithm algo) } assert(false); } + +CAmount SelectionResult::GetChange(const CAmount min_viable_change, const CAmount change_fee) const +{ + // change = SUM(inputs) - SUM(outputs) - fees + // 1) With SFFO we don't pay any fees + // 2) Otherwise we pay all the fees: + // - input fees are covered by GetSelectedEffectiveValue() + // - non_input_fee is included in m_target + // - change_fee + const CAmount change = m_use_effective + ? GetSelectedEffectiveValue() - m_target - change_fee + : GetSelectedValue() - m_target; + + if (change < min_viable_change) { + return 0; + } + + return change; +} + } // namespace wallet diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index 9135e48104..761c2be0b3 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -123,10 +123,12 @@ struct CoinSelectionParams { /** Mininmum change to target in Knapsack solver: select coins to cover the payment and * at least this value of change. */ CAmount m_min_change_target{0}; + /** Minimum amount for creating a change output. + * If change budget is smaller than min_change then we forgo creation of change output. + */ + CAmount min_viable_change{0}; /** Cost of creating the change output. */ CAmount m_change_fee{0}; - /** The pre-determined minimum value to target when funding a change output. */ - CAmount m_change_target{0}; /** Cost of creating the change output + cost of spending the change output in the future. */ CAmount m_cost_of_change{0}; /** The targeted feerate of the transaction being built. */ @@ -254,6 +256,7 @@ struct OutputGroup /** Choose a random change target for each transaction to make it harder to fingerprint the Core * wallet based on the change output values of transactions it creates. + * Change target covers at least change fees and adds a random value on top of it. * The random value is between 50ksat and min(2 * payment_value, 1milsat) * When payment_value <= 25ksat, the value is just 50ksat. * @@ -263,8 +266,9 @@ struct OutputGroup * coins selected are just sufficient to cover the payment amount ("unnecessary input" heuristic). * * @param[in] payment_value Average payment value of the transaction output(s). + * @param[in] change_fee Fee for creating a change output. */ -[[nodiscard]] CAmount GenerateChangeTarget(CAmount payment_value, FastRandomContext& rng); +[[nodiscard]] CAmount GenerateChangeTarget(const CAmount payment_value, const CAmount change_fee, FastRandomContext& rng); enum class SelectionAlgorithm : uint8_t { @@ -281,17 +285,16 @@ struct SelectionResult private: /** Set of inputs selected by the algorithm to use in the transaction */ std::set<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 */ + SelectionAlgorithm m_algo; /** Whether the input values for calculations should be the effective value (true) or normal value (false) */ bool m_use_effective{false}; /** The computed waste */ std::optional<CAmount> m_waste; public: - /** The target the algorithm selected for. Note that this may not be equal to the recipient amount as it can include non-input fees */ - const CAmount m_target; - /** The algorithm used to produce this result */ - const SelectionAlgorithm m_algo; - explicit SelectionResult(const CAmount target, SelectionAlgorithm algo) : m_target(target), m_algo(algo) {} @@ -300,20 +303,47 @@ public: /** Get the sum of the input values */ [[nodiscard]] CAmount GetSelectedValue() const; + [[nodiscard]] CAmount GetSelectedEffectiveValue() const; + void Clear(); void AddInput(const OutputGroup& group); /** Calculates and stores the waste for this selection via GetSelectionWaste */ - void ComputeAndSetWaste(CAmount change_cost); + void ComputeAndSetWaste(const CAmount min_viable_change, const CAmount change_cost, const CAmount change_fee); [[nodiscard]] CAmount GetWaste() const; + void Merge(const SelectionResult& other); + /** Get m_selected_inputs */ const std::set<COutput>& GetInputSet() const; /** Get the vector of COutputs that will be used to fill in a CTransaction's vin */ std::vector<COutput> GetShuffledInputVector() const; bool operator<(SelectionResult other) const; + + /** Get the amount for the change output after paying needed fees. + * + * The change amount is not 100% precise due to discrepancies in fee calculation. + * The final change amount (if any) should be corrected after calculating the final tx fees. + * When there is a discrepancy, most of the time the final change would be slightly bigger than estimated. + * + * Following are the possible factors of discrepancy: + * + non-input fees always include segwit flags + * + input fee estimation always include segwit stack size + * + input fees are rounded individually and not collectively, which leads to small rounding errors + * - input counter size is always assumed to be 1vbyte + * + * @param[in] min_viable_change Minimum amount for change output, if change would be less then we forgo change + * @param[in] change_fee Fees to include change output in the tx + * @returns Amount for change output, 0 when there is no change. + * + */ + CAmount GetChange(const CAmount min_viable_change, const CAmount change_fee) const; + + CAmount GetTarget() const { return m_target; } + + SelectionAlgorithm GetAlgo() const { return m_algo; } }; std::optional<SelectionResult> SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const CAmount& cost_of_change); diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index 9d5f58b784..76de51ac3e 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -45,7 +45,8 @@ ExternalSigner ExternalSignerScriptPubKeyMan::GetExternalSigner() { std::vector<ExternalSigner> signers; ExternalSigner::Enumerate(command, signers, Params().NetworkIDString()); if (signers.empty()) throw std::runtime_error(std::string(__func__) + ": No external signers found"); - // TODO: add fingerprint argument in case of multiple signers + // TODO: add fingerprint argument instead of failing in case of multiple signers. + if (signers.size() > 1) throw std::runtime_error(std::string(__func__) + ": More than one external signer found. Please connect only one at a time."); return signers[0]; } diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 43fdcee48d..6d7bb299cc 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.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 <consensus/validation.h> #include <interfaces/chain.h> #include <policy/fees.h> #include <policy/policy.h> @@ -19,9 +20,9 @@ namespace wallet { //! Check whether transaction has descendant in wallet or mempool, or has been //! mined, or conflicts with a mined transaction. Return a feebumper::Result. -static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, bool require_mine, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { - if (wallet.HasWalletSpend(wtx.GetHash())) { + if (wallet.HasWalletSpend(wtx.tx)) { errors.push_back(Untranslated("Transaction has descendants in the wallet")); return feebumper::Result::INVALID_PARAMETER; } @@ -48,20 +49,21 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet return feebumper::Result::WALLET_ERROR; } - // check that original tx consists entirely of our inputs - // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee) - isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - if (!AllInputsMine(wallet, *wtx.tx, filter)) { - errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet")); - return feebumper::Result::WALLET_ERROR; + if (require_mine) { + // check that original tx consists entirely of our inputs + // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee) + isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; + if (!AllInputsMine(wallet, *wtx.tx, filter)) { + errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet")); + return feebumper::Result::WALLET_ERROR; + } } - return feebumper::Result::OK; } //! Check if the user provided a valid feeRate -static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<bilingual_str>& errors) +static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, CAmount old_fee, std::vector<bilingual_str>& errors) { // check that fee rate is higher than mempool's minimum fee // (no point in bumping fee if we know that the new tx won't be accepted to the mempool) @@ -83,8 +85,6 @@ static feebumper::Result CheckFeeRate(const CWallet& wallet, const CWalletTx& wt CFeeRate incrementalRelayFee = std::max(wallet.chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE)); // Given old total fee and transaction size, calculate the old feeRate - isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - CAmount old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut(); const int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); CFeeRate nOldFeeRate(old_fee, txSize); // Min total fee is old fee + relay fee @@ -128,8 +128,8 @@ static CFeeRate EstimateFeeRate(const CWallet& wallet, const CWalletTx& wtx, con // WALLET_INCREMENTAL_RELAY_FEE value to future proof against changes to // network wide policy for incremental relay fee that our node may not be // aware of. This ensures we're over the required relay fee rate - // (BIP 125 rule 4). The replacement tx will be at least as large as the - // original tx, so the total fee will be greater (BIP 125 rule 3) + // (Rule 4). The replacement tx will be at least as large as the + // original tx, so the total fee will be greater (Rule 3) CFeeRate node_incremental_relay_fee = wallet.chain().relayIncrementalFee(); CFeeRate wallet_incremental_relay_fee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE); feerate += std::max(node_incremental_relay_fee, wallet_incremental_relay_fee); @@ -150,12 +150,12 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid) if (wtx == nullptr) return false; std::vector<bilingual_str> errors_dummy; - feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy); + feebumper::Result res = PreconditionChecks(wallet, *wtx, /* require_mine=*/ true, errors_dummy); return res == feebumper::Result::OK; } Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors, - CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) + CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine) { // We are going to modify coin control later, copy to re-use CCoinControl new_coin_control(coin_control); @@ -169,13 +169,63 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo } const CWalletTx& wtx = it->second; - Result result = PreconditionChecks(wallet, wtx, errors); + // Retrieve all of the UTXOs and add them to coin control + // While we're here, calculate the input amount + std::map<COutPoint, Coin> coins; + CAmount input_value = 0; + std::vector<CTxOut> spent_outputs; + for (const CTxIn& txin : wtx.tx->vin) { + coins[txin.prevout]; // Create empty map entry keyed by prevout. + } + wallet.chain().findCoins(coins); + for (const CTxIn& txin : wtx.tx->vin) { + const Coin& coin = coins.at(txin.prevout); + if (coin.out.IsNull()) { + errors.push_back(Untranslated(strprintf("%s:%u is already spent", txin.prevout.hash.GetHex(), txin.prevout.n))); + return Result::MISC_ERROR; + } + if (wallet.IsMine(txin.prevout)) { + new_coin_control.Select(txin.prevout); + } else { + new_coin_control.SelectExternal(txin.prevout, coin.out); + } + input_value += coin.out.nValue; + spent_outputs.push_back(coin.out); + } + + // Figure out if we need to compute the input weight, and do so if necessary + PrecomputedTransactionData txdata; + txdata.Init(*wtx.tx, std::move(spent_outputs), /* force=*/ true); + for (unsigned int i = 0; i < wtx.tx->vin.size(); ++i) { + const CTxIn& txin = wtx.tx->vin.at(i); + const Coin& coin = coins.at(txin.prevout); + + if (new_coin_control.IsExternalSelected(txin.prevout)) { + // For external inputs, we estimate the size using the size of this input + int64_t input_weight = GetTransactionInputWeight(txin); + // Because signatures can have different sizes, we need to figure out all of the + // signature sizes and replace them with the max sized signature. + // In order to do this, we verify the script with a special SignatureChecker which + // will observe the signatures verified and record their sizes. + SignatureWeights weights; + TransactionSignatureChecker tx_checker(wtx.tx.get(), i, coin.out.nValue, txdata, MissingDataBehavior::FAIL); + SignatureWeightChecker size_checker(weights, tx_checker); + VerifyScript(txin.scriptSig, coin.out.scriptPubKey, &txin.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, size_checker); + // Add the difference between max and current to input_weight so that it represents the largest the input could be + input_weight += weights.GetWeightDiffToMax(); + new_coin_control.SetInputWeight(txin.prevout, input_weight); + } + } + + Result result = PreconditionChecks(wallet, wtx, require_mine, errors); if (result != Result::OK) { 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; + CAmount output_value = 0; for (const auto& output : wtx.tx->vout) { if (!OutputIsChange(wallet, output)) { CRecipient recipient = {output.scriptPubKey, output.nValue, false}; @@ -185,16 +235,22 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo ExtractDestination(output.scriptPubKey, change_dest); new_coin_control.destChange = change_dest; } + output_value += output.nValue; } - isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE; - old_fee = CachedTxGetDebit(wallet, wtx, filter) - wtx.tx->GetValueOut(); + 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 - const int64_t maxTxSize{CalculateMaximumSignedTxSize(*wtx.tx, &wallet).vsize}; - Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, errors); + // We need to make a temporary transaction with no input witnesses as the dummy signer expects them to be empty for external inputs + CMutableTransaction mtx{*wtx.tx}; + for (auto& txin : mtx.vin) { + txin.scriptSig.clear(); + txin.scriptWitness.SetNull(); + } + const int64_t maxTxSize{CalculateMaximumSignedTxSize(CTransaction(mtx), &wallet, &new_coin_control).vsize}; + Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, old_fee, errors); if (res != Result::OK) { return res; } @@ -221,11 +277,11 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false); if (!res) { - errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + res.GetError()); + errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res)); return Result::WALLET_ERROR; } - const auto& txr = res.GetObj(); + const auto& txr = *res; // Write back new fee if successful new_fee = txr.fee; @@ -254,7 +310,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti const CWalletTx& oldWtx = it->second; // make sure the transaction still has no descendants and hasn't been mined in the meantime - Result result = PreconditionChecks(wallet, oldWtx, errors); + Result result = PreconditionChecks(wallet, oldWtx, /* require_mine=*/ false, errors); if (result != Result::OK) { return result; } diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index 191878a137..760ab58e5c 100644 --- a/src/wallet/feebumper.h +++ b/src/wallet/feebumper.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_WALLET_FEEBUMPER_H #define BITCOIN_WALLET_FEEBUMPER_H +#include <consensus/consensus.h> +#include <script/interpreter.h> #include <primitives/transaction.h> class uint256; @@ -31,14 +33,25 @@ enum class Result //! Return whether transaction can be bumped. bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid); -//! Create bumpfee transaction based on feerate estimates. +/** Create bumpfee transaction based on feerate estimates. + * + * @param[in] wallet The wallet to use for this bumping + * @param[in] txid The txid of the transaction to bump + * @param[in] coin_control A CCoinControl object which provides feerates and other information used for coin selection + * @param[out] errors Errors + * @param[out] old_fee The fee the original transaction pays + * @param[out] new_fee the fee that the bump transaction pays + * @param[out] mtx The bump transaction itself + * @param[in] require_mine Whether the original transaction must consist of inputs that can be spent by the wallet + */ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors, CAmount& old_fee, CAmount& new_fee, - CMutableTransaction& mtx); + CMutableTransaction& mtx, + bool require_mine); //! Sign the new transaction, //! @return false if the tx couldn't be found or if it was @@ -55,6 +68,55 @@ Result CommitTransaction(CWallet& wallet, std::vector<bilingual_str>& errors, uint256& bumped_txid); +struct SignatureWeights +{ +private: + int m_sigs_count{0}; + int64_t m_sigs_weight{0}; + +public: + void AddSigWeight(const size_t weight, const SigVersion sigversion) + { + switch (sigversion) { + case SigVersion::BASE: + m_sigs_weight += weight * WITNESS_SCALE_FACTOR; + m_sigs_count += 1 * WITNESS_SCALE_FACTOR; + break; + case SigVersion::WITNESS_V0: + m_sigs_weight += weight; + m_sigs_count++; + break; + case SigVersion::TAPROOT: + case SigVersion::TAPSCRIPT: + assert(false); + } + } + + int64_t GetWeightDiffToMax() const + { + // Note: the witness scaling factor is already accounted for because the count is multiplied by it. + return (/* max signature size=*/ 72 * m_sigs_count) - m_sigs_weight; + } +}; + +class SignatureWeightChecker : public DeferringSignatureChecker +{ +private: + SignatureWeights& m_weights; + +public: + SignatureWeightChecker(SignatureWeights& weights, const BaseSignatureChecker& checker) : DeferringSignatureChecker(checker), m_weights(weights) {} + + bool CheckECDSASignature(const std::vector<unsigned char>& sig, const std::vector<unsigned char>& pubkey, const CScript& script, SigVersion sigversion) const override + { + if (m_checker.CheckECDSASignature(sig, pubkey, script, sigversion)) { + m_weights.AddSigWeight(sig.size(), sigversion); + return true; + } + return false; + } +}; + } // namespace feebumper } // namespace wallet diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp index 6f81fa30a1..3514d018b7 100644 --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -87,7 +87,7 @@ CFeeRate GetDiscardRate(const CWallet& wallet) CFeeRate discard_rate = wallet.chain().estimateSmartFee(highest_target, false /* conservative */); // Don't let discard_rate be greater than longest possible fee estimate if we get a valid fee estimate discard_rate = (discard_rate == CFeeRate(0)) ? wallet.m_discard_rate : std::min(discard_rate, wallet.m_discard_rate); - // Discard rate must be at least dustRelayFee + // Discard rate must be at least dust relay feerate discard_rate = std::max(discard_rate, wallet.chain().relayDustFee()); return discard_rate; } diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 23f91d9b3a..98925e6b10 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -48,6 +48,8 @@ using interfaces::WalletTxStatus; using interfaces::WalletValueMap; namespace wallet { +// All members of the classes in this namespace are intentionally public, as the +// classes themselves are private. namespace { //! Construct wallet tx struct. WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) @@ -146,7 +148,7 @@ public: void abortRescan() override { m_wallet->AbortRescan(); } bool backupWallet(const std::string& filename) override { return m_wallet->BackupWallet(filename); } std::string getWalletName() override { return m_wallet->GetName(); } - BResult<CTxDestination> getNewDestination(const OutputType type, const std::string label) override + util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string& label) override { LOCK(m_wallet->cs_wallet); return m_wallet->GetNewDestination(type, label); @@ -249,17 +251,17 @@ public: LOCK(m_wallet->cs_wallet); return m_wallet->ListLockedCoins(outputs); } - BResult<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients, + util::Result<CTransactionRef> createTransaction(const std::vector<CRecipient>& recipients, const CCoinControl& coin_control, bool sign, int& change_pos, CAmount& fee) override { LOCK(m_wallet->cs_wallet); - const auto& res = CreateTransaction(*m_wallet, recipients, change_pos, + auto res = CreateTransaction(*m_wallet, recipients, change_pos, coin_control, sign); - if (!res) return res.GetError(); - const auto& txr = res.GetObj(); + if (!res) return util::Error{util::ErrorString(res)}; + const auto& txr = *res; fee = txr.fee; change_pos = txr.change_pos; @@ -289,7 +291,7 @@ public: CAmount& new_fee, CMutableTransaction& mtx) override { - return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) == feebumper::Result::OK; + return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true) == feebumper::Result::OK; } bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); } bool commitBumpTransaction(const uint256& txid, @@ -318,13 +320,12 @@ public: } return {}; } - std::vector<WalletTx> getWalletTxs() override + std::set<WalletTx> getWalletTxs() override { LOCK(m_wallet->cs_wallet); - std::vector<WalletTx> result; - result.reserve(m_wallet->mapWallet.size()); + std::set<WalletTx> result; for (const auto& entry : m_wallet->mapWallet) { - result.emplace_back(MakeWalletTx(*m_wallet, entry.second)); + result.emplace(MakeWalletTx(*m_wallet, entry.second)); } return result; } @@ -550,32 +551,34 @@ public: void setMockTime(int64_t time) override { return SetMockTime(time); } //! WalletLoader methods - std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) override + util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector<bilingual_str>& warnings) override { - std::shared_ptr<CWallet> wallet; DatabaseOptions options; DatabaseStatus status; ReadDatabaseArgs(*m_context.args, options); options.require_create = true; options.create_flags = wallet_creation_flags; options.create_passphrase = passphrase; - return MakeWallet(m_context, CreateWallet(m_context, name, true /* load_on_start */, options, status, error, warnings)); + bilingual_str error; + util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, CreateWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))}; + return wallet ? std::move(wallet) : util::Error{error}; } - std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override + util::Result<std::unique_ptr<Wallet>> loadWallet(const std::string& name, std::vector<bilingual_str>& warnings) override { DatabaseOptions options; DatabaseStatus status; ReadDatabaseArgs(*m_context.args, options); options.require_existing = true; - return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings)); + bilingual_str error; + util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, LoadWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))}; + return wallet ? std::move(wallet) : util::Error{error}; } - BResult<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override + util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override { DatabaseStatus status; bilingual_str error; - BResult<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))}; - if (!wallet) return error; - return wallet; + util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))}; + return wallet ? std::move(wallet) : util::Error{error}; } std::string getWalletDir() override { diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index c06513588b..6eb0ef5e7a 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -151,7 +151,7 @@ void StartWallets(WalletContext& context, CScheduler& scheduler) if (context.args->GetBoolArg("-flushwallet", DEFAULT_FLUSHWALLET)) { scheduler.scheduleEvery([&context] { MaybeCompactWalletDB(context); }, std::chrono::milliseconds{500}); } - scheduler.scheduleEvery([&context] { MaybeResendWalletTxs(context); }, std::chrono::milliseconds{1000}); + scheduler.scheduleEvery([&context] { MaybeResendWalletTxs(context); }, 1min); } void FlushWallets(WalletContext& context) diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index 944925d600..7fbf2ff8cf 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -193,7 +193,8 @@ CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx, std::list<COutputEntry>& listReceived, - std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter) + std::list<COutputEntry>& listSent, CAmount& nFee, const isminefilter& filter, + bool include_change) { nFee = 0; listReceived.clear(); @@ -218,8 +219,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx, // 2) the output is to us (received) if (nDebit > 0) { - // Don't report 'change' txouts - if (OutputIsChange(wallet, txout)) + if (!include_change && OutputIsChange(wallet, txout)) continue; } else if (!(fIsMine & filter)) @@ -416,7 +416,7 @@ std::set< std::set<CTxDestination> > GetAddressGroupings(const CWallet& wallet) std::set< std::set<CTxDestination>* > uniqueGroupings; // a set of pointers to groups of addresses std::map< CTxDestination, std::set<CTxDestination>* > setmap; // map addresses to the unique group containing it - for (std::set<CTxDestination> _grouping : groupings) + for (const std::set<CTxDestination>& _grouping : groupings) { // make a set of all the groups hit by this new group std::set< std::set<CTxDestination>* > hits; diff --git a/src/wallet/receive.h b/src/wallet/receive.h index 41a70b089a..9125b1e9aa 100644 --- a/src/wallet/receive.h +++ b/src/wallet/receive.h @@ -42,7 +42,8 @@ struct COutputEntry void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx, std::list<COutputEntry>& listReceived, std::list<COutputEntry>& listSent, - CAmount& nFee, const isminefilter& filter); + CAmount& nFee, const isminefilter& filter, + bool include_change); bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter); bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set<uint256>& trusted_parents) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet); bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx); diff --git a/src/wallet/rpc/addresses.cpp b/src/wallet/rpc/addresses.cpp index 2a6eb96871..903a569cb9 100644 --- a/src/wallet/rpc/addresses.cpp +++ b/src/wallet/rpc/addresses.cpp @@ -60,10 +60,10 @@ RPCHelpMan getnewaddress() auto op_dest = pwallet->GetNewDestination(output_type, label); if (!op_dest) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original); } - return EncodeDestination(op_dest.GetObj()); + return EncodeDestination(*op_dest); }, }; } @@ -107,9 +107,9 @@ RPCHelpMan getrawchangeaddress() auto op_dest = pwallet->GetNewChangeDestination(output_type); if (!op_dest) { - throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, op_dest.GetError().original); + throw JSONRPCError(RPC_WALLET_KEYPOOL_RAN_OUT, util::ErrorString(op_dest).original); } - return EncodeDestination(op_dest.GetObj()); + return EncodeDestination(*op_dest); }, }; } @@ -578,7 +578,7 @@ RPCHelpMan getaddressinfo() if (provider) { auto inferred = InferDescriptor(scriptPubKey, *provider); - bool solvable = inferred->IsSolvable() || IsSolvable(*provider, scriptPubKey); + bool solvable = inferred->IsSolvable(); ret.pushKV("solvable", solvable); if (solvable) { ret.pushKV("desc", inferred->ToString()); diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 306053fd0c..35df151e84 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -101,7 +101,7 @@ RPCHelpMan importprivkey() "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" - "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" + "but the key was used to create transactions, rescanblockchain needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"privkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The private key (see dumpprivkey)"}, @@ -204,7 +204,7 @@ RPCHelpMan importaddress() "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" - "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" + "but the key was used to create transactions, rescanblockchain needs to be called with the appropriate block range.\n" "If you have the full public key, you should call importpubkey instead of this.\n" "Hint: use importmulti to import more than one address.\n" "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" @@ -295,7 +295,7 @@ RPCHelpMan importaddress() RescanWallet(*pwallet, reserver); { LOCK(pwallet->cs_wallet); - pwallet->ReacceptWalletTransactions(); + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } } @@ -406,7 +406,7 @@ RPCHelpMan importpubkey() "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported pubkey exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" - "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" + "but the key was used to create transactions, rescanblockchain needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"pubkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The hex-encoded public key"}, @@ -476,7 +476,7 @@ RPCHelpMan importpubkey() RescanWallet(*pwallet, reserver); { LOCK(pwallet->cs_wallet); - pwallet->ReacceptWalletTransactions(); + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } } @@ -1257,7 +1257,7 @@ RPCHelpMan importmulti() "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported keys, addresses or scripts exist but related transactions are still missing.\n" "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" - "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" + "but the key was used to create transactions, rescanblockchain needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"requests", RPCArg::Type::ARR, RPCArg::Optional::NO, "Data to be imported", @@ -1397,7 +1397,7 @@ RPCHelpMan importmulti() int64_t scannedTime = pwallet->RescanFromTime(nLowestTimestamp, reserver, true /* update */); { LOCK(pwallet->cs_wallet); - pwallet->ReacceptWalletTransactions(); + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } if (pwallet->IsAbortingRescan()) { @@ -1691,7 +1691,7 @@ RPCHelpMan importdescriptors() int64_t scanned_time = pwallet->RescanFromTime(lowest_timestamp, reserver, true /* update */); { LOCK(pwallet->cs_wallet); - pwallet->ReacceptWalletTransactions(); + pwallet->ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); } if (pwallet->IsAbortingRescan()) { @@ -1784,34 +1784,59 @@ RPCHelpMan listdescriptors() LOCK(wallet->cs_wallet); - UniValue descriptors(UniValue::VARR); const auto active_spk_mans = wallet->GetActiveScriptPubKeyMans(); + + struct WalletDescInfo { + std::string descriptor; + uint64_t creation_time; + bool active; + std::optional<bool> internal; + std::optional<std::pair<int64_t,int64_t>> range; + int64_t next_index; + }; + + std::vector<WalletDescInfo> wallet_descriptors; for (const auto& spk_man : wallet->GetAllScriptPubKeyMans()) { const auto desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man); if (!desc_spk_man) { throw JSONRPCError(RPC_WALLET_ERROR, "Unexpected ScriptPubKey manager type."); } - UniValue spk(UniValue::VOBJ); LOCK(desc_spk_man->cs_desc_man); const auto& wallet_descriptor = desc_spk_man->GetWalletDescriptor(); std::string descriptor; - if (!desc_spk_man->GetDescriptorString(descriptor, priv)) { throw JSONRPCError(RPC_WALLET_ERROR, "Can't get descriptor string."); } - spk.pushKV("desc", descriptor); - spk.pushKV("timestamp", wallet_descriptor.creation_time); - spk.pushKV("active", active_spk_mans.count(desc_spk_man) != 0); - const auto internal = wallet->IsInternalScriptPubKeyMan(desc_spk_man); - if (internal.has_value()) { - spk.pushKV("internal", *internal); + const bool is_range = wallet_descriptor.descriptor->IsRange(); + wallet_descriptors.push_back({ + descriptor, + wallet_descriptor.creation_time, + active_spk_mans.count(desc_spk_man) != 0, + wallet->IsInternalScriptPubKeyMan(desc_spk_man), + is_range ? std::optional(std::make_pair(wallet_descriptor.range_start, wallet_descriptor.range_end)) : std::nullopt, + wallet_descriptor.next_index + }); + } + + std::sort(wallet_descriptors.begin(), wallet_descriptors.end(), [](const auto& a, const auto& b) { + return a.descriptor < b.descriptor; + }); + + UniValue descriptors(UniValue::VARR); + for (const WalletDescInfo& info : wallet_descriptors) { + UniValue spk(UniValue::VOBJ); + spk.pushKV("desc", info.descriptor); + spk.pushKV("timestamp", info.creation_time); + spk.pushKV("active", info.active); + if (info.internal.has_value()) { + spk.pushKV("internal", info.internal.value()); } - if (wallet_descriptor.descriptor->IsRange()) { + if (info.range.has_value()) { UniValue range(UniValue::VARR); - range.push_back(wallet_descriptor.range_start); - range.push_back(wallet_descriptor.range_end - 1); + range.push_back(info.range->first); + range.push_back(info.range->second - 1); spk.pushKV("range", range); - spk.pushKV("next", wallet_descriptor.next_index); + spk.pushKV("next", info.next_index); } descriptors.push_back(spk); } diff --git a/src/wallet/rpc/coins.cpp b/src/wallet/rpc/coins.cpp index d40d800f28..50766f20eb 100644 --- a/src/wallet/rpc/coins.cpp +++ b/src/wallet/rpc/coins.cpp @@ -543,6 +543,9 @@ RPCHelpMan listunspent() {RPCResult::Type::BOOL, "solvable", "Whether we know how to spend this output, ignoring the lack of keys"}, {RPCResult::Type::BOOL, "reused", /*optional=*/true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, {RPCResult::Type::STR, "desc", /*optional=*/true, "(only when solvable) A descriptor for spending this output"}, + {RPCResult::Type::ARR, "parent_descs", /*optional=*/false, "List of parent descriptors for the scriptPubKey of this coin.", { + {RPCResult::Type::STR, "desc", "The descriptor string."}, + }}, {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions\n" "from outside keys and unconfirmed replacement transactions are considered unsafe\n" "and are not eligible for spending by fundrawtransaction and sendtoaddress."}, @@ -638,7 +641,7 @@ RPCHelpMan listunspent() cctl.m_max_depth = nMaxDepth; cctl.m_include_unsafe_inputs = include_unsafe; LOCK(pwallet->cs_wallet); - vecOutputs = AvailableCoinsListUnspent(*pwallet, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount).all(); + vecOutputs = AvailableCoinsListUnspent(*pwallet, &cctl, nMinimumAmount, nMaximumAmount, nMinimumSumAmount, nMaximumCount).All(); } LOCK(pwallet->cs_wallet); @@ -722,6 +725,7 @@ RPCHelpMan listunspent() entry.pushKV("desc", descriptor->ToString()); } } + PushParentDescriptors(*pwallet, scriptPubKey, entry); if (avoid_reuse) entry.pushKV("reused", reused); entry.pushKV("safe", out.safe); results.push_back(entry); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index e27cb1139e..e38b13624c 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -158,14 +158,14 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, coin_control, true); if (!res) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, res.GetError().original); + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, util::ErrorString(res).original); } - const CTransactionRef& tx = res.GetObj().tx; + const CTransactionRef& tx = res->tx; wallet.CommitTransaction(tx, std::move(map_value), {} /* orderForm */); if (verbose) { UniValue entry(UniValue::VOBJ); entry.pushKV("txid", tx->GetHash().GetHex()); - entry.pushKV("fee_reason", StringForFeeReason(res.GetObj().fee_calc.reason)); + entry.pushKV("fee_reason", StringForFeeReason(res->fee_calc.reason)); return entry; } return tx->GetHash().GetHex(); @@ -224,10 +224,10 @@ RPCHelpMan sendtoaddress() "transaction, just kept in your wallet."}, {"subtractfeefromamount", RPCArg::Type::BOOL, RPCArg::Default{false}, "The fee will be deducted from the amount being sent.\n" "The recipient will receive less bitcoins than you enter in the amount field."}, - {"replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, + {"replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Signal that this transaction can be replaced by a transaction (BIP 125)"}, {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, - {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" - " \"" + FeeModes("\"\n\"") + "\""}, + {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" + "\"" + FeeModes("\"\n\"") + "\""}, {"avoid_reuse", RPCArg::Type::BOOL, RPCArg::Default{true}, "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n" "dirty if they have previously been used in a transaction. If true, this also activates avoidpartialspends, grouping outputs by their addresses."}, {"fee_rate", RPCArg::Type::AMOUNT, RPCArg::DefaultHint{"not set, fall back to wallet fee estimation"}, "Specify a fee rate in " + CURRENCY_ATOM + "/vB."}, @@ -333,10 +333,10 @@ RPCHelpMan sendmany() {"address", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Subtract fee from this address"}, }, }, - {"replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Allow this transaction to be replaced by a transaction with higher fees via BIP 125"}, + {"replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Signal that this transaction can be replaced by a transaction (BIP 125)"}, {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, - {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" - " \"" + FeeModes("\"\n\"") + "\""}, + {"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."}, {"verbose", RPCArg::Type::BOOL, RPCArg::Default{false}, "If true, return extra infomration about the transaction."}, }, @@ -451,8 +451,8 @@ static std::vector<RPCArg> FundTxDoc(bool solving_data = true) { std::vector<RPCArg> args = { {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, - {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" - " \"" + FeeModes("\"\n\"") + "\""}, + {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, "The fee estimate mode, must be one of (case insensitive):\n" + "\"" + FeeModes("\"\n\"") + "\""}, { "replaceable", RPCArg::Type::BOOL, RPCArg::DefaultHint{"wallet default"}, "Marks this transaction as BIP125-replaceable.\n" "Allows this transaction to be replaced by a transaction with higher fees" @@ -644,7 +644,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Unable to parse descriptor '%s': %s", desc_str, error)); } desc->Expand(0, desc_out, scripts_temp, desc_out); - coinControl.m_external_provider = Merge(coinControl.m_external_provider, desc_out); + coinControl.m_external_provider.Merge(std::move(desc_out)); } } } @@ -967,7 +967,7 @@ 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\"") + "\""}, }, "options"}, }, @@ -1045,7 +1045,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); + res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt); if (res != feebumper::Result::OK) { switch(res) { case feebumper::Result::INVALID_ADDRESS_OR_KEY: @@ -1131,13 +1131,13 @@ RPCHelpMan send() }, }, {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, - {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" - " \"" + FeeModes("\"\n\"") + "\""}, + {"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, "", Cat<std::vector<RPCArg>>( { - {"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{false}, "If inputs are specified, automatically include more if they are not enough."}, + {"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."}, @@ -1252,8 +1252,8 @@ RPCHelpMan sendall() }, }, {"conf_target", RPCArg::Type::NUM, RPCArg::DefaultHint{"wallet -txconfirmtarget"}, "Confirmation target in blocks"}, - {"estimate_mode", RPCArg::Type::STR, RPCArg::Default{"unset"}, std::string() + "The fee estimate mode, must be one of (case insensitive):\n" - " \"" + FeeModes("\"\n\"") + "\""}, + {"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, "", @@ -1264,11 +1264,15 @@ 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. A JSON array of JSON objects", + {"inputs", RPCArg::Type::ARR, RPCArg::Default{UniValue::VARR}, "Use exactly the specified inputs to build the transaction. Specifying inputs is incompatible with send_max.", { - {"txid", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The transaction id"}, - {"vout", RPCArg::Type::NUM, RPCArg::Optional::NO, "The output number"}, - {"sequence", RPCArg::Type::NUM, RPCArg::Optional::NO, "The sequence number"}, + {"", 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 number"}, + {"sequence", RPCArg::Type::NUM, RPCArg::DefaultHint{"depends on the value of the 'replaceable' and 'locktime' arguments"}, "The sequence number"}, + }, + }, }, }, {"locktime", RPCArg::Type::NUM, RPCArg::Default{0}, "Raw locktime. Non-0 value also locktime-activates inputs"}, @@ -1382,7 +1386,7 @@ RPCHelpMan sendall() total_input_value += tx->tx->vout[input.prevout.n].nValue; } } else { - for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, /*nMinimumAmount=*/0).all()) { + for (const COutput& output : AvailableCoins(*pwallet, &coin_control, fee_rate, /*nMinimumAmount=*/0).All()) { CHECK_NONFATAL(output.input_bytes > 0); if (send_max && fee_rate.GetFee(output.input_bytes) > output.txout.nValue) { continue; @@ -1398,6 +1402,10 @@ RPCHelpMan sendall() const CAmount fee_from_size{fee_rate.GetFee(tx_size.vsize)}; const CAmount effective_value{total_input_value - fee_from_size}; + if (fee_from_size > pwallet->m_default_max_tx_fee) { + throw JSONRPCError(RPC_WALLET_ERROR, TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED).original); + } + if (effective_value <= 0) { if (send_max) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Total value of UTXO pool too low to pay for transaction, try using lower feerate."); @@ -1406,8 +1414,13 @@ RPCHelpMan sendall() } } + // If this transaction is too large, e.g. because the wallet has many UTXOs, it will be rejected by the node's mempool. + if (tx_size.weight > MAX_STANDARD_TX_WEIGHT) { + throw JSONRPCError(RPC_WALLET_ERROR, "Transaction too large."); + } + CAmount output_amounts_claimed{0}; - for (CTxOut out : rawTx.vout) { + for (const CTxOut& out : rawTx.vout) { output_amounts_claimed += out.nValue; } @@ -1578,7 +1591,7 @@ RPCHelpMan walletcreatefundedpsbt() {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", Cat<std::vector<RPCArg>>( { - {"add_inputs", RPCArg::Type::BOOL, RPCArg::Default{false}, "If inputs are specified, automatically include more if they are not enough."}, + {"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."}, diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 9c8003d6d7..0e13e4756b 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -315,13 +315,16 @@ static void MaybePushAddress(UniValue & entry, const CTxDestination &dest) * @param filter_label Optional label string to filter incoming transactions. */ template <class Vec> -static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, Vec& ret, const isminefilter& filter_ismine, const std::string* filter_label) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nMinDepth, bool fLong, + Vec& ret, const isminefilter& filter_ismine, const std::string* filter_label, + bool include_change = false) + EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) { CAmount nFee; std::list<COutputEntry> listReceived; std::list<COutputEntry> listSent; - CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, filter_ismine); + CachedTxGetAmounts(wallet, wtx, listReceived, listSent, nFee, filter_ismine, include_change); bool involvesWatchonly = CachedTxIsFromMe(wallet, wtx, ISMINE_WATCH_ONLY); @@ -367,6 +370,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM entry.pushKV("involvesWatchonly", true); } MaybePushAddress(entry, r.destination); + PushParentDescriptors(wallet, wtx.tx->vout.at(r.vout).scriptPubKey, entry); if (wtx.IsCoinBase()) { if (wallet.GetTxDepthInMainChain(wtx) < 1) @@ -417,8 +421,12 @@ static const std::vector<RPCResult> TransactionDescriptionString() {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 + "."}, {RPCResult::Type::STR, "comment", /*optional=*/true, "If a comment is associated with the transaction, only present if not empty."}, - {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" - "may be unknown for unconfirmed transactions not in the mempool."}}; + {RPCResult::Type::STR, "bip125-replaceable", "(\"yes|no|unknown\") Whether this transaction signals BIP125 replaceability or has an unconfirmed ancestor signaling BIP125 replaceability.\n" + "May be unknown for unconfirmed transactions not in the mempool because their unconfirmed ancestors are unknown."}, + {RPCResult::Type::ARR, "parent_descs", /*optional=*/true, "Only if 'category' is 'received'. List of parent descriptors for the scriptPubKey of this coin.", { + {RPCResult::Type::STR, "desc", "The descriptor string."}, + }}, + }; } RPCHelpMan listtransactions() @@ -543,6 +551,7 @@ RPCHelpMan listsinceblock() {"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"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -623,6 +632,7 @@ 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()); int depth = height ? wallet.GetLastBlockHeight() + 1 - *height : -1; @@ -632,7 +642,7 @@ RPCHelpMan listsinceblock() const CWalletTx& tx = pairWtx.second; if (depth == -1 || abs(wallet.GetTxDepthInMainChain(tx)) < depth) { - ListTransactions(wallet, tx, 0, true, transactions, filter, nullptr /* filter_label */); + ListTransactions(wallet, tx, 0, true, transactions, filter, nullptr /* filter_label */, /*include_change=*/include_change); } } @@ -649,7 +659,7 @@ RPCHelpMan listsinceblock() if (it != wallet.mapWallet.end()) { // We want all transactions regardless of confirmation count to appear here, // even negative confirmation ones, hence the big negative. - ListTransactions(wallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */); + ListTransactions(wallet, it->second, -100000000, true, removed, filter, nullptr /* filter_label */, /*include_change=*/include_change); } } blockId = block.hashPrevBlock; @@ -709,6 +719,9 @@ RPCHelpMan gettransaction() "'send' category of transactions."}, {RPCResult::Type::BOOL, "abandoned", /*optional=*/true, "'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" "'send' category of transactions."}, + {RPCResult::Type::ARR, "parent_descs", /*optional=*/true, "Only if 'category' is 'received'. List of parent descriptors for the scriptPubKey of this coin.", { + {RPCResult::Type::STR, "desc", "The descriptor string."}, + }}, }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index 4fcb393226..3c1634324e 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -64,12 +64,11 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques return pwallet; } - std::vector<std::shared_ptr<CWallet>> wallets = GetWallets(context); - if (wallets.size() == 1) { - return wallets[0]; - } + size_t count{0}; + auto wallet = GetDefaultWallet(context, count); + if (wallet) return wallet; - if (wallets.empty()) { + if (count == 0) { throw JSONRPCError( RPC_WALLET_NOT_FOUND, "No wallet is loaded. Load a wallet using loadwallet or create a new one with createwallet. (Note: A default wallet is no longer automatically created)"); } @@ -123,6 +122,15 @@ std::string LabelFromValue(const UniValue& value) return label; } +void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry) +{ + UniValue parent_descs(UniValue::VARR); + for (const auto& desc: wallet.GetWalletDescriptors(script_pubkey)) { + parent_descs.push_back(desc.descriptor->ToString()); + } + entry.pushKV("parent_descs", parent_descs); +} + void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error) { if (!wallet) { diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h index 7b810eb06e..2ca28914e7 100644 --- a/src/wallet/rpc/util.h +++ b/src/wallet/rpc/util.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_WALLET_RPC_UTIL_H #define BITCOIN_WALLET_RPC_UTIL_H +#include <script/script.h> + #include <any> #include <memory> #include <string> @@ -39,6 +41,8 @@ const LegacyScriptPubKeyMan& EnsureConstLegacyScriptPubKeyMan(const CWallet& wal bool GetAvoidReuseFlag(const CWallet& wallet, const UniValue& param); bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wallet); std::string LabelFromValue(const UniValue& value); +//! Fetch parent descriptors of this scriptPubKey. +void PushParentDescriptors(const CWallet& wallet, const CScript& script_pubkey, UniValue& entry); void HandleWalletError(const std::shared_ptr<CWallet> wallet, DatabaseStatus& status, bilingual_str& error); } // namespace wallet diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 0fe8871152..675c4a759d 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -590,6 +590,170 @@ static RPCHelpMan upgradewallet() }; } +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", + { + {"rawtx", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, + }, + }, + {"options", RPCArg::Type::OBJ_USER_KEYS, RPCArg::Optional::OMITTED_NAMED_ARG, "Options", + { + {"include_watchonly", RPCArg::Type::BOOL, RPCArg::DefaultHint{"true for watch-only wallets, otherwise false"}, "Whether to include watch-only addresses (see RPC importaddress)"}, + }, + }, + }, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_AMOUNT, "balance_change", "The wallet balance change (negative means decrease)."}, + } + }, + RPCExamples{ + HelpExampleCli("simulaterawtransaction", "[\"myhex\"]") + + HelpExampleRpc("simulaterawtransaction", "[\"myhex\"]") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + const std::shared_ptr<const CWallet> rpc_wallet = GetWalletForJSONRPCRequest(request); + 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); + if (request.params[1].isObject()) { + UniValue options = request.params[1]; + RPCTypeCheckObj(options, + { + {"include_watchonly", UniValueType(UniValue::VBOOL)}, + }, + true, true); + + include_watchonly = options["include_watchonly"]; + } + + isminefilter filter = ISMINE_SPENDABLE; + if (ParseIncludeWatchonly(include_watchonly, wallet)) { + filter |= ISMINE_WATCH_ONLY; + } + + const auto& txs = request.params[0].get_array(); + CAmount changes{0}; + std::map<COutPoint, CAmount> new_utxos; // UTXO:s that were made available in transaction array + std::set<COutPoint> spent; + + for (size_t i = 0; i < txs.size(); ++i) { + CMutableTransaction mtx; + if (!DecodeHexTx(mtx, txs[i].get_str(), /* try_no_witness */ true, /* try_witness */ true)) { + throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Transaction hex string decoding failure."); + } + + // Fetch previous transactions (inputs) + std::map<COutPoint, Coin> coins; + for (const CTxIn& txin : mtx.vin) { + coins[txin.prevout]; // Create empty map entry keyed by prevout. + } + wallet.chain().findCoins(coins); + + // Fetch debit; we are *spending* these; if the transaction is signed and + // broadcast, we will lose everything in these + for (const auto& txin : mtx.vin) { + const auto& outpoint = txin.prevout; + if (spent.count(outpoint)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Transaction(s) are spending the same output more than once"); + } + if (new_utxos.count(outpoint)) { + changes -= new_utxos.at(outpoint); + new_utxos.erase(outpoint); + } else { + if (coins.at(outpoint).IsSpent()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "One or more transaction inputs are missing or have been spent already"); + } + changes -= wallet.GetDebit(txin, filter); + } + spent.insert(outpoint); + } + + // Iterate over outputs; we are *receiving* these, if the wallet considers + // them "mine"; if the transaction is signed and broadcast, we will receive + // everything in these + // Also populate new_utxos in case these are spent in later transactions + + const auto& hash = mtx.GetHash(); + for (size_t i = 0; i < mtx.vout.size(); ++i) { + const auto& txout = mtx.vout[i]; + bool is_mine = 0 < (wallet.IsMine(txout) & filter); + changes += new_utxos[COutPoint(hash, i)] = is_mine ? txout.nValue : 0; + } + } + + UniValue result(UniValue::VOBJ); + result.pushKV("balance_change", ValueFromAmount(changes)); + + return result; +} + }; +} + +static RPCHelpMan migratewallet() +{ + return RPCHelpMan{"migratewallet", + "EXPERIMENTAL warning: This call may not work as expected and may be changed in future releases\n" + "\nMigrate the wallet to a descriptor wallet.\n" + "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, + {}, + RPCResult{ + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR, "wallet_name", "The name of the primary migrated wallet"}, + {RPCResult::Type::STR, "watchonly_name", /*optional=*/true, "The name of the migrated wallet containing the watchonly scripts"}, + {RPCResult::Type::STR, "solvables_name", /*optional=*/true, "The name of the migrated wallet containing solvable but not watched scripts"}, + {RPCResult::Type::STR, "backup_path", "The location of the backup of the original wallet"}, + } + }, + RPCExamples{ + HelpExampleCli("migratewallet", "") + + HelpExampleRpc("migratewallet", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue + { + std::shared_ptr<CWallet> wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + + EnsureWalletIsUnlocked(*wallet); + + WalletContext& context = EnsureWalletContext(request.context); + + util::Result<MigrationResult> res = MigrateLegacyToDescriptor(std::move(wallet), context); + if (!res) { + throw JSONRPCError(RPC_WALLET_ERROR, util::ErrorString(res).original); + } + + UniValue r{UniValue::VOBJ}; + r.pushKV("wallet_name", res->wallet_name); + if (res->watchonly_wallet) { + r.pushKV("watchonly_name", res->watchonly_wallet->GetName()); + } + if (res->solvables_wallet) { + r.pushKV("solvables_name", res->solvables_wallet->GetName()); + } + r.pushKV("backup_path", res->backup_path.u8string()); + + return r; + }, + }; +} + // addresses RPCHelpMan getaddressinfo(); RPCHelpMan getnewaddress(); @@ -709,6 +873,7 @@ Span<const CRPCCommand> GetWalletRPCCommands() {"wallet", &listwallets}, {"wallet", &loadwallet}, {"wallet", &lockunspent}, + {"wallet", &migratewallet}, {"wallet", &newkeypool}, {"wallet", &removeprunedfunds}, {"wallet", &rescanblockchain}, @@ -721,6 +886,7 @@ Span<const CRPCCommand> GetWalletRPCCommands() {"wallet", &setwalletflag}, {"wallet", &signmessage}, {"wallet", &signrawtransactionwithwallet}, + {"wallet", &simulaterawtransaction}, {"wallet", &sendall}, {"wallet", &unloadwallet}, {"wallet", &upgradewallet}, diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index bba31dfe0b..41654579c6 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -21,10 +21,10 @@ namespace wallet { //! Value for the first BIP 32 hardened derivation. Can be used as a bit mask and as a value. See BIP 32 for more details. const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; -BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type) +util::Result<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputType type) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - return _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types");; + return util::Error{_("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types")}; } assert(type != OutputType::BECH32M); @@ -33,7 +33,7 @@ BResult<CTxDestination> LegacyScriptPubKeyMan::GetNewDestination(const OutputTyp // Generate a new key that is added to wallet CPubKey new_key; if (!GetKeyFromPool(new_key, type)) { - return _("Error: Keypool ran out, please call keypoolrefill first"); + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } LearnRelatedScripts(new_key, type); return GetDestinationForKey(new_key, type); @@ -292,26 +292,22 @@ bool LegacyScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, WalletBat return true; } -bool LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) +util::Result<CTxDestination> LegacyScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { if (LEGACY_OUTPUT_TYPES.count(type) == 0) { - error = _("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types"); - return false; + return util::Error{_("Error: Legacy wallets only support the \"legacy\", \"p2sh-segwit\", and \"bech32\" address types")}; } assert(type != OutputType::BECH32M); LOCK(cs_KeyStore); if (!CanGetAddresses(internal)) { - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } if (!ReserveKeyFromKeyPool(index, keypool, internal)) { - error = _("Error: Keypool ran out, please call keypoolrefill first"); - return false; + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } - address = GetDestinationForKey(keypool.vchPubKey, type); - return true; + return GetDestinationForKey(keypool.vchPubKey, type); } bool LegacyScriptPubKeyMan::TopUpInactiveHDChain(const CKeyID seed_id, int64_t index, bool internal) @@ -1003,9 +999,10 @@ bool LegacyScriptPubKeyMan::GetKeyOrigin(const CKeyID& keyID, KeyOriginInfo& inf { LOCK(cs_KeyStore); auto it = mapKeyMetadata.find(keyID); - if (it != mapKeyMetadata.end()) { - meta = it->second; + if (it == mapKeyMetadata.end()) { + return false; } + meta = it->second; } if (meta.has_key_origin) { std::copy(meta.key_origin.fingerprint, meta.key_origin.fingerprint + 4, info.fingerprint); @@ -1084,6 +1081,13 @@ CPubKey LegacyScriptPubKeyMan::GenerateNewKey(WalletBatch &batch, CHDChain& hd_c return pubkey; } +//! Try to derive an extended key, throw if it fails. +static void DeriveExtKey(CExtKey& key_in, unsigned int index, CExtKey& key_out) { + if (!key_in.Derive(key_out, index)) { + throw std::runtime_error("Could not derive extended key"); + } +} + void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& metadata, CKey& secret, CHDChain& hd_chain, bool internal) { // for now we use a fixed keypath scheme of m/0'/0'/k @@ -1101,11 +1105,11 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& // derive m/0' // use hardened derivation (child keys >= 0x80000000 are hardened after bip32) - masterKey.Derive(accountKey, BIP32_HARDENED_KEY_LIMIT); + DeriveExtKey(masterKey, BIP32_HARDENED_KEY_LIMIT, accountKey); // derive m/0'/0' (external chain) OR m/0'/1' (internal chain) assert(internal ? m_storage.CanSupportFeature(FEATURE_HD_SPLIT) : true); - accountKey.Derive(chainChildKey, BIP32_HARDENED_KEY_LIMIT+(internal ? 1 : 0)); + DeriveExtKey(accountKey, BIP32_HARDENED_KEY_LIMIT+(internal ? 1 : 0), chainChildKey); // derive child key at next index, skip keys already known to the wallet do { @@ -1113,7 +1117,7 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& // childIndex | BIP32_HARDENED_KEY_LIMIT = derive childIndex in hardened child-index-range // example: 1 | BIP32_HARDENED_KEY_LIMIT == 0x80000001 == 2147483649 if (internal) { - chainChildKey.Derive(childKey, hd_chain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT); + DeriveExtKey(chainChildKey, hd_chain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT, childKey); metadata.hdKeypath = "m/0'/1'/" + ToString(hd_chain.nInternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(1 | BIP32_HARDENED_KEY_LIMIT); @@ -1121,7 +1125,7 @@ void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata& hd_chain.nInternalChainCounter++; } else { - chainChildKey.Derive(childKey, hd_chain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT); + DeriveExtKey(chainChildKey, hd_chain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT, childKey); metadata.hdKeypath = "m/0'/0'/" + ToString(hd_chain.nExternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); @@ -1453,7 +1457,8 @@ void LegacyScriptPubKeyMan::LearnRelatedScripts(const CPubKey& key, OutputType t CTxDestination witdest = WitnessV0KeyHash(key.GetID()); CScript witprog = GetScriptForDestination(witdest); // Make sure the resulting program is solvable. - assert(IsSolvable(*this, witprog)); + const auto desc = InferDescriptor(witprog, *this); + assert(desc && desc->IsSolvable()); AddCScript(witprog); } } @@ -1654,11 +1659,323 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const return set_address; } -BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type) +const std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPubKeys() const +{ + LOCK(cs_KeyStore); + std::unordered_set<CScript, SaltedSipHasher> spks; + + // All keys have at least P2PK and P2PKH + for (const auto& key_pair : mapKeys) { + const CPubKey& pub = key_pair.second.GetPubKey(); + spks.insert(GetScriptForRawPubKey(pub)); + spks.insert(GetScriptForDestination(PKHash(pub))); + } + for (const auto& key_pair : mapCryptedKeys) { + const CPubKey& pub = key_pair.second.first; + spks.insert(GetScriptForRawPubKey(pub)); + spks.insert(GetScriptForDestination(PKHash(pub))); + } + + // For every script in mapScript, only the ISMINE_SPENDABLE ones are being tracked. + // The watchonly ones will be in setWatchOnly which we deal with later + // For all keys, if they have segwit scripts, those scripts will end up in mapScripts + for (const auto& script_pair : mapScripts) { + const CScript& script = script_pair.second; + if (IsMine(script) == ISMINE_SPENDABLE) { + // Add ScriptHash for scripts that are not already P2SH + if (!script.IsPayToScriptHash()) { + spks.insert(GetScriptForDestination(ScriptHash(script))); + } + // For segwit scripts, we only consider them spendable if we have the segwit spk + int wit_ver = -1; + std::vector<unsigned char> witprog; + if (script.IsWitnessProgram(wit_ver, witprog) && wit_ver == 0) { + spks.insert(script); + } + } else { + // Multisigs are special. They don't show up as ISMINE_SPENDABLE unless they are in a P2SH + // So check the P2SH of a multisig to see if we should insert it + std::vector<std::vector<unsigned char>> sols; + TxoutType type = Solver(script, sols); + if (type == TxoutType::MULTISIG) { + CScript ms_spk = GetScriptForDestination(ScriptHash(script)); + if (IsMine(ms_spk) != ISMINE_NO) { + spks.insert(ms_spk); + } + } + } + } + + // All watchonly scripts are raw + spks.insert(setWatchOnly.begin(), setWatchOnly.end()); + + return spks; +} + +std::optional<MigrationData> LegacyScriptPubKeyMan::MigrateToDescriptor() +{ + LOCK(cs_KeyStore); + if (m_storage.IsLocked()) { + return std::nullopt; + } + + MigrationData out; + + std::unordered_set<CScript, SaltedSipHasher> spks{GetScriptPubKeys()}; + + // Get all key ids + std::set<CKeyID> keyids; + for (const auto& key_pair : mapKeys) { + keyids.insert(key_pair.first); + } + for (const auto& key_pair : mapCryptedKeys) { + keyids.insert(key_pair.first); + } + + // Get key metadata and figure out which keys don't have a seed + // Note that we do not ignore the seeds themselves because they are considered IsMine! + for (auto keyid_it = keyids.begin(); keyid_it != keyids.end();) { + const CKeyID& keyid = *keyid_it; + const auto& it = mapKeyMetadata.find(keyid); + if (it != mapKeyMetadata.end()) { + const CKeyMetadata& meta = it->second; + if (meta.hdKeypath == "s" || meta.hdKeypath == "m") { + keyid_it++; + continue; + } + if (m_hd_chain.seed_id == meta.hd_seed_id || m_inactive_hd_chains.count(meta.hd_seed_id) > 0) { + keyid_it = keyids.erase(keyid_it); + continue; + } + } + keyid_it++; + } + + // keyids is now all non-HD keys. Each key will have its own combo descriptor + for (const CKeyID& keyid : keyids) { + CKey key; + if (!GetKey(keyid, key)) { + assert(false); + } + + // Get birthdate from key meta + uint64_t creation_time = 0; + const auto& it = mapKeyMetadata.find(keyid); + if (it != mapKeyMetadata.end()) { + creation_time = it->second.nCreateTime; + } + + // Get the key origin + // Maybe this doesn't matter because floating keys here shouldn't have origins + KeyOriginInfo info; + bool has_info = GetKeyOrigin(keyid, info); + std::string origin_str = has_info ? "[" + HexStr(info.fingerprint) + FormatHDKeypath(info.path) + "]" : ""; + + // Construct the combo descriptor + std::string desc_str = "combo(" + origin_str + HexStr(key.GetPubKey()) + ")"; + FlatSigningProvider keys; + std::string error; + std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, error, false); + 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)); + desc_spk_man->AddDescriptorKey(key, key.GetPubKey()); + desc_spk_man->TopUp(); + auto desc_spks = desc_spk_man->GetScriptPubKeys(); + + // Remove the scriptPubKeys from our current set + for (const CScript& spk : desc_spks) { + size_t erased = spks.erase(spk); + assert(erased == 1); + assert(IsMine(spk) == ISMINE_SPENDABLE); + } + + out.desc_spkms.push_back(std::move(desc_spk_man)); + } + + // Handle HD keys by using the CHDChains + std::vector<CHDChain> chains; + chains.push_back(m_hd_chain); + for (const auto& chain_pair : m_inactive_hd_chains) { + chains.push_back(chain_pair.second); + } + for (const CHDChain& chain : chains) { + for (int i = 0; i < 2; ++i) { + // Skip if doing internal chain and split chain is not supported + if (chain.seed_id.IsNull() || (i == 1 && !m_storage.CanSupportFeature(FEATURE_HD_SPLIT))) { + continue; + } + // Get the master xprv + CKey seed_key; + if (!GetKey(chain.seed_id, seed_key)) { + assert(false); + } + CExtKey master_key; + master_key.SetSeed(seed_key); + + // Make the combo descriptor + std::string xpub = EncodeExtPubKey(master_key.Neuter()); + std::string desc_str = "combo(" + xpub + "/0'/" + ToString(i) + "'/*')"; + FlatSigningProvider keys; + std::string error; + std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, error, false); + uint32_t chain_counter = std::max((i == 1 ? chain.nInternalChainCounter : chain.nExternalChainCounter), (uint32_t)0); + 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)); + desc_spk_man->AddDescriptorKey(master_key.key, master_key.key.GetPubKey()); + desc_spk_man->TopUp(); + auto desc_spks = desc_spk_man->GetScriptPubKeys(); + + // Remove the scriptPubKeys from our current set + for (const CScript& spk : desc_spks) { + size_t erased = spks.erase(spk); + assert(erased == 1); + assert(IsMine(spk) == ISMINE_SPENDABLE); + } + + out.desc_spkms.push_back(std::move(desc_spk_man)); + } + } + // Add the current master seed to the migration data + if (!m_hd_chain.seed_id.IsNull()) { + CKey seed_key; + if (!GetKey(m_hd_chain.seed_id, seed_key)) { + assert(false); + } + out.master_key.SetSeed(seed_key); + } + + // Handle the rest of the scriptPubKeys which must be imports and may not have all info + for (auto it = spks.begin(); it != spks.end();) { + const CScript& spk = *it; + + // Get birthdate from script meta + uint64_t creation_time = 0; + const auto& mit = m_script_metadata.find(CScriptID(spk)); + if (mit != m_script_metadata.end()) { + creation_time = mit->second.nCreateTime; + } + + // InferDescriptor as that will get us all the solving info if it is there + std::unique_ptr<Descriptor> desc = InferDescriptor(spk, *GetSolvingProvider(spk)); + // Get the private keys for this descriptor + std::vector<CScript> scripts; + FlatSigningProvider keys; + if (!desc->Expand(0, DUMMY_SIGNING_PROVIDER, scripts, keys)) { + assert(false); + } + std::set<CKeyID> privkeyids; + for (const auto& key_orig_pair : keys.origins) { + privkeyids.insert(key_orig_pair.first); + } + + std::vector<CScript> desc_spks; + + // Make the descriptor string with private keys + std::string desc_str; + bool watchonly = !desc->ToPrivateString(*this, desc_str); + if (watchonly && !m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + out.watch_descs.push_back({desc->ToString(), creation_time}); + + // Get the scriptPubKeys without writing this to the wallet + FlatSigningProvider provider; + desc->Expand(0, provider, desc_spks, provider); + } 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)); + for (const auto& keyid : privkeyids) { + CKey key; + if (!GetKey(keyid, key)) { + continue; + } + desc_spk_man->AddDescriptorKey(key, key.GetPubKey()); + } + desc_spk_man->TopUp(); + auto desc_spks_set = desc_spk_man->GetScriptPubKeys(); + desc_spks.insert(desc_spks.end(), desc_spks_set.begin(), desc_spks_set.end()); + + out.desc_spkms.push_back(std::move(desc_spk_man)); + } + + // Remove the scriptPubKeys from our current set + for (const CScript& desc_spk : desc_spks) { + auto del_it = spks.find(desc_spk); + assert(del_it != spks.end()); + assert(IsMine(desc_spk) != ISMINE_NO); + it = spks.erase(del_it); + } + } + + // Multisigs are special. They don't show up as ISMINE_SPENDABLE unless they are in a P2SH + // So we have to check if any of our scripts are a multisig and if so, add the P2SH + for (const auto& script_pair : mapScripts) { + const CScript script = script_pair.second; + + // Get birthdate from script meta + uint64_t creation_time = 0; + const auto& it = m_script_metadata.find(CScriptID(script)); + if (it != m_script_metadata.end()) { + creation_time = it->second.nCreateTime; + } + + std::vector<std::vector<unsigned char>> sols; + TxoutType type = Solver(script, sols); + if (type == TxoutType::MULTISIG) { + CScript sh_spk = GetScriptForDestination(ScriptHash(script)); + CTxDestination witdest = WitnessV0ScriptHash(script); + CScript witprog = GetScriptForDestination(witdest); + CScript sh_wsh_spk = GetScriptForDestination(ScriptHash(witprog)); + + // We only want the multisigs that we have not already seen, i.e. they are not watchonly and not spendable + // For P2SH, a multisig is not ISMINE_NO when: + // * All keys are in the wallet + // * The multisig itself is watch only + // * The P2SH is watch only + // For P2SH-P2WSH, if the script is in the wallet, then it will have the same conditions as P2SH. + // For P2WSH, a multisig is not ISMINE_NO when, other than the P2SH conditions: + // * The P2WSH script is in the wallet and it is being watched + std::vector<std::vector<unsigned char>> keys(sols.begin() + 1, sols.begin() + sols.size() - 1); + if (HaveWatchOnly(sh_spk) || HaveWatchOnly(script) || HaveKeys(keys, *this) || (HaveCScript(CScriptID(witprog)) && HaveWatchOnly(witprog))) { + // The above emulates IsMine for these 3 scriptPubKeys, so double check that by running IsMine + assert(IsMine(sh_spk) != ISMINE_NO || IsMine(witprog) != ISMINE_NO || IsMine(sh_wsh_spk) != ISMINE_NO); + continue; + } + assert(IsMine(sh_spk) == ISMINE_NO && IsMine(witprog) == ISMINE_NO && IsMine(sh_wsh_spk) == ISMINE_NO); + + std::unique_ptr<Descriptor> sh_desc = InferDescriptor(sh_spk, *GetSolvingProvider(sh_spk)); + out.solvable_descs.push_back({sh_desc->ToString(), creation_time}); + + const auto desc = InferDescriptor(witprog, *this); + if (desc->IsSolvable()) { + std::unique_ptr<Descriptor> wsh_desc = InferDescriptor(witprog, *GetSolvingProvider(witprog)); + out.solvable_descs.push_back({wsh_desc->ToString(), creation_time}); + std::unique_ptr<Descriptor> sh_wsh_desc = InferDescriptor(sh_wsh_spk, *GetSolvingProvider(sh_wsh_spk)); + out.solvable_descs.push_back({sh_wsh_desc->ToString(), creation_time}); + } + } + } + + // Make sure that we have accounted for all scriptPubKeys + assert(spks.size() == 0); + return out; +} + +bool LegacyScriptPubKeyMan::DeleteRecords() +{ + LOCK(cs_KeyStore); + WalletBatch batch(m_storage.GetDatabase()); + return batch.EraseRecords(DBKeys::LEGACY_TYPES); +} + +util::Result<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const OutputType type) { // Returns true if this descriptor supports getting new addresses. Conditions where we may be unable to fetch them (e.g. locked) are caught later if (!CanGetAddresses()) { - return _("No addresses available"); + return util::Error{_("No addresses available")}; } { LOCK(cs_desc_man); @@ -1676,11 +1993,11 @@ BResult<CTxDestination> DescriptorScriptPubKeyMan::GetNewDestination(const Outpu std::vector<CScript> scripts_temp; if (m_wallet_descriptor.range_end <= m_max_cached_index && !TopUp(1)) { // We can't generate anymore keys - return _("Error: Keypool ran out, please call keypoolrefill first"); + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } if (!m_wallet_descriptor.descriptor->ExpandFromCache(m_wallet_descriptor.next_index, m_wallet_descriptor.cache, scripts_temp, out_keys)) { // We can't generate anymore keys - return _("Error: Keypool ran out, please call keypoolrefill first"); + return util::Error{_("Error: Keypool ran out, please call keypoolrefill first")}; } CTxDestination dest; @@ -1760,17 +2077,12 @@ bool DescriptorScriptPubKeyMan::Encrypt(const CKeyingMaterial& master_key, Walle return true; } -bool DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) +util::Result<CTxDestination> DescriptorScriptPubKeyMan::GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { LOCK(cs_desc_man); auto op_dest = GetNewDestination(type); index = m_wallet_descriptor.next_index - 1; - if (op_dest) { - address = op_dest.GetObj(); - } else { - error = op_dest.GetError(); - } - return op_dest.HasRes(); + return op_dest; } void DescriptorScriptPubKeyMan::ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) @@ -1789,7 +2101,7 @@ std::map<CKeyID, CKey> DescriptorScriptPubKeyMan::GetKeys() const AssertLockHeld(cs_desc_man); if (m_storage.HasEncryptionKeys() && !m_storage.IsLocked()) { KeyMap keys; - for (auto key_pair : m_map_crypted_keys) { + for (const auto& key_pair : m_map_crypted_keys) { const CPubKey& pubkey = key_pair.second.first; const std::vector<unsigned char>& crypted_secret = key_pair.second.second; CKey key; @@ -1966,6 +2278,11 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ desc_prefix = "tr(" + xpub + "/86'"; break; } + case OutputType::UNKNOWN: { + // We should never have a DescriptorScriptPubKeyMan for an UNKNOWN OutputType, + // so if we get to this point something is wrong + assert(false); + } } // no default case, so the compiler can warn about missing cases assert(!desc_prefix.empty()); @@ -2075,10 +2392,21 @@ std::unique_ptr<FlatSigningProvider> DescriptorScriptPubKeyMan::GetSigningProvid std::unique_ptr<FlatSigningProvider> DescriptorScriptPubKeyMan::GetSigningProvider(int32_t index, bool include_private) const { AssertLockHeld(cs_desc_man); - // Get the scripts, keys, and key origins for this script + std::unique_ptr<FlatSigningProvider> out_keys = std::make_unique<FlatSigningProvider>(); - std::vector<CScript> scripts_temp; - if (!m_wallet_descriptor.descriptor->ExpandFromCache(index, m_wallet_descriptor.cache, scripts_temp, *out_keys)) return nullptr; + + // Fetch SigningProvider from cache to avoid re-deriving + auto it = m_map_signing_providers.find(index); + if (it != m_map_signing_providers.end()) { + out_keys->Merge(FlatSigningProvider{it->second}); + } else { + // Get the scripts, keys, and key origins for this script + std::vector<CScript> scripts_temp; + if (!m_wallet_descriptor.descriptor->ExpandFromCache(index, m_wallet_descriptor.cache, scripts_temp, *out_keys)) return nullptr; + + // Cache SigningProvider so we don't need to re-derive if we need this SigningProvider again + m_map_signing_providers[index] = *out_keys; + } if (HavePrivateKeys() && include_private) { FlatSigningProvider master_provider; @@ -2107,7 +2435,7 @@ bool DescriptorScriptPubKeyMan::SignTransaction(CMutableTransaction& tx, const s if (!coin_keys) { continue; } - *keys = Merge(*keys, *coin_keys); + keys->Merge(std::move(*coin_keys)); } return ::SignTransaction(tx, keys.get(), coins, sighash, input_errors); @@ -2168,7 +2496,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& std::unique_ptr<FlatSigningProvider> keys = std::make_unique<FlatSigningProvider>(); std::unique_ptr<FlatSigningProvider> script_keys = GetSigningProvider(script, sign); if (script_keys) { - *keys = Merge(*keys, *script_keys); + keys->Merge(std::move(*script_keys)); } else { // Maybe there are pubkeys listed that we can sign for script_keys = std::make_unique<FlatSigningProvider>(); @@ -2176,7 +2504,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& const CPubKey& pubkey = pk_pair.first; std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(pubkey); if (pk_keys) { - *keys = Merge(*keys, *pk_keys); + keys->Merge(std::move(*pk_keys)); } } for (const auto& pk_pair : input.m_tap_bip32_paths) { @@ -2188,7 +2516,7 @@ TransactionError DescriptorScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& fullpubkey.Set(b, b + 33); std::unique_ptr<FlatSigningProvider> pk_keys = GetSigningProvider(fullpubkey); if (pk_keys) { - *keys = Merge(*keys, *pk_keys); + keys->Merge(std::move(*pk_keys)); } } } @@ -2312,14 +2640,14 @@ const WalletDescriptor DescriptorScriptPubKeyMan::GetWalletDescriptor() const return m_wallet_descriptor; } -const std::vector<CScript> DescriptorScriptPubKeyMan::GetScriptPubKeys() const +const std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys() const { LOCK(cs_desc_man); - std::vector<CScript> script_pub_keys; + std::unordered_set<CScript, SaltedSipHasher> script_pub_keys; script_pub_keys.reserve(m_map_script_pub_keys.size()); for (auto const& script_pub_key: m_map_script_pub_keys) { - script_pub_keys.push_back(script_pub_key.first); + script_pub_keys.insert(script_pub_key.first); } return script_pub_keys; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index eebc05330f..3ab489c374 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -172,14 +172,14 @@ protected: public: explicit ScriptPubKeyMan(WalletStorage& storage) : m_storage(storage) {} virtual ~ScriptPubKeyMan() {}; - virtual BResult<CTxDestination> GetNewDestination(const OutputType type) { return Untranslated("Not supported"); } + virtual util::Result<CTxDestination> GetNewDestination(const OutputType type) { return util::Error{Untranslated("Not supported")}; } virtual isminetype IsMine(const CScript& script) const { return ISMINE_NO; } //! Check that the given decryption key is valid for this ScriptPubKeyMan, i.e. it decrypts all of the keys handled by it. virtual bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) { return false; } virtual bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) { return false; } - virtual bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) { return false; } + virtual util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) { return util::Error{Untranslated("Not supported")}; } virtual void KeepDestination(int64_t index, const OutputType& type) {} virtual void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) {} @@ -242,6 +242,9 @@ 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 {}; }; + /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ template<typename... Params> void WalletLogPrintf(std::string fmt, Params... parameters) const { @@ -262,6 +265,8 @@ static const std::unordered_set<OutputType> LEGACY_OUTPUT_TYPES { OutputType::BECH32, }; +class DescriptorScriptPubKeyMan; + class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProvider { private: @@ -360,13 +365,13 @@ private: public: using ScriptPubKeyMan::ScriptPubKeyMan; - BResult<CTxDestination> GetNewDestination(const OutputType type) override; + util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) override; + util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; void KeepDestination(int64_t index, const OutputType& type) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination&) override; @@ -507,6 +512,13 @@ 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; + + /** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan. + * Does not modify this ScriptPubKeyMan. */ + std::optional<MigrationData> MigrateToDescriptor(); + /** Delete all the records ofthis LegacyScriptPubKeyMan from disk*/ + bool DeleteRecords(); }; /** Wraps a LegacyScriptPubKeyMan so that it can be returned in a new unique_ptr. Does not provide privkeys */ @@ -547,6 +559,8 @@ private: KeyMap GetKeys() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); + // Cached FlatSigningProviders to avoid regenerating them each time they are needed. + mutable std::map<int32_t, FlatSigningProvider> m_map_signing_providers; // Fetch the SigningProvider for the given script and optionally include private keys std::unique_ptr<FlatSigningProvider> GetSigningProvider(const CScript& script, bool include_private = false) const; // Fetch the SigningProvider for the given pubkey and always include private keys. This should only be called by signing code. @@ -568,13 +582,13 @@ public: mutable RecursiveMutex cs_desc_man; - BResult<CTxDestination> GetNewDestination(const OutputType type) override; + util::Result<CTxDestination> GetNewDestination(const OutputType type) override; isminetype IsMine(const CScript& script) const override; bool CheckDecryptionKey(const CKeyingMaterial& master_key, bool accept_no_keys = false) override; bool Encrypt(const CKeyingMaterial& master_key, WalletBatch* batch) override; - bool GetReservedDestination(const OutputType type, bool internal, CTxDestination& address, int64_t& index, CKeyPool& keypool, bilingual_str& error) override; + util::Result<CTxDestination> GetReservedDestination(const OutputType type, bool internal, int64_t& index, CKeyPool& keypool) override; void ReturnDestination(int64_t index, bool internal, const CTxDestination& addr) override; // Tops up the descriptor cache and m_map_script_pub_keys. The cache is stored in the wallet file @@ -628,7 +642,7 @@ public: void WriteDescriptor(); const WalletDescriptor GetWalletDescriptor() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); - const std::vector<CScript> GetScriptPubKeys() const; + const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; bool GetDescriptorString(std::string& out, const bool priv) const; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index c00a2cd023..d84310c323 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -79,30 +79,68 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *walle return CalculateMaximumSignedTxSize(tx, wallet, txouts, coin_control); } -uint64_t CoinsResult::size() const +size_t CoinsResult::Size() const { - return bech32m.size() + bech32.size() + P2SH_segwit.size() + legacy.size() + other.size(); + size_t size{0}; + for (const auto& it : coins) { + size += it.second.size(); + } + return size; } -std::vector<COutput> CoinsResult::all() const +std::vector<COutput> CoinsResult::All() const { std::vector<COutput> all; - all.reserve(this->size()); - all.insert(all.end(), bech32m.begin(), bech32m.end()); - all.insert(all.end(), bech32.begin(), bech32.end()); - all.insert(all.end(), P2SH_segwit.begin(), P2SH_segwit.end()); - all.insert(all.end(), legacy.begin(), legacy.end()); - all.insert(all.end(), other.begin(), other.end()); + all.reserve(coins.size()); + for (const auto& it : coins) { + all.insert(all.end(), it.second.begin(), it.second.end()); + } return all; } -void CoinsResult::clear() +void CoinsResult::Clear() { + coins.clear(); +} + +void CoinsResult::Erase(std::set<COutPoint>& preset_coins) { - bech32m.clear(); - bech32.clear(); - P2SH_segwit.clear(); - legacy.clear(); - other.clear(); + for (auto& it : coins) { + auto& vec = it.second; + auto i = std::find_if(vec.begin(), vec.end(), [&](const COutput &c) { return preset_coins.count(c.outpoint);}); + if (i != vec.end()) { + vec.erase(i); + break; + } + } +} + +void CoinsResult::Shuffle(FastRandomContext& rng_fast) +{ + for (auto& it : coins) { + ::Shuffle(it.second.begin(), it.second.end(), rng_fast); + } +} + +void CoinsResult::Add(OutputType type, const COutput& out) +{ + coins[type].emplace_back(out); +} + +static OutputType GetOutputType(TxoutType type, bool is_from_p2sh) +{ + switch (type) { + case TxoutType::WITNESS_V1_TAPROOT: + return OutputType::BECH32M; + case TxoutType::WITNESS_V0_KEYHASH: + case TxoutType::WITNESS_V0_SCRIPTHASH: + if (is_from_p2sh) return OutputType::P2SH_SEGWIT; + else return OutputType::BECH32; + case TxoutType::SCRIPTHASH: + case TxoutType::PUBKEYHASH: + return OutputType::LEGACY; + default: + return OutputType::UNKNOWN; + } } CoinsResult AvailableCoins(const CWallet& wallet, @@ -213,58 +251,41 @@ CoinsResult AvailableCoins(const CWallet& wallet, std::unique_ptr<SigningProvider> provider = wallet.GetSolvingProvider(output.scriptPubKey); - bool solvable = provider ? IsSolvable(*provider, output.scriptPubKey) : false; + 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; bool spendable = ((mine & ISMINE_SPENDABLE) != ISMINE_NO) || (((mine & ISMINE_WATCH_ONLY) != ISMINE_NO) && (coinControl && coinControl->fAllowWatchOnly && solvable)); // Filter by spendable outputs only if (!spendable && only_spendable) continue; - // When parsing a scriptPubKey, Solver returns the parsed pubkeys or hashes (depending on the script) - // We don't need those here, so we are leaving them in return_values_unused - std::vector<std::vector<uint8_t>> return_values_unused; - TxoutType type; - bool is_from_p2sh{false}; - // If the Output is P2SH and spendable, we want to know if it is // a P2SH (legacy) or one of P2SH-P2WPKH, P2SH-P2WSH (P2SH-Segwit). We can determine // this from the redeemScript. If the Output is not spendable, it will be classified // as a P2SH (legacy), since we have no way of knowing otherwise without the redeemScript + CScript script; + bool is_from_p2sh{false}; if (output.scriptPubKey.IsPayToScriptHash() && solvable) { - CScript redeemScript; CTxDestination destination; if (!ExtractDestination(output.scriptPubKey, destination)) continue; const CScriptID& hash = CScriptID(std::get<ScriptHash>(destination)); - if (!provider->GetCScript(hash, redeemScript)) + if (!provider->GetCScript(hash, script)) continue; - type = Solver(redeemScript, return_values_unused); is_from_p2sh = true; } else { - type = Solver(output.scriptPubKey, return_values_unused); + script = output.scriptPubKey; } - int input_bytes = CalculateMaximumSignedInputSize(output, COutPoint(), provider.get(), coinControl); COutput coin(outpoint, output, nDepth, input_bytes, spendable, solvable, safeTx, wtx.GetTxTime(), tx_from_me, feerate); - switch (type) { - case TxoutType::WITNESS_UNKNOWN: - case TxoutType::WITNESS_V1_TAPROOT: - result.bech32m.push_back(coin); - break; - case TxoutType::WITNESS_V0_KEYHASH: - case TxoutType::WITNESS_V0_SCRIPTHASH: - if (is_from_p2sh) { - result.P2SH_segwit.push_back(coin); - break; - } - result.bech32.push_back(coin); - break; - case TxoutType::SCRIPTHASH: - case TxoutType::PUBKEYHASH: - result.legacy.push_back(coin); - break; - default: - result.other.push_back(coin); - }; + + // When parsing a scriptPubKey, Solver returns the parsed pubkeys or hashes (depending on the script) + // We don't need those here, so we are leaving them in return_values_unused + std::vector<std::vector<uint8_t>> return_values_unused; + TxoutType type; + type = Solver(script, return_values_unused); + result.Add(GetOutputType(type, is_from_p2sh), coin); // Cache total amount as we go result.total_amount += output.nValue; @@ -276,7 +297,7 @@ CoinsResult AvailableCoins(const CWallet& wallet, } // Checks the maximum number of UTXO's. - if (nMaximumCount > 0 && result.size() >= nMaximumCount) { + if (nMaximumCount > 0 && result.Size() >= nMaximumCount) { return result; } } @@ -332,7 +353,7 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet) std::map<CTxDestination, std::vector<COutput>> result; - for (const COutput& coin : AvailableCoinsListUnspent(wallet).all()) { + for (COutput& coin : AvailableCoinsListUnspent(wallet).All()) { CTxDestination address; if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) && ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) { @@ -455,31 +476,25 @@ std::optional<SelectionResult> AttemptSelection(const CWallet& wallet, const CAm { // Run coin selection on each OutputType and compute the Waste Metric std::vector<SelectionResult> results; - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.legacy, coin_selection_params)}) { - results.push_back(*result); - } - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.P2SH_segwit, coin_selection_params)}) { - results.push_back(*result); - } - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.bech32, coin_selection_params)}) { - results.push_back(*result); - } - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.bech32m, coin_selection_params)}) { - results.push_back(*result); + for (const auto& it : available_coins.coins) { + if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, it.second, coin_selection_params)}) { + results.push_back(*result); + } } - - // If we can't fund the transaction from any individual OutputType, run coin selection - // over all available coins, else pick the best solution from the results - if (results.size() == 0) { - if (allow_mixed_output_types) { - if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.all(), coin_selection_params)}) { - return 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 + if (results.size() > 0) return *std::min_element(results.begin(), results.end()); + + // 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 (allow_mixed_output_types) { + if (auto result{ChooseSelectionResult(wallet, nTargetValue, eligibility_filter, available_coins.All(), coin_selection_params)}) { + return result; } - return std::optional<SelectionResult>(); - }; - std::optional<SelectionResult> result{*std::min_element(results.begin(), results.end())}; - return result; + } + // 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; }; 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) @@ -487,32 +502,20 @@ std::optional<SelectionResult> ChooseSelectionResult(const CWallet& wallet, cons // Vector of results. We will choose the best one based on waste. std::vector<SelectionResult> results; - // Note that unlike KnapsackSolver, we do not include the fee for creating a change output as BnB will not create a change output. - std::vector<OutputGroup> positive_groups = GroupOutputs(wallet, available_coins, coin_selection_params, eligibility_filter, true /* positive_only */); + 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)}) { 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, false /* positive_only */); - CAmount target_with_change = nTargetValue; - // While nTargetValue includes the transaction fees for non-input things, it does not include the fee for creating a change output. - // So we need to include that for KnapsackSolver and SRD as well, as we are expecting to create a change output. - if (!coin_selection_params.m_subtract_fee_outputs) { - target_with_change += coin_selection_params.m_change_fee; - } - if (auto knapsack_result{KnapsackSolver(all_groups, target_with_change, coin_selection_params.m_min_change_target, coin_selection_params.rng_fast)}) { - knapsack_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); + 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)}) { + 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); } - // Include change for SRD as we want to avoid making really small change if the selection just - // barely meets the target. Just use the lower bound change target instead of the randomly - // generated one, since SRD will result in a random change amount anyway; avoid making the - // target needlessly large. - const CAmount srd_target = target_with_change + CHANGE_LOWER; - if (auto srd_result{SelectCoinsSRD(positive_groups, srd_target, coin_selection_params.rng_fast)}) { - srd_result->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); + if (auto srd_result{SelectCoinsSRD(positive_groups, 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); } @@ -554,8 +557,12 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a if (!coin_control.GetExternalOutput(outpoint, txout)) { return std::nullopt; } + } + + if (input_bytes == -1) { input_bytes = CalculateMaximumSignedInputSize(txout, outpoint, &coin_control.m_external_provider, &coin_control); } + // If available, override calculated size with coin control specified size if (coin_control.HasInputWeight(outpoint)) { input_bytes = GetVirtualTransactionSize(coin_control.GetInputWeight(outpoint), 0, 0); @@ -584,17 +591,13 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a SelectionResult result(nTargetValue, SelectionAlgorithm::MANUAL); result.AddInput(preset_inputs); if (result.GetSelectedValue() < nTargetValue) return std::nullopt; - result.ComputeAndSetWaste(coin_selection_params.m_cost_of_change); + result.ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); return result; } // remove preset inputs from coins so that Coin Selection doesn't pick them. if (coin_control.HasSelected()) { - available_coins.legacy.erase(remove_if(available_coins.legacy.begin(), available_coins.legacy.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.legacy.end()); - available_coins.P2SH_segwit.erase(remove_if(available_coins.P2SH_segwit.begin(), available_coins.P2SH_segwit.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.P2SH_segwit.end()); - available_coins.bech32.erase(remove_if(available_coins.bech32.begin(), available_coins.bech32.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.bech32.end()); - available_coins.bech32m.erase(remove_if(available_coins.bech32m.begin(), available_coins.bech32m.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.bech32m.end()); - available_coins.other.erase(remove_if(available_coins.other.begin(), available_coins.other.end(), [&](const COutput& c) { return preset_coins.count(c.outpoint); }), available_coins.other.end()); + available_coins.Erase(preset_coins); } unsigned int limit_ancestor_count = 0; @@ -606,23 +609,22 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a // 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) { + 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 - Shuffle(available_coins.legacy.begin(), available_coins.legacy.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.P2SH_segwit.begin(), available_coins.P2SH_segwit.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.bech32.begin(), available_coins.bech32.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.bech32m.begin(), available_coins.bech32m.end(), coin_selection_params.rng_fast); - Shuffle(available_coins.other.begin(), available_coins.other.end(), coin_selection_params.rng_fast); + available_coins.Shuffle(coin_selection_params.rng_fast); } + SelectionResult preselected(preset_inputs.GetSelectionAmount(), SelectionAlgorithm::MANUAL); + preselected.AddInput(preset_inputs); + // 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 = [&] { // Pre-selected inputs already cover the target amount. - if (value_to_select <= 0) return std::make_optional(SelectionResult(nTargetValue, SelectionAlgorithm::MANUAL)); + if (value_to_select <= 0) return std::make_optional(SelectionResult(value_to_select, SelectionAlgorithm::MANUAL)); // 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. @@ -676,9 +678,9 @@ std::optional<SelectionResult> SelectCoins(const CWallet& wallet, CoinsResult& a if (!res) return std::nullopt; // Add preset inputs to result - res->AddInput(preset_inputs); - if (res->m_algo == SelectionAlgorithm::MANUAL) { - res->ComputeAndSetWaste(coin_selection_params.m_cost_of_change); + res->Merge(preselected); + if (res->GetAlgo() == SelectionAlgorithm::MANUAL) { + res->ComputeAndSetWaste(coin_selection_params.min_viable_change, coin_selection_params.m_cost_of_change, coin_selection_params.m_change_fee); } return res; @@ -758,7 +760,7 @@ static void DiscourageFeeSniping(CMutableTransaction& tx, FastRandomContext& rng } } -static BResult<CreatedTransactionResult> CreateTransactionInternal( +static util::Result<CreatedTransactionResult> CreateTransactionInternal( CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, @@ -792,7 +794,6 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( coin_selection_params.m_subtract_fee_outputs = true; } } - coin_selection_params.m_change_target = GenerateChangeTarget(std::floor(recipients_sum / vecSend.size()), rng_fast); // Create change script that will be used if we need change CScript scriptChange; @@ -812,11 +813,13 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Reserve a new key pair from key pool. If it fails, provide a dummy // destination in case we don't need change. CTxDestination dest; - bilingual_str dest_err; - if (!reservedest.GetReservedDestination(dest, true, dest_err)) { - error = _("Transaction needs a change address, but we can't generate it.") + Untranslated(" ") + dest_err; + auto op_dest = reservedest.GetReservedDestination(true); + if (!op_dest) { + error = _("Transaction needs a change address, but we can't generate it.") + Untranslated(" ") + util::ErrorString(op_dest); + } else { + dest = *op_dest; + scriptChange = GetScriptForDestination(dest); } - scriptChange = GetScriptForDestination(dest); // A valid destination implies a change script (and // vice-versa). An empty change script will abort later, if the // change keypool ran out, but change is required. @@ -844,11 +847,11 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Do not, ever, assume that it's fine to change the fee rate if the user has explicitly // provided one if (coin_control.m_feerate && coin_selection_params.m_effective_feerate > *coin_control.m_feerate) { - return strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB)); + return util::Error{strprintf(_("Fee rate (%s) is lower than the minimum fee rate setting (%s)"), coin_control.m_feerate->ToString(FeeEstimateMode::SAT_VB), coin_selection_params.m_effective_feerate.ToString(FeeEstimateMode::SAT_VB))}; } if (feeCalc.reason == FeeReason::FALLBACK && !wallet.m_allow_fallback_fee) { // eventually allow a fallback fee - return _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee."); + return util::Error{_("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.")}; } // Calculate the cost of change @@ -859,6 +862,15 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( coin_selection_params.m_change_fee = coin_selection_params.m_effective_feerate.GetFee(coin_selection_params.change_output_size); coin_selection_params.m_cost_of_change = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size) + coin_selection_params.m_change_fee; + coin_selection_params.m_min_change_target = GenerateChangeTarget(std::floor(recipients_sum / vecSend.size()), coin_selection_params.m_change_fee, rng_fast); + + // The smallest change amount should be: + // 1. at least equal to dust threshold + // 2. at least 1 sat greater than fees to spend it at m_discard_feerate + const auto dust = GetDustThreshold(change_prototype_txout, coin_selection_params.m_discard_feerate); + const auto change_spend_fee = coin_selection_params.m_discard_feerate.GetFee(coin_selection_params.change_spend_size); + coin_selection_params.min_viable_change = std::max(change_spend_fee + 1, dust); + // vouts to the payees if (!coin_selection_params.m_subtract_fee_outputs) { coin_selection_params.tx_noinputs_size = 10; // Static vsize overhead + outputs vsize. 4 nVersion, 4 nLocktime, 1 input count, 1 witness overhead (dummy, flag, stack size) @@ -874,7 +886,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( } if (IsDust(txout, wallet.chain().relayDustFee())) { - return _("Transaction amount too small"); + return util::Error{_("Transaction amount too small")}; } txNew.vout.push_back(txout); } @@ -895,27 +907,24 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Choose coins to use std::optional<SelectionResult> result = SelectCoins(wallet, available_coins, /*nTargetValue=*/selection_target, coin_control, coin_selection_params); if (!result) { - return _("Insufficient funds"); + return util::Error{_("Insufficient funds")}; } - TRACE5(coin_selection, selected_coins, wallet.GetName().c_str(), GetAlgorithmName(result->m_algo).c_str(), result->m_target, result->GetWaste(), result->GetSelectedValue()); - - // Always make a change output - // We will reduce the fee from this change output later, and remove the output if it is too small. - const CAmount change_and_fee = result->GetSelectedValue() - recipients_sum; - assert(change_and_fee >= 0); - CTxOut newTxOut(change_and_fee, scriptChange); - - if (nChangePosInOut == -1) { - // Insert change txn at random position: - nChangePosInOut = rng_fast.randrange(txNew.vout.size() + 1); - } - else if ((unsigned int)nChangePosInOut > txNew.vout.size()) { - return _("Transaction change output index out of range"); + 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); + if (change_amount > 0) { + CTxOut newTxOut(change_amount, scriptChange); + if (nChangePosInOut == -1) { + // Insert change txn at random position: + nChangePosInOut = rng_fast.randrange(txNew.vout.size() + 1); + } else if ((unsigned int)nChangePosInOut > txNew.vout.size()) { + return util::Error{_("Transaction change output index out of range")}; + } + txNew.vout.insert(txNew.vout.begin() + nChangePosInOut, newTxOut); + } else { + nChangePosInOut = -1; } - assert(nChangePosInOut != -1); - auto change_position = txNew.vout.insert(txNew.vout.begin() + nChangePosInOut, newTxOut); - // Shuffle selected coins and fill in final vin std::vector<COutput> selected_coins = result->GetShuffledInputVector(); @@ -937,44 +946,25 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( TxSize tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control); int nBytes = tx_sizes.vsize; if (nBytes == -1) { - return _("Missing solving data for estimating transaction size"); - } - nFeeRet = coin_selection_params.m_effective_feerate.GetFee(nBytes); - - // Subtract fee from the change output if not subtracting it from recipient outputs - CAmount fee_needed = nFeeRet; - if (!coin_selection_params.m_subtract_fee_outputs) { - change_position->nValue -= fee_needed; - } - - // We want to drop the change to fees if: - // 1. The change output would be dust - // 2. The change is within the (almost) exact match window, i.e. it is less than or equal to the cost of the change output (cost_of_change) - CAmount change_amount = change_position->nValue; - if (IsDust(*change_position, coin_selection_params.m_discard_feerate) || change_amount <= coin_selection_params.m_cost_of_change) - { - nChangePosInOut = -1; - change_amount = 0; - txNew.vout.erase(change_position); - - // Because we have dropped this change, the tx size and required fee will be different, so let's recalculate those - tx_sizes = CalculateMaximumSignedTxSize(CTransaction(txNew), &wallet, &coin_control); - nBytes = tx_sizes.vsize; - fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); + return util::Error{_("Missing solving data for estimating transaction size")}; } + CAmount fee_needed = coin_selection_params.m_effective_feerate.GetFee(nBytes); + nFeeRet = result->GetSelectedValue() - recipients_sum - change_amount; - // The only time that fee_needed should be less than the amount available for fees (in change_and_fee - change_amount) is when + // The only time that fee_needed should be less than the amount available for fees is when // we are subtracting the fee from the outputs. If this occurs at any other time, it is a bug. - assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= change_and_fee - change_amount); + assert(coin_selection_params.m_subtract_fee_outputs || fee_needed <= nFeeRet); - // Update nFeeRet in case fee_needed changed due to dropping the change output - if (fee_needed <= change_and_fee - change_amount) { - nFeeRet = change_and_fee - change_amount; + // If there is a change output and we overpay the fees then increase the change to match the fee needed + if (nChangePosInOut != -1 && fee_needed < nFeeRet) { + auto& change = txNew.vout.at(nChangePosInOut); + change.nValue += nFeeRet - fee_needed; + nFeeRet = fee_needed; } // Reduce output values for subtractFeeFromAmount if (coin_selection_params.m_subtract_fee_outputs) { - CAmount to_reduce = fee_needed + change_amount - change_and_fee; + CAmount to_reduce = fee_needed - nFeeRet; int i = 0; bool fFirst = true; for (const auto& recipient : vecSend) @@ -997,9 +987,9 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Error if this output is reduced to be below dust if (IsDust(txout, wallet.chain().relayDustFee())) { if (txout.nValue < 0) { - return _("The transaction amount is too small to pay the fee"); + return util::Error{_("The transaction amount is too small to pay the fee")}; } else { - return _("The transaction amount is too small to send after the fee has been deducted"); + return util::Error{_("The transaction amount is too small to send after the fee has been deducted")}; } } } @@ -1010,11 +1000,11 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( // Give up if change keypool ran out and change is required if (scriptChange.empty() && nChangePosInOut != -1) { - return error; + return util::Error{error}; } if (sign && !wallet.SignTransaction(txNew)) { - return _("Signing transaction failed"); + return util::Error{_("Signing transaction failed")}; } // Return the constructed transaction data. @@ -1024,17 +1014,17 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( if ((sign && GetTransactionWeight(*tx) > MAX_STANDARD_TX_WEIGHT) || (!sign && tx_sizes.weight > MAX_STANDARD_TX_WEIGHT)) { - return _("Transaction too large"); + return util::Error{_("Transaction too large")}; } if (nFeeRet > wallet.m_default_max_tx_fee) { - return TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED); + return util::Error{TransactionErrorString(TransactionError::MAX_FEE_EXCEEDED)}; } if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits if (!wallet.chain().checkChainLimits(tx)) { - return _("Transaction has too long of a mempool chain"); + return util::Error{_("Transaction has too long of a mempool chain")}; } } @@ -1053,7 +1043,7 @@ static BResult<CreatedTransactionResult> CreateTransactionInternal( return CreatedTransactionResult(tx, nFeeRet, nChangePosInOut, feeCalc); } -BResult<CreatedTransactionResult> CreateTransaction( +util::Result<CreatedTransactionResult> CreateTransaction( CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, @@ -1061,28 +1051,26 @@ BResult<CreatedTransactionResult> CreateTransaction( bool sign) { if (vecSend.empty()) { - return _("Transaction must have at least one recipient"); + return util::Error{_("Transaction must have at least one recipient")}; } if (std::any_of(vecSend.cbegin(), vecSend.cend(), [](const auto& recipient){ return recipient.nAmount < 0; })) { - return _("Transaction amounts must not be negative"); + return util::Error{_("Transaction amounts must not be negative")}; } LOCK(wallet.cs_wallet); auto res = CreateTransactionInternal(wallet, vecSend, change_pos, coin_control, sign); - TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), res.HasRes(), - res ? res.GetObj().fee : 0, res ? res.GetObj().change_pos : 0); + TRACE4(coin_selection, normal_create_tx_internal, wallet.GetName().c_str(), bool(res), + res ? res->fee : 0, res ? res->change_pos : 0); if (!res) return res; - const auto& txr_ungrouped = res.GetObj(); + const auto& txr_ungrouped = *res; // try with avoidpartialspends unless it's enabled already if (txr_ungrouped.fee > 0 /* 0 means non-functional fee rate estimation */ && wallet.m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { TRACE1(coin_selection, attempting_aps_create_tx, wallet.GetName().c_str()); CCoinControl tmp_cc = coin_control; tmp_cc.m_avoid_partial_spends = true; - auto res_tx_grouped = CreateTransactionInternal(wallet, vecSend, change_pos, tmp_cc, sign); - // Helper optional class for now - std::optional<CreatedTransactionResult> txr_grouped{res_tx_grouped.HasRes() ? std::make_optional(res_tx_grouped.GetObj()) : std::nullopt}; + 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}; TRACE5(coin_selection, aps_create_tx_internal, wallet.GetName().c_str(), use_aps, txr_grouped.has_value(), @@ -1090,7 +1078,7 @@ BResult<CreatedTransactionResult> CreateTransaction( if (txr_grouped) { wallet.WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", txr_ungrouped.fee, txr_grouped->fee, use_aps ? "grouped" : "non-grouped"); - if (use_aps) return res_tx_grouped; + if (use_aps) return txr_grouped; } } return res; @@ -1120,21 +1108,25 @@ bool FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& nFeeRet, wallet.chain().findCoins(coins); for (const CTxIn& txin : tx.vin) { - // if it's not in the wallet and corresponding UTXO is found than select as external output const auto& outPoint = txin.prevout; - if (wallet.mapWallet.find(outPoint.hash) == wallet.mapWallet.end() && !coins[outPoint].out.IsNull()) { - coinControl.SelectExternal(outPoint, coins[outPoint].out); - } else { + if (wallet.IsMine(outPoint)) { + // The input was found in the wallet, so select as internal coinControl.Select(outPoint); + } else if (coins[outPoint].out.IsNull()) { + error = _("Unable to find UTXO for external input"); + return false; + } else { + // The input was not in the wallet, but is in the UTXO set, so select as external + coinControl.SelectExternal(outPoint, coins[outPoint].out); } } auto res = CreateTransaction(wallet, vecSend, nChangePosInOut, coinControl, false); if (!res) { - error = res.GetError(); + error = util::ErrorString(res); return false; } - const auto& txr = res.GetObj(); + const auto& txr = *res; CTransactionRef tx_new = txr.tx; nFeeRet = txr.fee; nChangePosInOut = txr.change_pos; diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 96ecd690be..c29e5be5c7 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -34,26 +34,22 @@ TxSize CalculateMaximumSignedTxSize(const CTransaction& tx, const CWallet* walle * This struct is really just a wrapper around OutputType vectors with a convenient * method for concatenating and returning all COutputs as one vector. * - * clear(), size() methods are implemented so that one can interact with - * the CoinsResult struct as if it was a vector + * Size(), Clear(), Erase(), Shuffle(), and Add() methods are implemented to + * allow easy interaction with the struct. */ struct CoinsResult { - /** Vectors for each OutputType */ - std::vector<COutput> legacy; - std::vector<COutput> P2SH_segwit; - std::vector<COutput> bech32; - std::vector<COutput> bech32m; - - /** Other is a catch-all for anything that doesn't match the known OutputTypes */ - std::vector<COutput> other; + std::map<OutputType, std::vector<COutput>> coins; /** Concatenate and return all COutputs as one vector */ - std::vector<COutput> all() const; + std::vector<COutput> All() const; /** The following methods are provided so that CoinsResult can mimic a vector, * i.e., methods can work with individual OutputType vectors or on the entire object */ - uint64_t size() const; - void clear(); + size_t Size() const; + void Clear(); + void Erase(std::set<COutPoint>& preset_coins); + void Shuffle(FastRandomContext& rng_fast); + void Add(OutputType type, const COutput& out); /** Sum of all available coins */ CAmount total_amount{0}; @@ -155,7 +151,7 @@ struct CreatedTransactionResult * selected by SelectCoins(); Also create the change output, when needed * @note passing change_pos as -1 will result in setting a random position */ -BResult<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true); +util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, int change_pos, const CCoinControl& coin_control, bool sign = true); /** * Insert additional inputs into the transaction by diff --git a/src/wallet/test/availablecoins_tests.cpp b/src/wallet/test/availablecoins_tests.cpp index 01d24da981..2427a343d5 100644 --- a/src/wallet/test/availablecoins_tests.cpp +++ b/src/wallet/test/availablecoins_tests.cpp @@ -33,7 +33,7 @@ public: constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy); BOOST_CHECK(res); - tx = res.GetObj().tx; + tx = res->tx; } wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; @@ -44,6 +44,7 @@ public: 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()); @@ -57,14 +58,14 @@ public: BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) { CoinsResult available_coins; - BResult<CTxDestination> dest; + 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.other.size(), 1U); + 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 @@ -75,30 +76,31 @@ BOOST_FIXTURE_TEST_CASE(BasicOutputTypesTest, AvailableCoinsTestingSetup) // Bech32m dest = wallet->GetNewDestination(OutputType::BECH32M, ""); - BOOST_ASSERT(dest.HasRes()); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 1 * COIN, /*fSubtractFeeFromAmount=*/true}); + BOOST_ASSERT(dest); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 1 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.bech32m.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::BECH32M].size(), 2U); // Bech32 dest = wallet->GetNewDestination(OutputType::BECH32, ""); - BOOST_ASSERT(dest.HasRes()); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 2 * COIN, /*fSubtractFeeFromAmount=*/true}); + BOOST_ASSERT(dest); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 2 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.bech32.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::BECH32].size(), 2U); // P2SH-SEGWIT dest = wallet->GetNewDestination(OutputType::P2SH_SEGWIT, ""); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 3 * COIN, /*fSubtractFeeFromAmount=*/true}); + BOOST_ASSERT(dest); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 3 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.P2SH_segwit.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::P2SH_SEGWIT].size(), 2U); // Legacy (P2PKH) dest = wallet->GetNewDestination(OutputType::LEGACY, ""); - BOOST_ASSERT(dest.HasRes()); - AddTx(CRecipient{{GetScriptForDestination(dest.GetObj())}, 4 * COIN, /*fSubtractFeeFromAmount=*/true}); + BOOST_ASSERT(dest); + AddTx(CRecipient{{GetScriptForDestination(*dest)}, 4 * COIN, /*fSubtractFeeFromAmount=*/true}); available_coins = AvailableCoins(*wallet); - BOOST_CHECK_EQUAL(available_coins.legacy.size(), 2U); + BOOST_CHECK_EQUAL(available_coins.coins[OutputType::LEGACY].size(), 2U); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index cd7fd3f4dd..30a7fb9eb6 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -74,9 +74,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; if (spendable) { - auto op_dest = wallet.GetNewDestination(OutputType::BECH32, ""); - assert(op_dest.HasRes()); - tx.vout[nInput].scriptPubKey = GetScriptForDestination(op_dest.GetObj()); + tx.vout[nInput].scriptPubKey = GetScriptForDestination(*Assert(wallet.GetNewDestination(OutputType::BECH32, ""))); } uint256 txid = tx.GetHash(); @@ -85,7 +83,7 @@ static void add_coin(CoinsResult& available_coins, CWallet& wallet, const CAmoun assert(ret.second); CWalletTx& wtx = (*ret.first).second; const auto& txout = wtx.tx->vout.at(nInput); - available_coins.bech32.emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); + available_coins.coins[OutputType::BECH32].emplace_back(COutPoint(wtx.GetHash(), nInput), txout, nAge, CalculateMaximumSignedInputSize(txout, &wallet, /*coin_control=*/nullptr), /*spendable=*/ true, /*solvable=*/ true, /*safe=*/ true, wtx.GetTxTime(), fIsFromMe, feerate); } /** Check if SelectionResult a is equivalent to SelectionResult b. @@ -250,9 +248,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Iteration exhaustion test CAmount target = make_hard_case(17, utxo_pool); - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 0)); // Should exhaust + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(utxo_pool), target, 1)); // Should exhaust target = make_hard_case(14, utxo_pool); - const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 0); // Should not exhaust + const auto result7 = SelectCoinsBnB(GroupCoins(utxo_pool), target, 1); // Should not exhaust BOOST_CHECK(result7); // Test same value early bailout optimization @@ -291,8 +289,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Make sure that effective value is working in AttemptSelection when BnB is used CoinSelectionParams coin_selection_params_bnb{ rand, - /*change_output_size=*/ 0, - /*change_spend_size=*/ 0, + /*change_output_size=*/ 31, + /*change_spend_size=*/ 68, /*min_change_target=*/ 0, /*effective_feerate=*/ CFeeRate(3000), /*long_term_feerate=*/ CFeeRate(1000), @@ -300,6 +298,9 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) /*tx_noinputs_size=*/ 0, /*avoid_partial=*/ false, }; + 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); { std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); wallet->LoadWallet(); @@ -310,15 +311,15 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CoinsResult available_coins; add_coin(available_coins, *wallet, 1, coin_selection_params_bnb.m_effective_feerate); - available_coins.all().at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail - BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.all()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change)); + available_coins.All().at(0).input_bytes = 40; // Make sure that it has a negative effective value. The next check should assert if this somehow got through. Otherwise it will fail + BOOST_CHECK(!SelectCoinsBnB(GroupCoins(available_coins.All()), 1 * CENT, coin_selection_params_bnb.m_cost_of_change)); // Test fees subtracted from output: - available_coins.clear(); + 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; + 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); + 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); } @@ -337,7 +338,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(available_coins, *wallet, 2 * CENT, coin_selection_params_bnb.m_effective_feerate, 6 * 24, false, 0, true); CCoinControl coin_control; coin_control.m_allow_other_inputs = true; - coin_control.Select(available_coins.all().at(0).outpoint); + coin_control.Select(available_coins.All().at(0).outpoint); coin_selection_params_bnb.m_effective_feerate = CFeeRate(0); const auto result10 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(result10); @@ -364,7 +365,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) CCoinControl coin_control; const auto result11 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result11)); - available_coins.clear(); + available_coins.Clear(); // more coins should be selected when effective fee < long term fee coin_selection_params_bnb.m_effective_feerate = CFeeRate(3000); @@ -379,7 +380,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(1 * CENT, 2, expected_result); const auto result12 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result12)); - available_coins.clear(); + available_coins.Clear(); // pre selected coin should be selected even if disadvantageous coin_selection_params_bnb.m_effective_feerate = CFeeRate(5000); @@ -393,7 +394,7 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) add_coin(9 * CENT, 2, expected_result); add_coin(1 * CENT, 2, expected_result); coin_control.m_allow_other_inputs = true; - coin_control.Select(available_coins.all().at(1).outpoint); // pre select 9 coin + coin_control.Select(available_coins.All().at(1).outpoint); // pre select 9 coin const auto result13 = SelectCoins(*wallet, available_coins, 10 * CENT, coin_control, coin_selection_params_bnb); BOOST_CHECK(EquivalentResult(expected_result, *result13)); } @@ -415,28 +416,28 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test multiple times to allow for differences in the shuffle order for (int i = 0; i < RUN_TESTS; i++) { - available_coins.clear(); + 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.All(), *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.All(), *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.All(), *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.All(), *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.All(), *wallet, filter_confirmed), 3 * CENT, CENT); BOOST_CHECK(result2); BOOST_CHECK_EQUAL(result2->GetSelectedValue(), 3 * CENT); @@ -447,44 +448,44 @@ 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.All(), *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.All(), *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.All(), *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.All(), *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.All(), *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.All(), *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.All(), *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.All(), *wallet, filter_confirmed), 9 * CENT, CENT); BOOST_CHECK(result8); BOOST_CHECK_EQUAL(result8->GetSelectedValue(), 10 * CENT); BOOST_CHECK_EQUAL(result8->GetInputSet().size(), 1U); // now clear out the wallet and start again to test choosing between subsets of smaller coins and the next biggest coin - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, 6*CENT); add_coin(available_coins, *wallet, 7*CENT); @@ -493,12 +494,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.All(), *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.All(), *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.All(), *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); @@ -506,7 +507,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.All(), *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); @@ -514,13 +515,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.All(), *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.All(), *wallet, filter_confirmed), 11 * CENT, CENT); BOOST_CHECK(result13); BOOST_CHECK_EQUAL(result13->GetSelectedValue(), 11 * CENT); BOOST_CHECK_EQUAL(result13->GetInputSet().size(), 2U); @@ -530,19 +531,19 @@ 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.All(), *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.All(), *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); // empty the wallet and start again, now with fractions of a cent, to test small change avoidance - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 1 / 10); add_coin(available_coins, *wallet, CENT * 2 / 10); add_coin(available_coins, *wallet, CENT * 3 / 10); @@ -551,7 +552,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.All(), *wallet, filter_confirmed), CENT, CENT); BOOST_CHECK(result16); BOOST_CHECK_EQUAL(result16->GetSelectedValue(), CENT); @@ -559,7 +560,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.All(), *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result17); BOOST_CHECK_EQUAL(result17->GetSelectedValue(), 1 * CENT); // we should get the exact amount @@ -568,17 +569,17 @@ 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.All(), *wallet, filter_confirmed), 1 * CENT, CENT); BOOST_CHECK(result18); BOOST_CHECK_EQUAL(result18->GetSelectedValue(), 1 * CENT); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) // they tried to consolidate 10 50k coins into one 500k coin, and ended up with 50k in change - available_coins.clear(); + available_coins.Clear(); 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.All(), *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 @@ -587,41 +588,41 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // we need to try finding an exact subset anyway // sometimes it will fail, and so we use the next biggest coin: - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 5 / 10); 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.All(), *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); // but sometimes it's possible, and we use an exact subset (0.4 + 0.6 = 1.0) - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 4 / 10); 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.All(), *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 // test avoiding small change - available_coins.clear(); + available_coins.Clear(); add_coin(available_coins, *wallet, CENT * 5 / 100); add_coin(available_coins, *wallet, CENT * 1); 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.All(), *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.All(), *wallet, filter_confirmed), CENT * 9990 / 100, CENT); BOOST_CHECK(result23); BOOST_CHECK_EQUAL(result23->GetSelectedValue(), 101 * CENT); BOOST_CHECK_EQUAL(result23->GetInputSet().size(), 2U); @@ -629,14 +630,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test with many inputs for (CAmount amt=1500; amt < COIN; amt*=10) { - available_coins.clear(); + available_coins.Clear(); // Create 676 inputs (= (old MAX_STANDARD_TX_SIZE == 100000) / 148 bytes per input) for (uint16_t j = 0; j < 676; j++) add_coin(available_coins, *wallet, amt); // 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.All(), *wallet, filter_confirmed), 2000, CENT); BOOST_CHECK(result24); if (amt - 2000 < CENT) { @@ -655,7 +656,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // test randomness { - available_coins.clear(); + available_coins.Clear(); for (int i2 = 0; i2 < 100; i2++) add_coin(available_coins, *wallet, COIN); @@ -663,9 +664,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int i = 0; i < RUN_TESTS; i++) { // picking 50 from 100 coins doesn't depend on the shuffle, // but does depend on randomness in the stochastic approximation code - const auto result25 = KnapsackSolver(GroupCoins(available_coins.all()), 50 * COIN, CENT); + const auto result25 = KnapsackSolver(GroupCoins(available_coins.All()), 50 * COIN, CENT); BOOST_CHECK(result25); - const auto result26 = KnapsackSolver(GroupCoins(available_coins.all()), 50 * COIN, CENT); + const auto result26 = KnapsackSolver(GroupCoins(available_coins.All()), 50 * COIN, CENT); BOOST_CHECK(result26); BOOST_CHECK(!EqualResult(*result25, *result26)); @@ -676,9 +677,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // When choosing 1 from 100 identical coins, 1% of the time, this test will choose the same coin twice // which will cause it to fail. // To avoid that issue, run the test RANDOM_REPEATS times and only complain if all of them fail - const auto result27 = KnapsackSolver(GroupCoins(available_coins.all()), COIN, CENT); + const auto result27 = KnapsackSolver(GroupCoins(available_coins.All()), COIN, CENT); BOOST_CHECK(result27); - const auto result28 = KnapsackSolver(GroupCoins(available_coins.all()), COIN, CENT); + const auto result28 = KnapsackSolver(GroupCoins(available_coins.All()), COIN, CENT); BOOST_CHECK(result28); if (EqualResult(*result27, *result28)) fails++; @@ -699,9 +700,9 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) int fails = 0; for (int j = 0; j < RANDOM_REPEATS; j++) { - const auto result29 = KnapsackSolver(GroupCoins(available_coins.all()), 90 * CENT, CENT); + const auto result29 = KnapsackSolver(GroupCoins(available_coins.All()), 90 * CENT, CENT); BOOST_CHECK(result29); - const auto result30 = KnapsackSolver(GroupCoins(available_coins.all()), 90 * CENT, CENT); + const auto result30 = KnapsackSolver(GroupCoins(available_coins.All()), 90 * CENT, CENT); BOOST_CHECK(result30); if (EqualResult(*result29, *result30)) fails++; @@ -727,7 +728,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.All(), *wallet, filter_standard), 1003 * COIN, CENT, rand); BOOST_CHECK(result); BOOST_CHECK_EQUAL(result->GetSelectedValue(), 1003 * COIN); BOOST_CHECK_EQUAL(result->GetInputSet().size(), 2U); @@ -779,6 +780,8 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) /*tx_noinputs_size=*/ 0, /*avoid_partial=*/ false, }; + cs_params.m_cost_of_change = 1; + cs_params.min_viable_change = 1; CCoinControl cc; const auto result = SelectCoins(*wallet, available_coins, target, cc, cs_params); BOOST_CHECK(result); diff --git a/src/wallet/test/feebumper_tests.cpp b/src/wallet/test/feebumper_tests.cpp new file mode 100644 index 0000000000..6add86dc3d --- /dev/null +++ b/src/wallet/test/feebumper_tests.cpp @@ -0,0 +1,54 @@ +// 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 <primitives/transaction.h> +#include <script/script.h> +#include <util/strencodings.h> +#include <wallet/feebumper.h> +#include <wallet/test/util.h> +#include <wallet/test/wallet_test_fixture.h> + +#include <boost/test/unit_test.hpp> + +namespace wallet { +namespace feebumper { +BOOST_FIXTURE_TEST_SUITE(feebumper_tests, WalletTestingSetup) + +static void CheckMaxWeightComputation(const std::string& script_str, const std::vector<std::string>& witness_str_stack, const std::string& prevout_script_str, int64_t expected_max_weight) +{ + std::vector script_data(ParseHex(script_str)); + CScript script(script_data.begin(), script_data.end()); + CTxIn input(uint256(), 0, script); + + for (const auto& s : witness_str_stack) { + input.scriptWitness.stack.push_back(ParseHex(s)); + } + + std::vector prevout_script_data(ParseHex(prevout_script_str)); + CScript prevout_script(prevout_script_data.begin(), prevout_script_data.end()); + + int64_t weight = GetTransactionInputWeight(input); + SignatureWeights weights; + SignatureWeightChecker size_checker(weights, DUMMY_CHECKER); + bool script_ok = VerifyScript(input.scriptSig, prevout_script, &input.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, size_checker); + BOOST_CHECK(script_ok); + weight += weights.GetWeightDiffToMax(); + BOOST_CHECK_EQUAL(weight, expected_max_weight); +} + +BOOST_AUTO_TEST_CASE(external_max_weight_test) +{ + // P2PKH + CheckMaxWeightComputation("453042021f03c8957c5ce12940ee6e3333ecc3f633d9a1ac53a55b3ce0351c617fa96abe021f0dccdcce3ef45a63998be9ec748b561baf077b8e862941d0cd5ec08f5afe68012102fccfeb395f0ecd3a77e7bc31c3bc61dc987418b18e395d441057b42ca043f22c", {}, "76a914f60dcfd3392b28adc7662669603641f578eed72d88ac", 593); + // P2SH-P2WPKH + CheckMaxWeightComputation("160014001dca1b22c599b5a56a87c78417ad2ff39552f1", {"3042021f5443c58eaf45f3e5ef46f8516f966b334a7d497cedda4edb2b9fad57c90c3b021f63a77cb56cde848e2e2dd20b487eec2f53101f634193786083f60b4d23a82301", "026cfe86116f161057deb240201d6b82ebd4f161e0200d63dc9aca65a1d6b38bb7"}, "a9147c8ab5ad7708b97ccb6b483d57aba48ee85214df87", 364); + // P2WPKH + CheckMaxWeightComputation("", {"3042021f0f8906f0394979d5b737134773e5b88bf036c7d63542301d600ab677ba5a59021f0e9fe07e62c113045fa1c1532e2914720e8854d189c4f5b8c88f57956b704401", "0359edba11ed1a0568094a6296a16c4d5ee4c8cfe2f5e2e6826871b5ecf8188f79"}, "00149961a78658030cc824af4c54fbf5294bec0cabdd", 272); + // P2WSH HTLC + CheckMaxWeightComputation("", {"3042021f5c4c29e6b686aae5b6d0751e90208592ea96d26bc81d78b0d3871a94a21fa8021f74dc2f971e438ccece8699c8fd15704c41df219ab37b63264f2147d15c34d801", "01", "6321024cf55e52ec8af7866617dc4e7ff8433758e98799906d80e066c6f32033f685f967029000b275210214827893e2dcbe4ad6c20bd743288edad21100404eb7f52ccd6062fd0e7808f268ac"}, "002089e84892873c679b1129edea246e484fd914c2601f776d4f2f4a001eb8059703", 318); +} + +BOOST_AUTO_TEST_SUITE_END() +} // namespace feebumper +} // namespace wallet diff --git a/src/wallet/test/fuzz/coinselection.cpp b/src/wallet/test/fuzz/coinselection.cpp index 3465f2f331..90a328179e 100644 --- a/src/wallet/test/fuzz/coinselection.cpp +++ b/src/wallet/test/fuzz/coinselection.cpp @@ -58,6 +58,8 @@ FUZZ_TARGET(coinselection) coin_params.m_subtract_fee_outputs = subtract_fee_outputs; coin_params.m_long_term_feerate = long_term_fee_rate; coin_params.m_effective_feerate = effective_fee_rate; + coin_params.change_output_size = fuzzed_data_provider.ConsumeIntegralInRange<int>(10, 1000); + coin_params.m_change_fee = effective_fee_rate.GetFee(coin_params.change_output_size); // Create some coins CAmount total_balance{0}; @@ -83,11 +85,11 @@ FUZZ_TARGET(coinselection) const auto result_bnb = SelectCoinsBnB(group_pos, target, cost_of_change); auto result_srd = SelectCoinsSRD(group_pos, target, fast_random_context); - if (result_srd) result_srd->ComputeAndSetWaste(cost_of_change); + if (result_srd) result_srd->ComputeAndSetWaste(cost_of_change, cost_of_change, 0); - CAmount change_target{GenerateChangeTarget(target, fast_random_context)}; + CAmount change_target{GenerateChangeTarget(target, coin_params.m_change_fee, fast_random_context)}; auto result_knapsack = KnapsackSolver(group_all, target, change_target, fast_random_context); - if (result_knapsack) result_knapsack->ComputeAndSetWaste(cost_of_change); + if (result_knapsack) result_knapsack->ComputeAndSetWaste(cost_of_change, cost_of_change, 0); // If the total balance is sufficient for the target and we are not using // effective values, Knapsack should always find a solution. diff --git a/src/wallet/test/fuzz/notifications.cpp b/src/wallet/test/fuzz/notifications.cpp index 5c173773e4..5e9cd4001b 100644 --- a/src/wallet/test/fuzz/notifications.cpp +++ b/src/wallet/test/fuzz/notifications.cpp @@ -69,14 +69,13 @@ struct FuzzedWallet { CScript GetScriptPubKey(FuzzedDataProvider& fuzzed_data_provider) { auto type{fuzzed_data_provider.PickValueInArray(OUTPUT_TYPES)}; - BResult<CTxDestination> op_dest; + util::Result<CTxDestination> op_dest{util::Error{}}; if (fuzzed_data_provider.ConsumeBool()) { op_dest = wallet->GetNewDestination(type, ""); } else { op_dest = wallet->GetNewChangeDestination(type); } - assert(op_dest.HasRes()); - return GetScriptForDestination(op_dest.GetObj()); + return GetScriptForDestination(*Assert(op_dest)); } }; diff --git a/src/wallet/test/ismine_tests.cpp b/src/wallet/test/ismine_tests.cpp index dd5cd0af46..68146eb079 100644 --- a/src/wallet/test/ismine_tests.cpp +++ b/src/wallet/test/ismine_tests.cpp @@ -43,11 +43,13 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore does not have key result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has key BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // P2PK uncompressed @@ -60,11 +62,13 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore does not have key result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has key BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // P2PKH compressed @@ -77,11 +81,13 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore does not have key result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has key BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // P2PKH uncompressed @@ -94,11 +100,13 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore does not have key result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has key BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // P2SH @@ -113,16 +121,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore does not have redeemScript or key result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has redeemScript but no key BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(redeemScript)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has redeemScript and key BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // (P2PKH inside) P2SH inside P2SH (invalid) @@ -141,6 +152,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // (P2PKH inside) P2SH inside P2WSH (invalid) @@ -159,6 +171,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // P2WPKH inside P2WSH (invalid) @@ -175,6 +188,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // (P2PKH inside) P2WSH inside P2WSH (invalid) @@ -193,6 +207,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // P2WPKH compressed @@ -208,6 +223,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(scriptPubKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // P2WPKH uncompressed @@ -222,11 +238,13 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore has key, but no P2SH redeemScript result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has key and P2SH redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(scriptPubKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // scriptPubKey multisig @@ -240,24 +258,28 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore does not have any keys result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has 1/2 keys BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(uncompressedKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has 2/2 keys BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[1])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has 2/2 keys and the script BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(scriptPubKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // P2SH multisig @@ -274,11 +296,13 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore has no redeemScript result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(redeemScript)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // P2WSH multisig with compressed keys @@ -295,16 +319,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore has keys, but no witnessScript or P2SH redeemScript result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has keys and witnessScript, but no P2SH redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(witnessScript)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has keys, witnessScript, P2SH redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(scriptPubKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // P2WSH multisig with uncompressed key @@ -321,16 +348,19 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore has keys, but no witnessScript or P2SH redeemScript result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has keys and witnessScript, but no P2SH redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(witnessScript)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has keys, witnessScript, P2SH redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(scriptPubKey)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // P2WSH multisig wrapped in P2SH @@ -346,18 +376,21 @@ BOOST_AUTO_TEST_CASE(ismine_standard) // Keystore has no witnessScript, P2SH redeemScript, or keys result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has witnessScript and P2SH redeemScript, but no keys BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(redeemScript)); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddCScript(witnessScript)); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); // Keystore has keys, witnessScript, P2SH redeemScript BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[0])); BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->AddKey(keys[1])); result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_SPENDABLE); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 1); } // OP_RETURN @@ -372,6 +405,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // witness unspendable @@ -386,6 +420,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // witness unknown @@ -400,6 +435,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } // Nonstandard @@ -414,6 +450,7 @@ BOOST_AUTO_TEST_CASE(ismine_standard) result = keystore.GetLegacyScriptPubKeyMan()->IsMine(scriptPubKey); BOOST_CHECK_EQUAL(result, ISMINE_NO); + BOOST_CHECK(keystore.GetLegacyScriptPubKeyMan()->GetScriptPubKeys().count(scriptPubKey) == 0); } } diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index 53c3b5d2ae..a75b014870 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, 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()), m_args, coinbaseKey); // Check that a subtract-from-recipient transaction slightly less than the // coinbase input amount does not create a change output (because it would @@ -35,7 +35,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) coin_control.m_change_type = OutputType::LEGACY; auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, coin_control); BOOST_CHECK(res); - const auto& txr = res.GetObj(); + const auto& txr = *res; BOOST_CHECK_EQUAL(txr.tx->vout.size(), 1); BOOST_CHECK_EQUAL(txr.tx->vout[0].nValue, recipient.nAmount + leftover_input_amount - txr.fee); BOOST_CHECK_GT(txr.fee, 0); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 81183aa738..60fdbde71b 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -91,16 +91,17 @@ static void AddKey(CWallet& wallet, const CKey& key) BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) { // Cap last block file size, and mine new block in a new block file. - CBlockIndex* oldTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* oldTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); WITH_LOCK(::cs_main, m_node.chainman->m_blockman.GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - CBlockIndex* newTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* newTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); // Verify ScanForWalletTransactions fails to read an unknown start block. { CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); + LOCK(Assert(m_node.chainman)->GetMutex()); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } @@ -121,6 +122,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CWallet wallet(m_node.chain.get(), "", m_args, CreateMockWalletDatabase()); { LOCK(wallet.cs_wallet); + LOCK(Assert(m_node.chainman)->GetMutex()); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } @@ -165,6 +167,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); + LOCK(Assert(m_node.chainman)->GetMutex()); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } @@ -192,6 +195,7 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); { LOCK(wallet.cs_wallet); + LOCK(Assert(m_node.chainman)->GetMutex()); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } @@ -210,10 +214,10 @@ BOOST_FIXTURE_TEST_CASE(scan_for_wallet_transactions, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) { // Cap last block file size, and mine new block in a new block file. - CBlockIndex* oldTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* oldTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); WITH_LOCK(::cs_main, m_node.chainman->m_blockman.GetBlockFileInfo(oldTip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - CBlockIndex* newTip = m_node.chainman->ActiveChain().Tip(); + CBlockIndex* newTip = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()); // Prune the older block file. int file_number; @@ -277,7 +281,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) { // Create two blocks with same timestamp to verify that importwallet rescan // will pick up both blocks, not just the first. - const int64_t BLOCK_TIME = m_node.chainman->ActiveChain().Tip()->GetBlockTimeMax() + 5; + const int64_t BLOCK_TIME = WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain().Tip()->GetBlockTimeMax() + 5); SetMockTime(BLOCK_TIME); m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); m_coinbase_txns.emplace_back(CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); @@ -302,6 +306,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) spk_man->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey()); AddWallet(context, wallet); + LOCK(Assert(m_node.chainman)->GetMutex()); wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); } JSONRPCRequest request; @@ -327,6 +332,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.setArray(); request.params.push_back(backup_file); AddWallet(context, wallet); + LOCK(Assert(m_node.chainman)->GetMutex()); wallet->SetLastBlockProcessed(m_node.chainman->ActiveChain().Height(), m_node.chainman->ActiveChain().Tip()->GetBlockHash()); wallet::importwallet().HandleRequest(request); RemoveWallet(context, wallet, /* load_on_start= */ std::nullopt); @@ -350,9 +356,10 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { CWallet wallet(m_node.chain.get(), "", m_args, CreateDummyWalletDatabase()); - CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/0}}; LOCK(wallet.cs_wallet); + LOCK(Assert(m_node.chainman)->GetMutex()); + CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/0}}; wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetupDescriptorScriptPubKeyMans(); @@ -520,7 +527,7 @@ public: ListCoinsTestingSetup() { CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); - wallet = CreateSyncedWallet(*m_node.chain, m_node.chainman->ActiveChain(), m_args, coinbaseKey); + wallet = CreateSyncedWallet(*m_node.chain, WITH_LOCK(Assert(m_node.chainman)->GetMutex(), return m_node.chainman->ActiveChain()), m_args, coinbaseKey); } ~ListCoinsTestingSetup() @@ -536,7 +543,7 @@ public: constexpr int RANDOM_CHANGE_POSITION = -1; auto res = CreateTransaction(*wallet, {recipient}, RANDOM_CHANGE_POSITION, dummy); BOOST_CHECK(res); - tx = res.GetObj().tx; + tx = res->tx; } wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; @@ -547,6 +554,7 @@ public: CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); LOCK(wallet->cs_wallet); + LOCK(Assert(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()); @@ -591,7 +599,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) // Lock both coins. Confirm number of available coins drops to 0. { LOCK(wallet->cs_wallet); - BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).size(), 2U); + BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 2U); } for (const auto& group : list) { for (const auto& coin : group.second) { @@ -601,7 +609,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoinsTest, ListCoinsTestingSetup) } { LOCK(wallet->cs_wallet); - BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).size(), 0U); + BOOST_CHECK_EQUAL(AvailableCoinsListUnspent(*wallet).Size(), 0U); } // Confirm ListCoins still returns same result as before, despite coins // being locked. @@ -843,21 +851,126 @@ BOOST_FIXTURE_TEST_CASE(ZapSelectTx, TestChain100Setup) { auto block_hash = block_tx.GetHash(); - auto prev_hash = m_coinbase_txns[0]->GetHash(); + auto prev_tx = m_coinbase_txns[0]; LOCK(wallet->cs_wallet); - BOOST_CHECK(wallet->HasWalletSpend(prev_hash)); + BOOST_CHECK(wallet->HasWalletSpend(prev_tx)); BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 1u); std::vector<uint256> vHashIn{ block_hash }, vHashOut; BOOST_CHECK_EQUAL(wallet->ZapSelectTx(vHashIn, vHashOut), DBErrors::LOAD_OK); - BOOST_CHECK(!wallet->HasWalletSpend(prev_hash)); + BOOST_CHECK(!wallet->HasWalletSpend(prev_tx)); BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_hash), 0u); } TestUnloadWallet(std::move(wallet)); } +/** 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; } + +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 {} + bool TxnBegin() override { return false; } + bool TxnCommit() override { return false; } + bool TxnAbort() override { return false; } +}; + +/** A dummy WalletDatabase that does nothing, only fails if needed.**/ +class FailDatabase : public WalletDatabase +{ +public: + bool m_pass{true}; // false when this db should fail + + void Open() override {}; + void AddRef() override {} + void RemoveRef() override {} + bool Rewrite(const char* pszSkip=nullptr) override { return true; } + bool Backup(const std::string& strDest) const override { return true; } + void Close() override {} + void Flush() override {} + bool PeriodicFlush() override { return true; } + void IncrementUpdateCounter() override { ++nUpdateCounter; } + void ReloadDbEnv() override {} + std::string Filename() override { return "faildb"; } + std::string Format() override { return "faildb"; } + std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<FailBatch>(m_pass); } +}; + +/** + * Checks a wallet invalid state where the inputs (prev-txs) of a new arriving transaction are not marked dirty, + * while the transaction that spends them exist inside the in-memory wallet tx map (not stored on db due a db write failure). + */ +BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestingSetup) +{ + CWallet wallet(m_node.chain.get(), "", m_args, std::make_unique<FailDatabase>()); + { + LOCK(wallet.cs_wallet); + wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + wallet.SetupDescriptorScriptPubKeyMans(); + } + + // Add tx to wallet + const auto& op_dest = wallet.GetNewDestination(OutputType::BECH32M, ""); + BOOST_ASSERT(op_dest); + + CMutableTransaction mtx; + mtx.vout.push_back({COIN, GetScriptForDestination(*op_dest)}); + mtx.vin.push_back(CTxIn(g_insecure_rand_ctx.rand256(), 0)); + const auto& tx_id_to_spend = wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInMempool{})->GetHash(); + + { + // Cache and verify available balance for the wtx + LOCK(wallet.cs_wallet); + const CWalletTx* wtx_to_spend = wallet.GetWalletTx(tx_id_to_spend); + BOOST_CHECK_EQUAL(CachedTxGetAvailableCredit(wallet, *wtx_to_spend), 1 * COIN); + } + + // Now the good case: + // 1) Add a transaction that spends the previously created transaction + // 2) Verify that the available balance of this new tx and the old one is updated (prev tx is marked dirty) + + mtx.vin.clear(); + mtx.vin.push_back(CTxIn(tx_id_to_spend, 0)); + wallet.transactionAddedToMempool(MakeTransactionRef(mtx), 0); + const uint256& good_tx_id = mtx.GetHash(); + + { + // Verify balance update for the new tx and the old one + LOCK(wallet.cs_wallet); + const CWalletTx* new_wtx = wallet.GetWalletTx(good_tx_id); + BOOST_CHECK_EQUAL(CachedTxGetAvailableCredit(wallet, *new_wtx), 1 * COIN); + + // Now the old wtx + const CWalletTx* wtx_to_spend = wallet.GetWalletTx(tx_id_to_spend); + BOOST_CHECK_EQUAL(CachedTxGetAvailableCredit(wallet, *wtx_to_spend), 0 * COIN); + } + + // Now the bad case: + // 1) Make db always fail + // 2) Try to add a transaction that spends the previously created transaction and + // verify that we are not moving forward if the wallet cannot store it + 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), + std::runtime_error, + HasReason("DB error adding transaction to wallet, write failed")); +} + BOOST_AUTO_TEST_SUITE_END() } // namespace wallet diff --git a/src/wallet/test/walletload_tests.cpp b/src/wallet/test/walletload_tests.cpp new file mode 100644 index 0000000000..f45b69a418 --- /dev/null +++ b/src/wallet/test/walletload_tests.cpp @@ -0,0 +1,54 @@ +// 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 <wallet/wallet.h> +#include <test/util/setup_common.h> + +#include <boost/test/unit_test.hpp> + +namespace wallet { + +BOOST_AUTO_TEST_SUITE(walletload_tests) + +class DummyDescriptor final : public Descriptor { +private: + std::string desc; +public: + explicit DummyDescriptor(const std::string& descriptor) : desc(descriptor) {}; + ~DummyDescriptor() = default; + + std::string ToString() const override { return desc; } + std::optional<OutputType> GetOutputType() const override { return OutputType::UNKNOWN; } + + bool IsRange() const override { return false; } + bool IsSolvable() const override { return false; } + bool IsSingleType() const override { return true; } + bool ToPrivateString(const SigningProvider& provider, std::string& out) const override { return false; } + bool ToNormalizedString(const SigningProvider& provider, std::string& out, const DescriptorCache* cache = nullptr) const override { return false; } + bool Expand(int pos, const SigningProvider& provider, std::vector<CScript>& output_scripts, FlatSigningProvider& out, DescriptorCache* write_cache = nullptr) const override { return false; }; + bool ExpandFromCache(int pos, const DescriptorCache& read_cache, std::vector<CScript>& output_scripts, FlatSigningProvider& out) const override { return false; } + void ExpandPrivate(int pos, const SigningProvider& provider, FlatSigningProvider& out) const override {} +}; + +BOOST_FIXTURE_TEST_CASE(wallet_load_unknown_descriptor, TestingSetup) +{ + std::unique_ptr<WalletDatabase> database = CreateMockWalletDatabase(); + { + // Write unknown active descriptor + WalletBatch batch(*database, false); + std::string unknown_desc = "trx(tpubD6NzVbkrYhZ4Y4S7m6Y5s9GD8FqEMBy56AGphZXuagajudVZEnYyBahZMgHNCTJc2at82YX6s8JiL1Lohu5A3v1Ur76qguNH4QVQ7qYrBQx/86'/1'/0'/0/*)#8pn8tzdt"; + WalletDescriptor wallet_descriptor(std::make_shared<DummyDescriptor>(unknown_desc), 0, 0, 0, 0); + BOOST_CHECK(batch.WriteDescriptor(uint256(), wallet_descriptor)); + BOOST_CHECK(batch.WriteActiveScriptPubKeyMan(static_cast<uint8_t>(OutputType::UNKNOWN), uint256(), false)); + } + + { + // 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))); + BOOST_CHECK_EQUAL(wallet->LoadWallet(), DBErrors::UNKNOWN_DESCRIPTOR); + } +} + +BOOST_AUTO_TEST_SUITE_END() +} // namespace wallet diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 271d698e56..27983e356d 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -305,6 +305,13 @@ public: CWalletTx(CWalletTx const &) = delete; void operator=(CWalletTx const &x) = delete; }; + +struct WalletTxOrderComparator { + bool operator()(const CWalletTx* a, const CWalletTx* b) const + { + return a->nOrderPos < b->nOrderPos; + } +}; } // namespace wallet #endif // BITCOIN_WALLET_TRANSACTION_H diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8667f8d6f4..c084ef10ec 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -149,6 +149,13 @@ std::vector<std::shared_ptr<CWallet>> GetWallets(WalletContext& context) return context.wallets; } +std::shared_ptr<CWallet> GetDefaultWallet(WalletContext& context, size_t& count) +{ + LOCK(context.wallets_mutex); + count = context.wallets.size(); + return count == 1 ? context.wallets[0] : nullptr; +} + std::shared_ptr<CWallet> GetWallet(WalletContext& context, const std::string& name) { LOCK(context.wallets_mutex); @@ -414,7 +421,7 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& b const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const { AssertLockHeld(cs_wallet); - std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(hash); + const auto it = mapWallet.find(hash); if (it == mapWallet.end()) return nullptr; return &(it->second); @@ -535,6 +542,7 @@ void CWallet::SetMinVersion(enum WalletFeature nVersion, WalletBatch* batch_in) LOCK(cs_wallet); if (nWalletVersion >= nVersion) return; + WalletLogPrintf("Setting minversion to %d\n", nVersion); nWalletVersion = nVersion; { @@ -551,7 +559,7 @@ std::set<uint256> CWallet::GetConflicts(const uint256& txid) const std::set<uint256> result; AssertLockHeld(cs_wallet); - std::map<uint256, CWalletTx>::const_iterator it = mapWallet.find(txid); + const auto it = mapWallet.find(txid); if (it == mapWallet.end()) return result; const CWalletTx& wtx = it->second; @@ -569,11 +577,17 @@ std::set<uint256> CWallet::GetConflicts(const uint256& txid) const return result; } -bool CWallet::HasWalletSpend(const uint256& txid) const +bool CWallet::HasWalletSpend(const CTransactionRef& tx) const { AssertLockHeld(cs_wallet); - auto iter = mapTxSpends.lower_bound(COutPoint(txid, 0)); - return (iter != mapTxSpends.end() && iter->first.hash == txid); + 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()) { + return true; + } + } + return false; } void CWallet::Flush() @@ -636,7 +650,7 @@ bool CWallet::IsSpent(const COutPoint& outpoint) const for (TxSpends::const_iterator it = range.first; it != range.second; ++it) { const uint256& wtxid = it->second; - std::map<uint256, CWalletTx>::const_iterator mit = mapWallet.find(wtxid); + const auto mit = mapWallet.find(wtxid); if (mit != mapWallet.end()) { int depth = GetTxDepthInMainChain(mit->second); if (depth > 0 || (depth == 0 && !mit->second.isAbandoned())) @@ -868,7 +882,7 @@ bool CWallet::MarkReplaced(const uint256& originalHash, const uint256& newHash) wtx.mapValue["replaced_by_txid"] = newHash.ToString(); - // Refresh mempool status without waiting for transactionRemovedFromMempool + // Refresh mempool status without waiting for transactionRemovedFromMempool or transactionAddedToMempool RefreshMempoolStatus(wtx, chain()); WalletBatch batch(GetDatabase()); @@ -1130,7 +1144,13 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxS // Block disconnection override an abandoned tx as unconfirmed // which means user may have to call abandontransaction again TxState tx_state = std::visit([](auto&& s) -> TxState { return s; }, state); - return AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block); + CWalletTx* wtx = AddToWallet(MakeTransactionRef(tx), tx_state, /*update_wtx=*/nullptr, /*fFlushOnClose=*/false, rescanning_old_block); + if (!wtx) { + // Can only be nullptr if there was a db write error (missing db, read-only db or a db engine internal writing error). + // As we only store arriving transaction in this process, and we don't want an inconsistent state, let's throw an error. + throw std::runtime_error("DB error adding transaction to wallet, write failed"); + } + return true; } } return false; @@ -1191,12 +1211,13 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) batch.WriteTx(wtx); NotifyTransactionChanged(wtx.GetHash(), CT_UPDATED); // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too - TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(now, 0)); - while (iter != mapTxSpends.end() && iter->first.hash == now) { - if (!done.count(iter->second)) { - todo.insert(iter->second); + 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) { + if (!done.count(iter->second)) { + todo.insert(iter->second); + } } - iter++; } // If a transaction changes 'conflicted' state, that changes the balance // available of the outputs it spends. So force those to be recomputed @@ -1242,12 +1263,13 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c wtx.MarkDirty(); batch.WriteTx(wtx); // Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too - TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(now, 0)); - while (iter != mapTxSpends.end() && iter->first.hash == now) { - if (!done.count(iter->second)) { - todo.insert(iter->second); - } - iter++; + 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) { + if (!done.count(iter->second)) { + todo.insert(iter->second); + } + } } // If a transaction changes 'conflicted' state, that changes the balance // available of the outputs it spends. So force those to be recomputed @@ -1364,7 +1386,7 @@ CAmount CWallet::GetDebit(const CTxIn &txin, const isminefilter& filter) const { { LOCK(cs_wallet); - std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(txin.prevout.hash); + const auto mi = mapWallet.find(txin.prevout.hash); if (mi != mapWallet.end()) { const CWalletTx& prev = (*mi).second; @@ -1407,6 +1429,19 @@ bool CWallet::IsMine(const CTransaction& tx) const return false; } +isminetype CWallet::IsMine(const COutPoint& outpoint) const +{ + AssertLockHeld(cs_wallet); + auto wtx = GetWalletTx(outpoint.hash); + if (!wtx) { + return ISMINE_NO; + } + if (outpoint.n >= wtx->tx->vout.size()) { + return ISMINE_NO; + } + return IsMine(wtx->tx->vout[outpoint.n]); +} + bool CWallet::IsFromMe(const CTransaction& tx) const { return (GetDebit(tx, ISMINE_ALL) > 0); @@ -1492,16 +1527,20 @@ bool CWallet::LoadWalletFlags(uint64_t flags) return true; } -bool CWallet::AddWalletFlags(uint64_t flags) +void CWallet::InitWalletFlags(uint64_t flags) { LOCK(cs_wallet); + // We should never be writing unknown non-tolerable wallet flags assert(((flags & KNOWN_WALLET_FLAGS) >> 32) == (flags >> 32)); + // This should only be used once, when creating a new wallet - so current flags are expected to be blank + assert(m_wallet_flags == 0); + if (!WalletBatch(GetDatabase()).WriteWalletFlags(flags)) { throw std::runtime_error(std::string(__func__) + ": writing wallet flags failed"); } - return LoadWalletFlags(flags); + if (!LoadWalletFlags(flags)) assert(false); } // Helper for producing a max-sized low-S low-R signature (eg 71 bytes) @@ -1818,34 +1857,6 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc return result; } -void CWallet::ReacceptWalletTransactions() -{ - // If transactions aren't being broadcasted, don't let them into local mempool either - if (!fBroadcastTransactions) - return; - std::map<int64_t, CWalletTx*> mapSorted; - - // Sort pending wallet transactions based on their initial wallet insertion order - for (std::pair<const uint256, CWalletTx>& item : mapWallet) { - const uint256& wtxid = item.first; - CWalletTx& wtx = item.second; - assert(wtx.GetHash() == wtxid); - - int nDepth = GetTxDepthInMainChain(wtx); - - if (!wtx.IsCoinBase() && (nDepth == 0 && !wtx.isAbandoned())) { - mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx)); - } - } - - // Try to add wallet transactions to memory pool - for (const std::pair<const int64_t, CWalletTx*>& item : mapSorted) { - CWalletTx& wtx = *(item.second); - std::string unused_err_string; - SubmitTxMemoryPoolAndRelay(wtx, unused_err_string, false); - } -} - bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const { AssertLockHeld(cs_wallet); @@ -1886,43 +1897,69 @@ std::set<uint256> CWallet::GetTxConflicts(const CWalletTx& wtx) const return result; } -// Rebroadcast transactions from the wallet. We do this on a random timer -// to slightly obfuscate which transactions come from our wallet. +// Resubmit transactions from the wallet to the mempool, optionally asking the +// mempool to relay them. On startup, we will do this for all unconfirmed +// transactions but will not ask the mempool to relay them. We do this on startup +// to ensure that our own mempool is aware of our transactions, and to also +// initialize nNextResend so that the actual rebroadcast is scheduled. There +// is a privacy side effect here as not broadcasting on startup also means that we won't +// inform the world of our wallet's state, particularly if the wallet (or node) is not +// yet synced. +// +// Otherwise this function is called periodically in order to relay our unconfirmed txs. +// We do this on a random timer to slightly obfuscate which transactions +// come from our wallet. // -// Ideally, we'd only resend transactions that we think should have been +// TODO: Ideally, we'd only resend transactions that we think should have been // mined in the most recent block. Any transaction that wasn't in the top // blockweight of transactions in the mempool shouldn't have been mined, // and so is probably just sitting in the mempool waiting to be confirmed. // Rebroadcasting does nothing to speed up confirmation and only damages // privacy. -void CWallet::ResendWalletTransactions() +// +// The `force` option results in all unconfirmed transactions being submitted to +// the mempool. This does not necessarily result in those transactions being relayed, +// that depends on the `relay` option. Periodic rebroadcast uses the pattern +// relay=true force=false, while loading into the mempool +// (on start, or after import) uses relay=false force=true. +void CWallet::ResubmitWalletTransactions(bool relay, bool force) { + // Don't attempt to resubmit if the wallet is configured to not broadcast, + // even if forcing. + if (!fBroadcastTransactions) return; + // During reindex, importing and IBD, old wallet transactions become // unconfirmed. Don't resend them as that would spam other nodes. - if (!chain().isReadyToBroadcast()) return; + // We only allow forcing mempool submission when not relaying to avoid this spam. + if (!force && relay && !chain().isReadyToBroadcast()) return; // Do this infrequently and randomly to avoid giving away // that these are our transactions. - if (GetTime() < nNextResend || !fBroadcastTransactions) return; - bool fFirst = (nNextResend == 0); + if (!force && GetTime() < nNextResend) return; // resend 12-36 hours from now, ~1 day on average. nNextResend = GetTime() + (12 * 60 * 60) + GetRand(24 * 60 * 60); - if (fFirst) return; int submitted_tx_count = 0; { // cs_wallet scope LOCK(cs_wallet); - // Relay transactions - for (std::pair<const uint256, CWalletTx>& item : mapWallet) { - CWalletTx& wtx = item.second; - // Attempt to rebroadcast all txes more than 5 minutes older than - // the last block. SubmitTxMemoryPoolAndRelay() will not rebroadcast - // any confirmed or conflicting txs. - if (wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; + // First filter for the transactions we want to rebroadcast. + // We use a set with WalletTxOrderComparator so that rebroadcasting occurs in insertion order + std::set<CWalletTx*, WalletTxOrderComparator> to_submit; + for (auto& [txid, wtx] : mapWallet) { + // Only rebroadcast unconfirmed txs + if (!wtx.isUnconfirmed()) continue; + + // attempt to rebroadcast all txes more than 5 minutes older than + // the last block, or all txs if forcing. + if (!force && wtx.nTimeReceived > m_best_block_time - 5 * 60) continue; + to_submit.insert(&wtx); + } + // Now try submitting the transactions to the memory pool and (optionally) relay them. + for (auto wtx : to_submit) { std::string unused_err_string; - if (SubmitTxMemoryPoolAndRelay(wtx, unused_err_string, true)) ++submitted_tx_count; + if (SubmitTxMemoryPoolAndRelay(*wtx, unused_err_string, relay)) ++submitted_tx_count; } } // cs_wallet @@ -1936,7 +1973,7 @@ void CWallet::ResendWalletTransactions() void MaybeResendWalletTxs(WalletContext& context) { for (const std::shared_ptr<CWallet>& pwallet : GetWallets(context)) { - pwallet->ResendWalletTransactions(); + pwallet->ResubmitWalletTransactions(/*relay=*/true, /*force=*/false); } } @@ -1953,7 +1990,7 @@ bool CWallet::SignTransaction(CMutableTransaction& tx) const // Build coins map std::map<COutPoint, Coin> coins; for (auto& input : tx.vin) { - std::map<uint256, CWalletTx>::const_iterator mi = mapWallet.find(input.prevout.hash); + const auto mi = mapWallet.find(input.prevout.hash); if(mi == mapWallet.end() || input.prevout.n >= mi->second.tx->vout.size()) { return false; } @@ -2324,36 +2361,32 @@ bool CWallet::TopUpKeyPool(unsigned int kpSize) return res; } -BResult<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label) +util::Result<CTxDestination> CWallet::GetNewDestination(const OutputType type, const std::string label) { LOCK(cs_wallet); auto spk_man = GetScriptPubKeyMan(type, false /* internal */); if (!spk_man) { - return strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); + return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))}; } spk_man->TopUp(); auto op_dest = spk_man->GetNewDestination(type); if (op_dest) { - SetAddressBook(op_dest.GetObj(), label, "receive"); + SetAddressBook(*op_dest, label, "receive"); } return op_dest; } -BResult<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type) +util::Result<CTxDestination> CWallet::GetNewChangeDestination(const OutputType type) { LOCK(cs_wallet); - CTxDestination dest; - bilingual_str error; ReserveDestination reservedest(this, type); - if (!reservedest.GetReservedDestination(dest, true, error)) { - return error; - } + auto op_dest = reservedest.GetReservedDestination(true); + if (op_dest) reservedest.KeepDestination(); - reservedest.KeepDestination(); - return dest; + return op_dest; } std::optional<int64_t> CWallet::GetOldestKeyPoolTime() const @@ -2423,27 +2456,24 @@ std::set<std::string> CWallet::ListAddrBookLabels(const std::string& purpose) co return label_set; } -bool ReserveDestination::GetReservedDestination(CTxDestination& dest, bool internal, bilingual_str& error) +util::Result<CTxDestination> ReserveDestination::GetReservedDestination(bool internal) { m_spk_man = pwallet->GetScriptPubKeyMan(type, internal); if (!m_spk_man) { - error = strprintf(_("Error: No %s addresses available."), FormatOutputType(type)); - return false; + return util::Error{strprintf(_("Error: No %s addresses available."), FormatOutputType(type))}; } - if (nIndex == -1) { m_spk_man->TopUp(); CKeyPool keypool; - if (!m_spk_man->GetReservedDestination(type, internal, address, nIndex, keypool, error)) { - return false; - } + auto op_address = m_spk_man->GetReservedDestination(type, internal, nIndex, keypool); + if (!op_address) return op_address; + address = *op_address; fInternal = keypool.fInternal; } - dest = address; - return true; + return address; } void ReserveDestination::KeepDestination() @@ -2789,8 +2819,12 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri warnings.push_back(strprintf(_("Error reading %s! Transaction data may be missing or incorrect." " Rescanning wallet."), walletFile)); rescan_required = true; - } - else { + } else if (nLoadWalletRet == DBErrors::UNKNOWN_DESCRIPTOR) { + error = strprintf(_("Unrecognized descriptor found. Loading wallet %s\n\n" + "The wallet might had been created on a newer version.\n" + "Please try running the latest software version.\n"), walletFile); + return nullptr; + } else { error = strprintf(_("Error loading %s"), walletFile); return nullptr; } @@ -2805,7 +2839,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri // ensure this wallet.dat can only be opened by clients supporting HD with chain split and expects no default key walletInstance->SetMinVersion(FEATURE_LATEST); - walletInstance->AddWalletFlags(wallet_creation_flags); + walletInstance->InitWalletFlags(wallet_creation_flags); // Only create LegacyScriptPubKeyMan when not descriptor wallet if (!walletInstance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { @@ -3159,7 +3193,7 @@ void CWallet::postInitProcess() // Add wallet transactions that aren't already in a block to mempool // Do this here as mempool requires genesis block to be loaded - ReacceptWalletTransactions(); + ResubmitWalletTransactions(/*relay=*/false, /*force=*/true); // Update wallet transactions with current mempool transactions. chain().requestMempoolTransactions(*this); @@ -3329,6 +3363,18 @@ std::unique_ptr<SigningProvider> CWallet::GetSolvingProvider(const CScript& scri return nullptr; } +std::vector<WalletDescriptor> CWallet::GetWalletDescriptors(const CScript& script) const +{ + std::vector<WalletDescriptor> descs; + for (const auto spk_man: GetScriptPubKeyMans(script)) { + if (const auto desc_spk_man = dynamic_cast<DescriptorScriptPubKeyMan*>(spk_man)) { + LOCK(desc_spk_man->cs_desc_man); + descs.push_back(desc_spk_man->GetWalletDescriptor()); + } + } + return descs; +} + LegacyScriptPubKeyMan* CWallet::GetLegacyScriptPubKeyMan() const { if (IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { @@ -3390,6 +3436,29 @@ void CWallet::LoadDescriptorScriptPubKeyMan(uint256 id, WalletDescriptor& desc) } } +void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) +{ + AssertLockHeld(cs_wallet); + + for (bool internal : {false, true}) { + for (OutputType t : OUTPUT_TYPES) { + auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this)); + if (IsCrypted()) { + if (IsLocked()) { + throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); + } + if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, nullptr)) { + throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors"); + } + } + spk_manager->SetupDescriptorGeneration(master_key, t, internal); + uint256 id = spk_manager->GetID(); + m_spk_managers[id] = std::move(spk_manager); + AddActiveScriptPubKeyMan(id, t, internal); + } + } +} + void CWallet::SetupDescriptorScriptPubKeyMans() { AssertLockHeld(cs_wallet); @@ -3405,23 +3474,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() CExtKey master_key; master_key.SetSeed(seed_key); - for (bool internal : {false, true}) { - for (OutputType t : OUTPUT_TYPES) { - auto spk_manager = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this)); - if (IsCrypted()) { - if (IsLocked()) { - throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); - } - if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, nullptr)) { - throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors"); - } - } - spk_manager->SetupDescriptorGeneration(master_key, t, internal); - uint256 id = spk_manager->GetID(); - m_spk_managers[id] = std::move(spk_manager); - AddActiveScriptPubKeyMan(id, t, internal); - } - } + SetupDescriptorScriptPubKeyMans(master_key); } else { ExternalSigner signer = ExternalSignerScriptPubKeyMan::GetExternalSigner(); @@ -3434,7 +3487,7 @@ void CWallet::SetupDescriptorScriptPubKeyMans() const UniValue& descriptor_vals = find_value(signer_res, internal ? "internal" : "receive"); if (!descriptor_vals.isArray()) throw std::runtime_error(std::string(__func__) + ": Unexpected result"); for (const UniValue& desc_val : descriptor_vals.get_array().getValues()) { - std::string desc_str = desc_val.getValStr(); + const std::string& desc_str = desc_val.getValStr(); FlatSigningProvider keys; std::string desc_error; std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, desc_error, false); @@ -3470,7 +3523,7 @@ void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool intern // Legacy wallets have only one ScriptPubKeyManager and it's active for all output and change types. Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); - WalletLogPrintf("Setting spkMan to active: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(type), static_cast<int>(internal)); + WalletLogPrintf("Setting spkMan to active: id = %s, type = %s, internal = %s\n", id.ToString(), FormatOutputType(type), internal ? "true" : "false"); auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers; auto& spk_mans_other = internal ? m_external_spk_managers : m_internal_spk_managers; auto spk_man = m_spk_managers.at(id).get(); @@ -3488,7 +3541,7 @@ void CWallet::DeactivateScriptPubKeyMan(uint256 id, OutputType type, bool intern { auto spk_man = GetScriptPubKeyMan(type, internal); if (spk_man != nullptr && spk_man->GetID() == id) { - WalletLogPrintf("Deactivate spkMan: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(type), static_cast<int>(internal)); + WalletLogPrintf("Deactivate spkMan: id = %s, type = %s, internal = %s\n", id.ToString(), FormatOutputType(type), internal ? "true" : "false"); WalletBatch batch(GetDatabase()); if (!batch.EraseActiveScriptPubKeyMan(static_cast<uint8_t>(type), internal)) { throw std::runtime_error(std::string(__func__) + ": erasing active ScriptPubKeyMan id failed"); @@ -3589,9 +3642,13 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat return nullptr; } - CTxDestination dest; - if (!internal && ExtractDestination(script_pub_keys.at(0), dest)) { - SetAddressBook(dest, label, "receive"); + if (!internal) { + for (const auto& script : script_pub_keys) { + CTxDestination dest; + if (ExtractDestination(script, dest)) { + SetAddressBook(dest, label, "receive"); + } + } } } @@ -3600,4 +3657,472 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat return spk_man; } + +bool CWallet::MigrateToSQLite(bilingual_str& error) +{ + AssertLockHeld(cs_wallet); + + WalletLogPrintf("Migrating wallet storage database from BerkeleyDB to SQLite.\n"); + + if (m_database->Format() == "sqlite") { + error = _("Error: This wallet already uses SQLite"); + return false; + } + + // Get all of the records for DB type migration + std::unique_ptr<DatabaseBatch> batch = m_database->MakeBatch(); + std::vector<std::pair<SerializeData, SerializeData>> records; + if (!batch->StartCursor()) { + error = _("Error: Unable to begin reading all records in the database"); + return false; + } + bool complete = false; + 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) { + break; + } + SerializeData key(ss_key.begin(), ss_key.end()); + SerializeData value(ss_value.begin(), ss_value.end()); + records.emplace_back(key, value); + } + batch->CloseCursor(); + batch.reset(); + if (!complete) { + error = _("Error: Unable to read all records in the database"); + return false; + } + + // Close this database and delete the file + fs::path db_path = fs::PathFromString(m_database->Filename()); + fs::path db_dir = db_path.parent_path(); + m_database->Close(); + fs::remove(db_path); + + // Make new DB + DatabaseOptions opts; + opts.require_create = true; + opts.require_format = DatabaseFormat::SQLITE; + DatabaseStatus db_status; + std::unique_ptr<WalletDatabase> new_db = MakeDatabase(db_dir, opts, db_status, error); + assert(new_db); // This is to prevent doing anything further with this wallet. The original file was deleted, but a backup exists. + m_database.reset(); + m_database = std::move(new_db); + + // Write existing records into the new DB + batch = m_database->MakeBatch(); + 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); + if (!batch->Write(ss_key, ss_value)) { + batch->TxnAbort(); + m_database->Close(); + fs::remove(m_database->Filename()); + assert(false); // 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. + } + } + bool committed = batch->TxnCommit(); + assert(committed); // 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. + return true; +} + +std::optional<MigrationData> CWallet::GetDescriptorsForLegacy(bilingual_str& error) const +{ + AssertLockHeld(cs_wallet); + + LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); + if (!legacy_spkm) { + error = _("Error: This wallet is already a descriptor wallet"); + return std::nullopt; + } + + 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"); + return std::nullopt; + } + return res; +} + +bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error) +{ + AssertLockHeld(cs_wallet); + + LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); + if (!legacy_spkm) { + error = _("Error: This wallet is already a descriptor wallet"); + return false; + } + + for (auto& desc_spkm : data.desc_spkms) { + if (m_spk_managers.count(desc_spkm->GetID()) > 0) { + error = _("Error: Duplicate descriptors created during migration. Your wallet may be corrupted."); + return false; + } + m_spk_managers[desc_spkm->GetID()] = std::move(desc_spkm); + } + + // Remove the LegacyScriptPubKeyMan from disk + if (!legacy_spkm->DeleteRecords()) { + return false; + } + + // Remove the LegacyScriptPubKeyMan from memory + m_spk_managers.erase(legacy_spkm->GetID()); + m_external_spk_managers.clear(); + m_internal_spk_managers.clear(); + + // Setup new descriptors + SetWalletFlag(WALLET_FLAG_DESCRIPTORS); + if (!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + // Use the existing master key if we have it + if (data.master_key.key.IsValid()) { + SetupDescriptorScriptPubKeyMans(data.master_key); + } else { + // Setup with a new seed if we don't. + SetupDescriptorScriptPubKeyMans(); + } + } + + // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet. + // We need to go through these in the tx insertion order so that lookups to spends works. + std::vector<uint256> txids_to_delete; + for (const auto& [_pos, wtx] : wtxOrdered) { + if (!IsMine(*wtx->tx) && !IsFromMe(*wtx->tx)) { + // Check it is the watchonly wallet's + // solvable_wallet doesn't need to be checked because transactions for those scripts weren't being watched for + if (data.watchonly_wallet) { + LOCK(data.watchonly_wallet->cs_wallet); + if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) { + // Add to watchonly wallet + if (!data.watchonly_wallet->AddToWallet(wtx->tx, wtx->m_state)) { + error = _("Error: Could not add watchonly tx to watchonly wallet"); + return false; + } + // Mark as to remove from this wallet + txids_to_delete.push_back(wtx->GetHash()); + continue; + } + } + // Both not ours and not in the watchonly wallet + error = strprintf(_("Error: Transaction %s in wallet cannot be identified to belong to migrated wallets"), wtx->GetHash().GetHex()); + return false; + } + } + // Do the removes + if (txids_to_delete.size() > 0) { + std::vector<uint256> deleted_txids; + if (ZapSelectTx(txids_to_delete, deleted_txids) != DBErrors::LOAD_OK) { + error = _("Error: Could not delete watchonly transactions"); + return false; + } + if (deleted_txids != txids_to_delete) { + error = _("Error: Not all watchonly txs could be deleted"); + return false; + } + // Tell the GUI of each tx + for (const uint256& txid : deleted_txids) { + NotifyTransactionChanged(txid, CT_UPDATED); + } + } + + // Check the address book data in the same way we did for transactions + std::vector<CTxDestination> dests_to_delete; + for (const auto& addr_pair : m_address_book) { + // Labels applied to receiving addresses should go based on IsMine + if (addr_pair.second.purpose == "receive") { + if (!IsMine(addr_pair.first)) { + // Check the address book data is the watchonly wallet's + if (data.watchonly_wallet) { + LOCK(data.watchonly_wallet->cs_wallet); + if (data.watchonly_wallet->IsMine(addr_pair.first)) { + // Add to the watchonly. Preserve the labels, purpose, and change-ness + std::string label = addr_pair.second.GetLabel(); + std::string purpose = addr_pair.second.purpose; + if (!purpose.empty()) { + data.watchonly_wallet->m_address_book[addr_pair.first].purpose = purpose; + } + if (!addr_pair.second.IsChange()) { + data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label); + } + dests_to_delete.push_back(addr_pair.first); + continue; + } + } + if (data.solvable_wallet) { + LOCK(data.solvable_wallet->cs_wallet); + if (data.solvable_wallet->IsMine(addr_pair.first)) { + // Add to the solvable. Preserve the labels, purpose, and change-ness + std::string label = addr_pair.second.GetLabel(); + std::string purpose = addr_pair.second.purpose; + if (!purpose.empty()) { + data.solvable_wallet->m_address_book[addr_pair.first].purpose = purpose; + } + if (!addr_pair.second.IsChange()) { + data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label); + } + dests_to_delete.push_back(addr_pair.first); + continue; + } + } + // Not ours, not in watchonly wallet, and not in solvable + error = _("Error: Address book data in wallet cannot be identified to belong to migrated wallets"); + return false; + } + } else { + // Labels for everything else (send) should be cloned to all + if (data.watchonly_wallet) { + LOCK(data.watchonly_wallet->cs_wallet); + // Add to the watchonly. Preserve the labels, purpose, and change-ness + std::string label = addr_pair.second.GetLabel(); + std::string purpose = addr_pair.second.purpose; + if (!purpose.empty()) { + data.watchonly_wallet->m_address_book[addr_pair.first].purpose = purpose; + } + if (!addr_pair.second.IsChange()) { + data.watchonly_wallet->m_address_book[addr_pair.first].SetLabel(label); + } + continue; + } + if (data.solvable_wallet) { + LOCK(data.solvable_wallet->cs_wallet); + // Add to the solvable. Preserve the labels, purpose, and change-ness + std::string label = addr_pair.second.GetLabel(); + std::string purpose = addr_pair.second.purpose; + if (!purpose.empty()) { + data.solvable_wallet->m_address_book[addr_pair.first].purpose = purpose; + } + if (!addr_pair.second.IsChange()) { + data.solvable_wallet->m_address_book[addr_pair.first].SetLabel(label); + } + continue; + } + } + } + // Remove the things to delete + if (dests_to_delete.size() > 0) { + for (const auto& dest : dests_to_delete) { + if (!DelAddressBook(dest)) { + error = _("Error: Unable to remove watchonly address book data"); + return false; + } + } + } + + // Connect the SPKM signals + ConnectScriptPubKeyManNotifiers(); + NotifyCanGetAddressesChanged(); + + WalletLogPrintf("Wallet migration complete.\n"); + + return true; +} + +bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, MigrationResult& res) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet) +{ + AssertLockHeld(wallet.cs_wallet); + + // Get all of the descriptors from the legacy wallet + std::optional<MigrationData> data = wallet.GetDescriptorsForLegacy(error); + if (data == std::nullopt) return false; + + // Create the watchonly and solvable wallets if necessary + if (data->watch_descs.size() > 0 || data->solvable_descs.size() > 0) { + DatabaseOptions options; + options.require_existing = false; + options.require_create = true; + + // Make the wallets + options.create_flags = WALLET_FLAG_DISABLE_PRIVATE_KEYS | WALLET_FLAG_BLANK_WALLET | WALLET_FLAG_DESCRIPTORS; + if (wallet.IsWalletFlagSet(WALLET_FLAG_AVOID_REUSE)) { + options.create_flags |= WALLET_FLAG_AVOID_REUSE; + } + if (wallet.IsWalletFlagSet(WALLET_FLAG_KEY_ORIGIN_METADATA)) { + options.create_flags |= WALLET_FLAG_KEY_ORIGIN_METADATA; + } + if (data->watch_descs.size() > 0) { + wallet.WalletLogPrintf("Making a new watchonly wallet containing the watched scripts\n"); + + DatabaseStatus status; + std::vector<bilingual_str> warnings; + std::string wallet_name = wallet.GetName() + "_watchonly"; + data->watchonly_wallet = CreateWallet(context, wallet_name, std::nullopt, options, status, error, warnings); + if (status != DatabaseStatus::SUCCESS) { + error = _("Error: Failed to create new watchonly wallet"); + return false; + } + res.watchonly_wallet = data->watchonly_wallet; + LOCK(data->watchonly_wallet->cs_wallet); + + // Parse the descriptors and add them to the new wallet + for (const auto& [desc_str, creation_time] : data->watch_descs) { + // Parse the descriptor + FlatSigningProvider keys; + std::string parse_err; + std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, parse_err, /* require_checksum */ true); + assert(desc); // It shouldn't be possible to have the LegacyScriptPubKeyMan make an invalid descriptor + assert(!desc->IsRange()); // It shouldn't be possible to have LegacyScriptPubKeyMan make a ranged watchonly descriptor + + // Add to the wallet + WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0); + data->watchonly_wallet->AddWalletDescriptor(w_desc, keys, "", false); + } + + // Add the wallet to settings + UpdateWalletSetting(*context.chain, wallet_name, /*load_on_startup=*/true, warnings); + } + if (data->solvable_descs.size() > 0) { + wallet.WalletLogPrintf("Making a new watchonly wallet containing the unwatched solvable scripts\n"); + + DatabaseStatus status; + std::vector<bilingual_str> warnings; + std::string wallet_name = wallet.GetName() + "_solvables"; + data->solvable_wallet = CreateWallet(context, wallet_name, std::nullopt, options, status, error, warnings); + if (status != DatabaseStatus::SUCCESS) { + error = _("Error: Failed to create new watchonly wallet"); + return false; + } + res.solvables_wallet = data->solvable_wallet; + LOCK(data->solvable_wallet->cs_wallet); + + // Parse the descriptors and add them to the new wallet + for (const auto& [desc_str, creation_time] : data->solvable_descs) { + // Parse the descriptor + FlatSigningProvider keys; + std::string parse_err; + std::unique_ptr<Descriptor> desc = Parse(desc_str, keys, parse_err, /* require_checksum */ true); + assert(desc); // It shouldn't be possible to have the LegacyScriptPubKeyMan make an invalid descriptor + assert(!desc->IsRange()); // It shouldn't be possible to have LegacyScriptPubKeyMan make a ranged watchonly descriptor + + // Add to the wallet + WalletDescriptor w_desc(std::move(desc), creation_time, 0, 0, 0); + data->solvable_wallet->AddWalletDescriptor(w_desc, keys, "", false); + } + + // Add the wallet to settings + UpdateWalletSetting(*context.chain, wallet_name, /*load_on_startup=*/true, warnings); + } + } + + // Add the descriptors to wallet, remove LegacyScriptPubKeyMan, and cleanup txs and address book data + if (!wallet.ApplyMigrationData(*data, error)) { + return false; + } + return true; +} + +util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>&& wallet, 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")}; + } + 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 + WalletContext empty_context; + empty_context.args = context.args; + DatabaseOptions options; + options.require_existing = true; + DatabaseStatus status; + std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(wallet_name, options, status, error); + if (!database) { + return util::Error{Untranslated("Wallet file verification failed.") + Untranslated(" ") + error}; + } + + 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}; + } + + bool success = false; + { + LOCK(local_wallet->cs_wallet); + + // First change to using SQLite + if (!local_wallet->MigrateToSQLite(error)) return util::Error{error}; + + // Do the migration, and cleanup if it fails + success = DoMigration(*local_wallet, context, error, res); + } + + if (success) { + // Migration successful, unload the wallet locally, then reload it. + assert(local_wallet.use_count() == 1); + local_wallet.reset(); + LoadWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error, warnings); + res.wallet_name = wallet_name; + } else { + // Migration failed, cleanup + // Copy the backup to the actual wallet dir + fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename); + fs::copy_file(backup_path, temp_backup_location, fs::copy_options::none); + + // Remember this wallet's walletdir to remove after unloading + std::vector<fs::path> wallet_dirs; + wallet_dirs.push_back(fs::PathFromString(local_wallet->GetDatabase().Filename()).parent_path()); + + // Unload the wallet locally + assert(local_wallet.use_count() == 1); + local_wallet.reset(); + + // Make list of wallets to cleanup + std::vector<std::shared_ptr<CWallet>> created_wallets; + created_wallets.push_back(std::move(res.watchonly_wallet)); + created_wallets.push_back(std::move(res.solvables_wallet)); + + // Get the directories to remove after unloading + for (std::shared_ptr<CWallet>& w : created_wallets) { + wallet_dirs.push_back(fs::PathFromString(w->GetDatabase().Filename()).parent_path()); + } + + // Unload the wallets + for (std::shared_ptr<CWallet>& w : created_wallets) { + if (!RemoveWallet(context, w, /*load_on_start=*/false)) { + error += _("\nUnable to cleanup failed migration"); + return util::Error{error}; + } + UnloadWallet(std::move(w)); + } + + // Delete the wallet directories + for (fs::path& dir : wallet_dirs) { + fs::remove_all(dir); + } + + // Restore the backup + DatabaseStatus status; + std::vector<bilingual_str> warnings; + if (!RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, error, warnings)) { + error += _("\nUnable to restore backup of wallet."); + return util::Error{error}; + } + + // Move the backup to the wallet dir + fs::copy_file(temp_backup_location, backup_path, fs::copy_options::none); + fs::remove(temp_backup_location); + + return util::Error{error}; + } + return res; +} } // namespace wallet diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 15f5917040..f4ea5f1920 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -14,6 +14,7 @@ #include <policy/feerate.h> #include <psbt.h> #include <tinyformat.h> +#include <util/hasher.h> #include <util/message.h> #include <util/result.h> #include <util/strencodings.h> @@ -37,6 +38,7 @@ #include <stdint.h> #include <string> #include <utility> +#include <unordered_map> #include <vector> #include <boost/signals2/signal.hpp> @@ -62,6 +64,7 @@ bool AddWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet); bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start, std::vector<bilingual_str>& warnings); bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet, std::optional<bool> load_on_start); std::vector<std::shared_ptr<CWallet>> GetWallets(WalletContext& context); +std::shared_ptr<CWallet> GetDefaultWallet(WalletContext& context, size_t& count); std::shared_ptr<CWallet> GetWallet(WalletContext& context, const std::string& name); std::shared_ptr<CWallet> LoadWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); @@ -90,7 +93,7 @@ static const CAmount DEFAULT_CONSOLIDATE_FEERATE{10000}; // 10 sat/vbyte static const CAmount DEFAULT_MAX_AVOIDPARTIALSPEND_FEE = 0; //! discourage APS fee higher than this amount constexpr CAmount HIGH_APS_FEE{COIN / 10000}; -//! minimum recommended increment for BIP 125 replacement txs +//! minimum recommended increment for replacement txs static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; @@ -99,7 +102,7 @@ static const bool DEFAULT_WALLET_REJECT_LONG_CHAINS{true}; //! -txconfirmtarget default static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6; //! -walletrbf default -static const bool DEFAULT_WALLET_RBF = false; +static const bool DEFAULT_WALLET_RBF = true; static const bool DEFAULT_WALLETBROADCAST = true; static const bool DEFAULT_DISABLE_WALLET = false; static const bool DEFAULT_WALLETCROSSCHAIN = false; @@ -189,7 +192,7 @@ public: } //! Reserve an address - bool GetReservedDestination(CTxDestination& pubkey, bool internal, bilingual_str& error); + util::Result<CTxDestination> GetReservedDestination(bool internal); //! Return reserved address void ReturnDestination(); //! Keep the address. Do not return it's key to the keypool when this object goes out of scope @@ -259,7 +262,7 @@ private: * detect and report conflicts (double-spends or * mutated transactions where the mutant gets mined). */ - typedef std::multimap<COutPoint, uint256> TxSpends; + typedef std::unordered_multimap<COutPoint, uint256, SaltedOutpointHasher> TxSpends; TxSpends mapTxSpends GUARDED_BY(cs_wallet); void AddToSpends(const COutPoint& outpoint, const uint256& wtxid, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void AddToSpends(const CWalletTx& wtx, WalletBatch* batch = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -313,7 +316,7 @@ private: std::string m_name; /** Internal database handle. */ - std::unique_ptr<WalletDatabase> const m_database; + std::unique_ptr<WalletDatabase> m_database; /** * The following is used to keep track of how far behind the wallet is @@ -390,7 +393,7 @@ public: /** Map from txid to CWalletTx for all transactions this wallet is * interested in, including received and sent transactions. */ - std::map<uint256, CWalletTx> mapWallet GUARDED_BY(cs_wallet); + std::unordered_map<uint256, CWalletTx, SaltedTxidHasher> mapWallet GUARDED_BY(cs_wallet); typedef std::multimap<int64_t, CWalletTx*> TxItems; TxItems wtxOrdered; @@ -505,6 +508,10 @@ public: //! @return true if wtx is changed and needs to be saved to disk, otherwise false using UpdateWalletTxFn = std::function<bool(CWalletTx& wtx, bool new_tx)>; + /** + * Add the transaction to the wallet, wrapping it up inside a CWalletTx + * @return the recently added wtx pointer or nullptr if there was a db write error. + */ 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; @@ -530,8 +537,7 @@ public: }; 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 ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - void ResendWalletTransactions(); + void ResubmitWalletTransactions(bool relay, bool force); OutputType TransactionChangeType(const std::optional<OutputType>& change_type, const std::vector<CRecipient>& vecSend) const; @@ -666,8 +672,8 @@ public: */ void MarkDestinationsDirty(const std::set<CTxDestination>& destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - BResult<CTxDestination> GetNewDestination(const OutputType type, const std::string label); - BResult<CTxDestination> GetNewChangeDestination(const OutputType type); + util::Result<CTxDestination> GetNewDestination(const OutputType type, const std::string label); + util::Result<CTxDestination> GetNewChangeDestination(const OutputType type); isminetype IsMine(const CTxDestination& dest) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); isminetype IsMine(const CScript& script) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -678,6 +684,7 @@ public: CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; isminetype IsMine(const CTxOut& txout) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool IsMine(const CTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + isminetype IsMine(const COutPoint& outpoint) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /** should probably be renamed to IsRelevantToMe */ bool IsFromMe(const CTransaction& tx) const; CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; @@ -708,7 +715,7 @@ public: std::set<uint256> GetConflicts(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Check if a given transaction has any of its outputs spent by another transaction in the wallet - bool HasWalletSpend(const uint256& txid) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool HasWalletSpend(const CTransactionRef& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Flush wallet (bitdb flush) void Flush(); @@ -760,7 +767,7 @@ public: /* Mark a transaction (and it in-wallet descendants) as abandoned so its inputs may be respent. */ bool AbandonTransaction(const uint256& hashTx); - /** Mark a transaction as replaced by another transaction (e.g., BIP 125). */ + /** Mark a transaction as replaced by another transaction. */ bool MarkReplaced(const uint256& originalHash, const uint256& newHash); /* Initializes the wallet, returns a new CWallet instance or a null pointer in case of an error */ @@ -798,8 +805,9 @@ public: bool IsWalletFlagSet(uint64_t flag) const override; /** overwrite all flags by the given uint64_t - returns false if unknown, non-tolerable flags are present */ - bool AddWalletFlags(uint64_t flags); + flags must be uninitialised (or 0) + only known flags may be present */ + void InitWalletFlags(uint64_t flags); /** Loads the flags into the wallet. (used by LoadWallet) */ bool LoadWalletFlags(uint64_t flags); @@ -839,6 +847,9 @@ public: std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script) const; std::unique_ptr<SigningProvider> GetSolvingProvider(const CScript& script, SignatureData& sigdata) const; + //! Get the wallet descriptors for a script. + std::vector<WalletDescriptor> GetWalletDescriptors(const CScript& script) const; + //! Get the LegacyScriptPubKeyMan which is used for all types, internal, and external. LegacyScriptPubKeyMan* GetLegacyScriptPubKeyMan() const; LegacyScriptPubKeyMan* GetOrCreateLegacyScriptPubKeyMan(); @@ -895,6 +906,7 @@ public: void DeactivateScriptPubKeyMan(uint256 id, OutputType type, bool internal); //! Create new DescriptorScriptPubKeyMans and add them to the wallet + void SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetupDescriptorScriptPubKeyMans() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); //! Return the DescriptorScriptPubKeyMan for a WalletDescriptor if it is already in the wallet @@ -907,6 +919,20 @@ public: //! Add a descriptor to the wallet, return a ScriptPubKeyMan & associated output type ScriptPubKeyMan* AddWalletDescriptor(WalletDescriptor& desc, const FlatSigningProvider& signing_provider, const std::string& label, bool internal) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + /** Move all records from the BDB database to a new SQLite database for storage. + * The original BDB file will be deleted and replaced with a new SQLite file. + * A backup is not created. + * May crash if something unexpected happens in the filesystem. + */ + bool MigrateToSQLite(bilingual_str& error) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + //! Get all of the descriptors from a legacy wallet + std::optional<MigrationData> GetDescriptorsForLegacy(bilingual_str& error) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + + //! 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); }; /** @@ -965,6 +991,16 @@ bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_nam bool DummySignInput(const SigningProvider& provider, CTxIn &tx_in, const CTxOut &txout, const CCoinControl* coin_control = nullptr); bool FillInputToWeight(CTxIn& txin, int64_t target_weight); + +struct MigrationResult { + std::string wallet_name; + std::shared_ptr<CWallet> watchonly_wallet; + std::shared_ptr<CWallet> solvables_wallet; + fs::path backup_path; +}; + +//! Do all steps to migrate a legacy wallet to a descriptor wallet +util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>&& wallet, WalletContext& context); } // namespace wallet #endif // BITCOIN_WALLET_WALLET_H diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 8afd3f416d..6a8f0d2481 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -59,6 +59,7 @@ const std::string WALLETDESCRIPTORCKEY{"walletdescriptorckey"}; const std::string WALLETDESCRIPTORKEY{"walletdescriptorkey"}; const std::string WATCHMETA{"watchmeta"}; const std::string WATCHS{"watchs"}; +const std::unordered_set<std::string> LEGACY_TYPES{CRYPTED_KEY, CSCRIPT, DEFAULTKEY, HDCHAIN, KEYMETA, KEY, OLD_KEY, POOL, WATCHMETA, WATCHS}; } // namespace DBKeys // @@ -313,6 +314,7 @@ public: std::map<std::pair<uint256, CKeyID>, std::pair<CPubKey, std::vector<unsigned char>>> m_descriptor_crypt_keys; std::map<uint160, CHDChain> m_hd_chains; bool tx_corrupt{false}; + bool descriptor_unknown{false}; CWalletScanState() = default; }; @@ -626,7 +628,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, uint256 id; ssKey >> id; WalletDescriptor desc; - ssValue >> desc; + try { + ssValue >> desc; + } catch (const std::ios_base::failure& e) { + strErr = e.what(); + wss.descriptor_unknown = true; + return false; + } if (wss.m_descriptor_caches.count(id) == 0) { wss.m_descriptor_caches[id] = DescriptorCache(); } @@ -766,6 +774,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) DBErrors result = DBErrors::LOAD_OK; LOCK(pwallet->cs_wallet); + + // Last client version to open this wallet + int last_client = CLIENT_VERSION; + bool has_last_client = m_batch->Read(DBKeys::VERSION, last_client); + pwallet->WalletLogPrintf("Wallet file version = %d, last client version = %d\n", pwallet->GetVersion(), last_client); + try { int nMinVersion = 0; if (m_batch->Read(DBKeys::MINVERSION, nMinVersion)) { @@ -831,6 +845,13 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) // Set tx_corrupt back to false so that the error is only printed once (per corrupt tx) wss.tx_corrupt = false; result = DBErrors::CORRUPT; + } else if (wss.descriptor_unknown) { + strErr = strprintf("Error: Unrecognized descriptor found in wallet %s. ", pwallet->GetName()); + strErr += (last_client > CLIENT_VERSION) ? "The wallet might had been created on a newer version. " : + "The database might be corrupted or the software version is not compatible with one of your wallet descriptors. "; + strErr += "Please try running the latest software version"; + pwallet->WalletLogPrintf("%s\n", strErr); + return DBErrors::UNKNOWN_DESCRIPTOR; } else { // Leave other errors alone, if we try to fix them we might make things worse. fNoncriticalErrors = true; // ... but do warn the user there is something wrong. @@ -856,18 +877,18 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) } // Set the descriptor caches - for (auto desc_cache_pair : wss.m_descriptor_caches) { + for (const auto& desc_cache_pair : wss.m_descriptor_caches) { auto spk_man = pwallet->GetScriptPubKeyMan(desc_cache_pair.first); assert(spk_man); ((DescriptorScriptPubKeyMan*)spk_man)->SetCache(desc_cache_pair.second); } // Set the descriptor keys - for (auto desc_key_pair : wss.m_descriptor_keys) { + for (const auto& desc_key_pair : wss.m_descriptor_keys) { auto spk_man = pwallet->GetScriptPubKeyMan(desc_key_pair.first.first); ((DescriptorScriptPubKeyMan*)spk_man)->AddKey(desc_key_pair.first.second, desc_key_pair.second); } - for (auto desc_key_pair : wss.m_descriptor_crypt_keys) { + for (const auto& desc_key_pair : wss.m_descriptor_crypt_keys) { auto spk_man = pwallet->GetScriptPubKeyMan(desc_key_pair.first.first); ((DescriptorScriptPubKeyMan*)spk_man)->AddCryptedKey(desc_key_pair.first.second, desc_key_pair.second.first, desc_key_pair.second.second); } @@ -883,11 +904,6 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet) if (result != DBErrors::LOAD_OK) return result; - // Last client version to open this wallet - int last_client = CLIENT_VERSION; - bool has_last_client = m_batch->Read(DBKeys::VERSION, last_client); - pwallet->WalletLogPrintf("Wallet file version = %d, last client version = %d\n", pwallet->GetVersion(), last_client); - pwallet->WalletLogPrintf("Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total. Unknown wallet records: %u\n", wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys, wss.m_unknown_records); @@ -1083,6 +1099,45 @@ bool WalletBatch::WriteWalletFlags(const uint64_t flags) return WriteIC(DBKeys::FLAGS, flags); } +bool WalletBatch::EraseRecords(const std::unordered_set<std::string>& types) +{ + // Get cursor + if (!m_batch->StartCursor()) + { + return false; + } + + // Iterate the DB and look for any records that have the type prefixes + 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) { + break; + } + else if (!ret) + { + m_batch->CloseCursor(); + return false; + } + + // Make a copy of key to avoid data being deleted by the following read of the type + Span<const unsigned char> key_data = MakeUCharSpan(key); + + std::string type; + key >> type; + + if (types.count(type) > 0) { + m_batch->Erase(key_data); + } + } + m_batch->CloseCursor(); + return true; +} + bool WalletBatch::TxnBegin() { return m_batch->TxnBegin(); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index a04ea598b6..da6efe534b 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -51,7 +51,8 @@ enum class DBErrors EXTERNAL_SIGNER_SUPPORT_REQUIRED, LOAD_FAIL, NEED_REWRITE, - NEED_RESCAN + NEED_RESCAN, + UNKNOWN_DESCRIPTOR }; namespace DBKeys { @@ -84,6 +85,9 @@ extern const std::string WALLETDESCRIPTORCKEY; extern const std::string WALLETDESCRIPTORKEY; extern const std::string WATCHMETA; extern const std::string WATCHS; + +// Keys in this set pertain only to the legacy wallet (LegacyScriptPubKeyMan) and are removed during migration from legacy to descriptors. +extern const std::unordered_set<std::string> LEGACY_TYPES; } // namespace DBKeys /* simple HD chain data model */ @@ -276,6 +280,9 @@ public: //! write the hdchain model (external chain child index counter) bool WriteHDChain(const CHDChain& chain); + //! Delete records of the given types + bool EraseRecords(const std::unordered_set<std::string>& types); + bool WriteWalletFlags(const uint64_t flags); //! Begin a new transaction bool TxnBegin(); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index 769175b5a8..e991bc0814 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -34,7 +34,7 @@ static void WalletCreate(CWallet* wallet_instance, uint64_t wallet_creation_flag LOCK(wallet_instance->cs_wallet); wallet_instance->SetMinVersion(FEATURE_LATEST); - wallet_instance->AddWalletFlags(wallet_creation_flags); + wallet_instance->InitWalletFlags(wallet_creation_flags); if (!wallet_instance->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { auto spk_man = wallet_instance->GetOrCreateLegacyScriptPubKeyMan(); diff --git a/src/wallet/walletutil.h b/src/wallet/walletutil.h index 788d41ceb7..8434d64fb5 100644 --- a/src/wallet/walletutil.h +++ b/src/wallet/walletutil.h @@ -104,6 +104,20 @@ public: WalletDescriptor() {} WalletDescriptor(std::shared_ptr<Descriptor> descriptor, uint64_t creation_time, int32_t range_start, int32_t range_end, int32_t next_index) : descriptor(descriptor), creation_time(creation_time), range_start(range_start), range_end(range_end), next_index(next_index) {} }; + +class CWallet; +class DescriptorScriptPubKeyMan; + +/** struct containing information needed for migrating legacy wallets to descriptor wallets */ +struct MigrationData +{ + CExtKey master_key; + std::vector<std::pair<std::string, int64_t>> watch_descs; + std::vector<std::pair<std::string, int64_t>> solvable_descs; + std::vector<std::unique_ptr<DescriptorScriptPubKeyMan>> desc_spkms; + std::shared_ptr<CWallet> watchonly_wallet{nullptr}; + std::shared_ptr<CWallet> solvable_wallet{nullptr}; +}; } // namespace wallet #endif // BITCOIN_WALLET_WALLETUTIL_H |