diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/bdb.cpp | 21 | ||||
-rw-r--r-- | src/wallet/bdb.h | 4 | ||||
-rw-r--r-- | src/wallet/coinselection.cpp | 66 | ||||
-rw-r--r-- | src/wallet/coinselection.h | 28 | ||||
-rw-r--r-- | src/wallet/init.cpp | 5 | ||||
-rw-r--r-- | src/wallet/ismine.h | 22 | ||||
-rw-r--r-- | src/wallet/load.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpcdump.cpp | 73 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 156 | ||||
-rw-r--r-- | src/wallet/scriptpubkeyman.cpp | 5 | ||||
-rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 102 | ||||
-rw-r--r-- | src/wallet/test/db_tests.cpp | 8 | ||||
-rw-r--r-- | src/wallet/test/init_test_fixture.cpp | 4 | ||||
-rw-r--r-- | src/wallet/test/init_tests.cpp | 8 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 167 | ||||
-rw-r--r-- | src/wallet/wallet.h | 16 | ||||
-rw-r--r-- | src/wallet/wallettool.cpp | 10 | ||||
-rw-r--r-- | src/wallet/wallettool.h | 2 |
18 files changed, 422 insertions, 277 deletions
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 6ed48593fb..ad40e6da9a 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -488,9 +488,9 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) break; } if (pszSkip && - strncmp(ssKey.data(), pszSkip, std::min(ssKey.size(), strlen(pszSkip))) == 0) + strncmp((const char*)ssKey.data(), pszSkip, std::min(ssKey.size(), strlen(pszSkip))) == 0) continue; - if (strncmp(ssKey.data(), "\x07version", 8) == 0) { + if (strncmp((const char*)ssKey.data(), "\x07version", 8) == 0) { // Update version: ssValue.clear(); ssValue << CLIENT_VERSION; @@ -723,6 +723,23 @@ bool BerkeleyBatch::TxnAbort() return (ret == 0); } +bool BerkeleyDatabaseSanityCheck() +{ + int major, minor; + DbEnv::version(&major, &minor, nullptr); + + /* If the major version differs, or the minor version of library is *older* + * than the header that was compiled against, flag an error. + */ + if (major != DB_VERSION_MAJOR || minor < DB_VERSION_MINOR) { + LogPrintf("BerkeleyDB database version conflict: header version is %d.%d, library version is %d.%d\n", + DB_VERSION_MAJOR, DB_VERSION_MINOR, major, minor); + return false; + } + + return true; +} + std::string BerkeleyDatabaseVersion() { return DbEnv::version(nullptr, nullptr, nullptr); diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index bf1617d67f..a8209587d7 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -223,6 +223,10 @@ public: std::string BerkeleyDatabaseVersion(); +/** Perform sanity check of runtime BDB version versus linked BDB version. + */ +bool BerkeleyDatabaseSanityCheck(); + //! Return object giving access to Berkeley database at specified path. std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index a2dea84d17..10f89e3a6f 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -300,8 +300,26 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group ******************************************************************************/ -void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) { +void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only) { + // Compute the effective value first + const CAmount coin_fee = output.m_input_bytes < 0 ? 0 : m_effective_feerate.GetFee(output.m_input_bytes); + const CAmount ev = output.txout.nValue - coin_fee; + + // Filter for positive only here before adding the coin + if (positive_only && ev <= 0) return; + m_outputs.push_back(output); + CInputCoin& coin = m_outputs.back(); + + coin.m_fee = coin_fee; + fee += coin.m_fee; + + coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : m_long_term_feerate.GetFee(coin.m_input_bytes); + long_term_fee += coin.m_long_term_fee; + + coin.effective_value = ev; + effective_value += coin.effective_value; + m_from_me &= from_me; m_value += output.txout.nValue; m_depth = std::min(m_depth, depth); @@ -312,20 +330,6 @@ void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size // descendants is the count as seen from the top ancestor, not the descendants as seen from the // coin itself; thus, this value is counted as the max, not the sum m_descendants = std::max(m_descendants, descendants); - effective_value += output.effective_value; - fee += output.m_fee; - long_term_fee += output.m_long_term_fee; -} - -std::vector<CInputCoin>::iterator OutputGroup::Discard(const CInputCoin& output) { - auto it = m_outputs.begin(); - while (it != m_outputs.end() && it->outpoint != output.outpoint) ++it; - if (it == m_outputs.end()) return it; - m_value -= output.txout.nValue; - effective_value -= output.effective_value; - fee -= output.m_fee; - long_term_fee -= output.m_long_term_fee; - return m_outputs.erase(it); } bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const @@ -334,35 +338,3 @@ bool OutputGroup::EligibleForSpending(const CoinEligibilityFilter& eligibility_f && m_ancestors <= eligibility_filter.max_ancestors && m_descendants <= eligibility_filter.max_descendants; } - -void OutputGroup::SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate) -{ - fee = 0; - long_term_fee = 0; - effective_value = 0; - for (CInputCoin& coin : m_outputs) { - coin.m_fee = coin.m_input_bytes < 0 ? 0 : effective_feerate.GetFee(coin.m_input_bytes); - fee += coin.m_fee; - - coin.m_long_term_fee = coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes); - long_term_fee += coin.m_long_term_fee; - - coin.effective_value = coin.txout.nValue - coin.m_fee; - effective_value += coin.effective_value; - } -} - -OutputGroup OutputGroup::GetPositiveOnlyGroup() -{ - OutputGroup group(*this); - for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) { - const CInputCoin& coin = *it; - // Only include outputs that are positive effective value (i.e. not dust) - if (coin.effective_value <= 0) { - it = group.Discard(coin); - } else { - ++it; - } - } - return group; -} diff --git a/src/wallet/coinselection.h b/src/wallet/coinselection.h index cf9768b927..f0e1addaf1 100644 --- a/src/wallet/coinselection.h +++ b/src/wallet/coinselection.h @@ -6,11 +6,10 @@ #define BITCOIN_WALLET_COINSELECTION_H #include <amount.h> +#include <policy/feerate.h> #include <primitives/transaction.h> #include <random.h> -class CFeeRate; - //! target minimum change amount static constexpr CAmount MIN_CHANGE{COIN / 100}; //! final minimum change amount after paying for fees @@ -63,9 +62,11 @@ struct CoinEligibilityFilter const int conf_theirs; const uint64_t max_ancestors; const uint64_t max_descendants; + const bool m_include_partial_groups{false}; //! Include partial destination groups when avoid_reuse and there are full groups CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_ancestors) {} CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants) {} + CoinEligibilityFilter(int conf_mine, int conf_theirs, uint64_t max_ancestors, uint64_t max_descendants, bool include_partial) : conf_mine(conf_mine), conf_theirs(conf_theirs), max_ancestors(max_ancestors), max_descendants(max_descendants), m_include_partial_groups(include_partial) {} }; struct OutputGroup @@ -78,27 +79,18 @@ struct OutputGroup size_t m_descendants{0}; CAmount effective_value{0}; CAmount fee{0}; + CFeeRate m_effective_feerate{0}; CAmount long_term_fee{0}; + CFeeRate m_long_term_feerate{0}; OutputGroup() {} - OutputGroup(std::vector<CInputCoin>&& outputs, bool from_me, CAmount value, int depth, size_t ancestors, size_t descendants) - : m_outputs(std::move(outputs)) - , m_from_me(from_me) - , m_value(value) - , m_depth(depth) - , m_ancestors(ancestors) - , m_descendants(descendants) + OutputGroup(const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate) : + m_effective_feerate(effective_feerate), + m_long_term_feerate(long_term_feerate) {} - OutputGroup(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants) : OutputGroup() { - Insert(output, depth, from_me, ancestors, descendants); - } - void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants); - std::vector<CInputCoin>::iterator Discard(const CInputCoin& output); - bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; - //! Update the OutputGroup's fee, long_term_fee, and effective_value based on the given feerates - void SetFees(const CFeeRate effective_feerate, const CFeeRate long_term_feerate); - OutputGroup GetPositiveOnlyGroup(); + void Insert(const CInputCoin& output, int depth, bool from_me, size_t ancestors, size_t descendants, bool positive_only); + bool EligibleForSpending(const CoinEligibilityFilter& eligibility_filter) const; }; bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& target_value, const CAmount& cost_of_change, std::set<CInputCoin>& out_set, CAmount& value_ret, CAmount not_input_fees); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 085dde1026..0d2be64dfb 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -86,6 +86,11 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const bool WalletInit::ParameterInteraction() const { +#ifdef USE_BDB + if (!BerkeleyDatabaseSanityCheck()) { + return InitError(Untranslated("A version conflict was detected between the run-time BerkeleyDB library and the one used during compilation.")); + } +#endif if (gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET)) { for (const std::string& wallet : gArgs.GetArgs("-wallet")) { LogPrintf("%s: parameter interaction: -disablewallet -> ignoring -wallet=%s\n", __func__, wallet); diff --git a/src/wallet/ismine.h b/src/wallet/ismine.h index 5cdd7dff80..38ed7e7770 100644 --- a/src/wallet/ismine.h +++ b/src/wallet/ismine.h @@ -14,7 +14,27 @@ class CWallet; class CScript; -/** IsMine() return codes */ +/** + * IsMine() return codes, which depend on ScriptPubKeyMan implementation. + * Not every ScriptPubKeyMan covers all types, please refer to + * https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.21.0.md#ismine-semantics + * for better understanding. + * + * For LegacyScriptPubKeyMan, + * ISMINE_NO: the scriptPubKey is not in the wallet; + * ISMINE_WATCH_ONLY: the scriptPubKey has been imported into the wallet; + * ISMINE_SPENDABLE: the scriptPubKey corresponds to an address owned by the wallet user (can spend with the private key); + * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user; + * ISMINE_ALL: all ISMINE flags except for USED; + * ISMINE_ALL_USED: all ISMINE flags including USED; + * ISMINE_ENUM_ELEMENTS: the number of isminetype enum elements. + * + * For DescriptorScriptPubKeyMan and future ScriptPubKeyMan, + * ISMINE_NO: the scriptPubKey is not in the wallet; + * ISMINE_SPENDABLE: the scriptPubKey matches a scriptPubKey in the wallet. + * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user. + * + */ enum isminetype : unsigned int { ISMINE_NO = 0, diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 036fd4956f..30832f983b 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -62,7 +62,7 @@ bool VerifyWallets(interfaces::Chain& chain) std::set<fs::path> wallet_paths; for (const auto& wallet_file : gArgs.GetArgs("-wallet")) { - const fs::path path = fs::absolute(wallet_file, GetWalletDir()); + const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), wallet_file); if (!wallet_paths.insert(path).second) { chain.initWarning(strprintf(_("Ignoring duplicate -wallet %s."), wallet_file)); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 6b46868d10..99803a91d2 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -934,9 +934,9 @@ static std::string RecurseImportData(const CScript& script, ImportData& import_d case TxoutType::NONSTANDARD: case TxoutType::WITNESS_UNKNOWN: case TxoutType::WITNESS_V1_TAPROOT: - default: return "unrecognized script"; - } + } // no default case, so the compiler can warn about missing cases + CHECK_NONFATAL(false); } static UniValue ProcessImportLegacy(ImportData& import_data, std::map<CKeyID, CPubKey>& pubkey_map, std::map<CKeyID, CKey>& privkey_map, std::set<CScript>& script_pub_keys, bool& have_solving_data, const UniValue& data, std::vector<CKeyID>& ordered_pubkeys) @@ -1740,3 +1740,72 @@ RPCHelpMan importdescriptors() }, }; } + +RPCHelpMan listdescriptors() +{ + return RPCHelpMan{ + "listdescriptors", + "\nList descriptors imported into a descriptor-enabled wallet.", + {}, + RPCResult{ + RPCResult::Type::ARR, "", "Response is an array of descriptor objects", + { + {RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "desc", "Descriptor string representation"}, + {RPCResult::Type::NUM, "timestamp", "The creation time of the descriptor"}, + {RPCResult::Type::BOOL, "active", "Activeness flag"}, + {RPCResult::Type::BOOL, "internal", true, "Whether this is internal or external descriptor; defined only for active descriptors"}, + {RPCResult::Type::ARR_FIXED, "range", true, "Defined only for ranged descriptors", { + {RPCResult::Type::NUM, "", "Range start inclusive"}, + {RPCResult::Type::NUM, "", "Range end inclusive"}, + }}, + {RPCResult::Type::NUM, "next", true, "The next index to generate addresses from; defined only for ranged descriptors"}, + }}, + } + }, + RPCExamples{ + HelpExampleCli("listdescriptors", "") + HelpExampleRpc("listdescriptors", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); + if (!wallet) return NullUniValue; + + if (!wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { + throw JSONRPCError(RPC_WALLET_ERROR, "listdescriptors is not available for non-descriptor wallets"); + } + + LOCK(wallet->cs_wallet); + + UniValue response(UniValue::VARR); + const auto active_spk_mans = wallet->GetActiveScriptPubKeyMans(); + 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(); + spk.pushKV("desc", wallet_descriptor.descriptor->ToString()); + spk.pushKV("timestamp", wallet_descriptor.creation_time); + const bool active = active_spk_mans.count(desc_spk_man) != 0; + spk.pushKV("active", active); + const auto& type = wallet_descriptor.descriptor->GetOutputType(); + if (active && type != nullopt) { + spk.pushKV("internal", wallet->GetScriptPubKeyMan(*type, true) == desc_spk_man); + } + if (wallet_descriptor.descriptor->IsRange()) { + UniValue range(UniValue::VARR); + range.push_back(wallet_descriptor.range_start); + range.push_back(wallet_descriptor.range_end - 1); + spk.pushKV("range", range); + spk.pushKV("next", wallet_descriptor.next_index); + } + response.push_back(spk); + } + + return response; +}, + }; +} diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 7705794c63..46de273d63 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2615,7 +2615,18 @@ static RPCHelpMan loadwallet() if (!wallet) { // Map bad format to not found, since bad format is returned when the // wallet directory exists, but doesn't contain a data file. - RPCErrorCode code = status == DatabaseStatus::FAILED_NOT_FOUND || status == DatabaseStatus::FAILED_BAD_FORMAT ? RPC_WALLET_NOT_FOUND : RPC_WALLET_ERROR; + RPCErrorCode code = RPC_WALLET_ERROR; + switch (status) { + case DatabaseStatus::FAILED_NOT_FOUND: + case DatabaseStatus::FAILED_BAD_FORMAT: + code = RPC_WALLET_NOT_FOUND; + break; + case DatabaseStatus::FAILED_ALREADY_LOADED: + code = RPC_WALLET_ALREADY_LOADED; + break; + default: // RPC_WALLET_ERROR is returned for all other cases. + break; + } throw JSONRPCError(code, error.original); } @@ -3449,10 +3460,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name) CWallet* const pwallet = wallet.get(); if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && !want_psbt) { - if (!pwallet->chain().rpcEnableDeprecated("bumpfee")) { - throw JSONRPCError(RPC_METHOD_DEPRECATED, "Using bumpfee with wallets that have private keys disabled is deprecated. Use psbtbumpfee instead or restart bitcoind with -deprecatedrpc=bumpfee. This functionality will be removed in 0.22"); - } - want_psbt = true; + throw JSONRPCError(RPC_WALLET_ERROR, "bumpfee is not available with wallets that have private keys disabled. Use psbtbumpfee instead."); } RPCTypeCheck(request.params, {UniValue::VSTR, UniValue::VOBJ}); @@ -3823,13 +3831,19 @@ RPCHelpMan getaddressinfo() LOCK(pwallet->cs_wallet); - UniValue ret(UniValue::VOBJ); - CTxDestination dest = DecodeDestination(request.params[0].get_str()); + std::string error_msg; + CTxDestination dest = DecodeDestination(request.params[0].get_str(), error_msg); + // Make sure the destination is valid if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); + // Set generic error message in case 'DecodeDestination' didn't set it + if (error_msg.empty()) error_msg = "Invalid address"; + + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, error_msg); } + UniValue ret(UniValue::VOBJ); + std::string currentAddress = EncodeDestination(dest); ret.pushKV("address", currentAddress); @@ -4537,73 +4551,75 @@ RPCHelpMan importprunedfunds(); RPCHelpMan removeprunedfunds(); RPCHelpMan importmulti(); RPCHelpMan importdescriptors(); +RPCHelpMan listdescriptors(); Span<const CRPCCommand> GetWalletRPCCommands() { // clang-format off static const CRPCCommand commands[] = -{ // category name actor (function) argNames - // --------------------- ------------------------ ----------------------- ---------- - { "rawtransactions", "fundrawtransaction", &fundrawtransaction, {"hexstring","options","iswitness"} }, - { "wallet", "abandontransaction", &abandontransaction, {"txid"} }, - { "wallet", "abortrescan", &abortrescan, {} }, - { "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","label","address_type"} }, - { "wallet", "backupwallet", &backupwallet, {"destination"} }, - { "wallet", "bumpfee", &bumpfee, {"txid", "options"} }, - { "wallet", "psbtbumpfee", &psbtbumpfee, {"txid", "options"} }, - { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "descriptors", "load_on_startup"} }, - { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, - { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, - { "wallet", "encryptwallet", &encryptwallet, {"passphrase"} }, - { "wallet", "getaddressesbylabel", &getaddressesbylabel, {"label"} }, - { "wallet", "getaddressinfo", &getaddressinfo, {"address"} }, - { "wallet", "getbalance", &getbalance, {"dummy","minconf","include_watchonly","avoid_reuse"} }, - { "wallet", "getnewaddress", &getnewaddress, {"label","address_type"} }, - { "wallet", "getrawchangeaddress", &getrawchangeaddress, {"address_type"} }, - { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf"} }, - { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf"} }, - { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","verbose"} }, - { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, - { "wallet", "getbalances", &getbalances, {} }, - { "wallet", "getwalletinfo", &getwalletinfo, {} }, - { "wallet", "importaddress", &importaddress, {"address","label","rescan","p2sh"} }, - { "wallet", "importdescriptors", &importdescriptors, {"requests"} }, - { "wallet", "importmulti", &importmulti, {"requests","options"} }, - { "wallet", "importprivkey", &importprivkey, {"privkey","label","rescan"} }, - { "wallet", "importprunedfunds", &importprunedfunds, {"rawtransaction","txoutproof"} }, - { "wallet", "importpubkey", &importpubkey, {"pubkey","label","rescan"} }, - { "wallet", "importwallet", &importwallet, {"filename"} }, - { "wallet", "keypoolrefill", &keypoolrefill, {"newsize"} }, - { "wallet", "listaddressgroupings", &listaddressgroupings, {} }, - { "wallet", "listlabels", &listlabels, {"purpose"} }, - { "wallet", "listlockunspent", &listlockunspent, {} }, - { "wallet", "listreceivedbyaddress", &listreceivedbyaddress, {"minconf","include_empty","include_watchonly","address_filter"} }, - { "wallet", "listreceivedbylabel", &listreceivedbylabel, {"minconf","include_empty","include_watchonly"} }, - { "wallet", "listsinceblock", &listsinceblock, {"blockhash","target_confirmations","include_watchonly","include_removed"} }, - { "wallet", "listtransactions", &listtransactions, {"label|dummy","count","skip","include_watchonly"} }, - { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, - { "wallet", "listwalletdir", &listwalletdir, {} }, - { "wallet", "listwallets", &listwallets, {} }, - { "wallet", "loadwallet", &loadwallet, {"filename", "load_on_startup"} }, - { "wallet", "lockunspent", &lockunspent, {"unlock","transactions"} }, - { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, - { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, - { "wallet", "send", &send, {"outputs","conf_target","estimate_mode","fee_rate","options"} }, - { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","comment","subtractfeefrom","replaceable","conf_target","estimate_mode","fee_rate","verbose"} }, - { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","replaceable","conf_target","estimate_mode","avoid_reuse","fee_rate","verbose"} }, - { "wallet", "sethdseed", &sethdseed, {"newkeypool","seed"} }, - { "wallet", "setlabel", &setlabel, {"address","label"} }, - { "wallet", "settxfee", &settxfee, {"amount"} }, - { "wallet", "setwalletflag", &setwalletflag, {"flag","value"} }, - { "wallet", "signmessage", &signmessage, {"address","message"} }, - { "wallet", "signrawtransactionwithwallet", &signrawtransactionwithwallet, {"hexstring","prevtxs","sighashtype"} }, - { "wallet", "unloadwallet", &unloadwallet, {"wallet_name", "load_on_startup"} }, - { "wallet", "upgradewallet", &upgradewallet, {"version"} }, - { "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs"} }, - { "wallet", "walletlock", &walletlock, {} }, - { "wallet", "walletpassphrase", &walletpassphrase, {"passphrase","timeout"} }, - { "wallet", "walletpassphrasechange", &walletpassphrasechange, {"oldpassphrase","newpassphrase"} }, - { "wallet", "walletprocesspsbt", &walletprocesspsbt, {"psbt","sign","sighashtype","bip32derivs"} }, +{ // category actor (function) + // ------------------ ------------------------ + { "rawtransactions", &fundrawtransaction, }, + { "wallet", &abandontransaction, }, + { "wallet", &abortrescan, }, + { "wallet", &addmultisigaddress, }, + { "wallet", &backupwallet, }, + { "wallet", &bumpfee, }, + { "wallet", &psbtbumpfee, }, + { "wallet", &createwallet, }, + { "wallet", &dumpprivkey, }, + { "wallet", &dumpwallet, }, + { "wallet", &encryptwallet, }, + { "wallet", &getaddressesbylabel, }, + { "wallet", &getaddressinfo, }, + { "wallet", &getbalance, }, + { "wallet", &getnewaddress, }, + { "wallet", &getrawchangeaddress, }, + { "wallet", &getreceivedbyaddress, }, + { "wallet", &getreceivedbylabel, }, + { "wallet", &gettransaction, }, + { "wallet", &getunconfirmedbalance, }, + { "wallet", &getbalances, }, + { "wallet", &getwalletinfo, }, + { "wallet", &importaddress, }, + { "wallet", &importdescriptors, }, + { "wallet", &importmulti, }, + { "wallet", &importprivkey, }, + { "wallet", &importprunedfunds, }, + { "wallet", &importpubkey, }, + { "wallet", &importwallet, }, + { "wallet", &keypoolrefill, }, + { "wallet", &listaddressgroupings, }, + { "wallet", &listdescriptors, }, + { "wallet", &listlabels, }, + { "wallet", &listlockunspent, }, + { "wallet", &listreceivedbyaddress, }, + { "wallet", &listreceivedbylabel, }, + { "wallet", &listsinceblock, }, + { "wallet", &listtransactions, }, + { "wallet", &listunspent, }, + { "wallet", &listwalletdir, }, + { "wallet", &listwallets, }, + { "wallet", &loadwallet, }, + { "wallet", &lockunspent, }, + { "wallet", &removeprunedfunds, }, + { "wallet", &rescanblockchain, }, + { "wallet", &send, }, + { "wallet", &sendmany, }, + { "wallet", &sendtoaddress, }, + { "wallet", &sethdseed, }, + { "wallet", &setlabel, }, + { "wallet", &settxfee, }, + { "wallet", &setwalletflag, }, + { "wallet", &signmessage, }, + { "wallet", &signrawtransactionwithwallet, }, + { "wallet", &unloadwallet, }, + { "wallet", &upgradewallet, }, + { "wallet", &walletcreatefundedpsbt, }, + { "wallet", &walletlock, }, + { "wallet", &walletpassphrase, }, + { "wallet", &walletpassphrasechange, }, + { "wallet", &walletprocesspsbt, }, }; // clang-format on return MakeSpan(commands); diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 15972fe7bb..55110f30d9 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -94,8 +94,7 @@ IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& s TxoutType whichType = Solver(scriptPubKey, vSolutions); CKeyID keyID; - switch (whichType) - { + switch (whichType) { case TxoutType::NONSTANDARD: case TxoutType::NULL_DATA: case TxoutType::WITNESS_UNKNOWN: @@ -194,7 +193,7 @@ IsMineResult IsMineInner(const LegacyScriptPubKeyMan& keystore, const CScript& s } break; } - } + } // no default case, so the compiler can warn about missing cases if (ret == IsMineResult::NO && keystore.HaveWatchOnly(scriptPubKey)) { ret = std::max(ret, IsMineResult::WATCH_ONLY); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 202804c9ff..ffac78d752 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -35,7 +35,7 @@ static CAmount balance = 0; CoinEligibilityFilter filter_standard(1, 6, 0); CoinEligibilityFilter filter_confirmed(1, 1, 0); CoinEligibilityFilter filter_standard_extra(6, 6, 0); -CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0); +CoinSelectionParams coin_selection_params(false, 0, 0, CFeeRate(0), 0, false); static void add_coin(const CAmount& nValue, int nInput, std::vector<CInputCoin>& set) { @@ -115,7 +115,10 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<CInputCoin>& coins { static std::vector<OutputGroup> static_groups; static_groups.clear(); - for (auto& coin : coins) static_groups.emplace_back(coin, 0, true, 0, 0); + for (auto& coin : coins) { + static_groups.emplace_back(); + static_groups.back().Insert(coin, 0, true, 0, 0, false); + } return static_groups; } @@ -123,7 +126,10 @@ inline std::vector<OutputGroup>& GroupCoins(const std::vector<COutput>& coins) { static std::vector<OutputGroup> static_groups; static_groups.clear(); - for (auto& coin : coins) static_groups.emplace_back(coin.GetInputCoin(), coin.nDepth, coin.tx->m_amounts[CWalletTx::DEBIT].m_cached[ISMINE_SPENDABLE] && coin.tx->m_amounts[CWalletTx::DEBIT].m_value[ISMINE_SPENDABLE] == 1 /* HACK: we can't figure out the is_me flag so we use the conditions defined above; perhaps set safe to false for !fIsFromMe in add_coin() */, 0, 0); + for (auto& coin : coins) { + static_groups.emplace_back(); + static_groups.back().Insert(coin.GetInputCoin(), coin.nDepth, coin.tx->m_amounts[CWalletTx::DEBIT].m_cached[ISMINE_SPENDABLE] && coin.tx->m_amounts[CWalletTx::DEBIT].m_value[ISMINE_SPENDABLE] == 1 /* HACK: we can't figure out the is_me flag so we use the conditions defined above; perhaps set safe to false for !fIsFromMe in add_coin() */, 0, 0, false); + } return static_groups; } @@ -263,22 +269,22 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) } // Make sure that effective value is working in SelectCoinsMinConf when BnB is used - CoinSelectionParams coin_selection_params_bnb(true, 0, 0, CFeeRate(3000), 0); + CoinSelectionParams coin_selection_params_bnb(true, 0, 0, CFeeRate(3000), 0, false); CoinSet setCoinsRet; CAmount nValueRet; bool bnb_used; empty_wallet(); add_coin(1); vCoins.at(0).nInputBytes = 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(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); // Test fees subtracted from output: empty_wallet(); add_coin(1 * CENT); vCoins.at(0).nInputBytes = 40; - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); coin_selection_params_bnb.m_subtract_fee_outputs = true; - BOOST_CHECK(testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params_bnb, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); // Make sure that can use BnB when there are preset inputs @@ -317,24 +323,24 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) empty_wallet(); // with an empty wallet we can't even pay one cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); add_coin(1*CENT, 4); // add a new 1 cent coin // with a new 1 cent coin, we still can't find a mature 1 cent - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 1 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can find a new 1 cent - BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 1 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * CENT); add_coin(2*CENT); // add a mature 2 cent coin // we can't make 3 cents of mature coins - BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf( 3 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can make 3 cents of new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 3 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 3 * CENT); add_coin(5*CENT); // add a mature 5 cent coin, @@ -344,33 +350,33 @@ 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(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // we can't even make 37 cents if we don't allow new coins even if they're from us - BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(38 * CENT, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // but we can make 37 cents if we accept new coins from ourself - BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(37 * CENT, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 37 * CENT); // and we can make 38 cents if we accept all new coins - BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(38 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 38 * CENT); // try making 34 cents from 1,2,5,10,20 - we can't do it exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(34 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 35 * CENT); // but 35 cents is closest BOOST_CHECK_EQUAL(setCoinsRet.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 - BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 7 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 7 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // when we try making 8 cents, the smaller coins (1,2,5) are exactly enough. - BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 8 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK(nValueRet == 8 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // when we try making 9 cents, no subset of smaller coins is enough, and we get the next bigger coin (10) - BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf( 9 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 10 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -384,30 +390,30 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(30*CENT); // now we have 6+7+8+20+30 = 71 cents total // check that we have 71 and not 72 - BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(71 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(!testWallet.SelectCoinsMinConf(72 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // 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 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 20 * CENT); // we should get 20 in one coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); add_coin( 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 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 3 coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); add_coin( 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 - BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(16 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 18 * CENT); // we should get 18 in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); // because in the event of a tie, the biggest coin wins // now try making 11 cents. we should get 5+6 - BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(11 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 11 * CENT); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -416,11 +422,11 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin( 2*COIN); add_coin( 3*COIN); add_coin( 4*COIN); // now we have 5+6+7+8+18+20+30+100+200+300+400 = 1094 cents - BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(95 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * COIN); // we should get 1 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); - BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(195 * CENT, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 2 * COIN); // we should get 2 BTC in 1 coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -435,14 +441,14 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // try making 1 * MIN_CHANGE from the 1.5 * MIN_CHANGE // we'll get change smaller than MIN_CHANGE whatever happens, so can expect MIN_CHANGE exactly - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // but if we add a bigger coin, small change is avoided add_coin(1111*MIN_CHANGE); // try making 1 from 0.1 + 0.2 + 0.3 + 0.4 + 0.5 + 1111 = 1112.5 - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // if we add more small coins: @@ -450,7 +456,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 7 / 10); // and try again to make 1.0 * MIN_CHANGE - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1 * MIN_CHANGE); // we should get the exact amount // run the 'mtgox' test (see https://blockexplorer.com/tx/29a3efd3ef04f9153d47a990bd7b048a4b2d213daaa5fb8ed670fb85f13bdbcf) @@ -459,7 +465,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) for (int j = 0; j < 20; j++) add_coin(50000 * COIN); - BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(500000 * COIN, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 500000 * COIN); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 10U); // in ten coins @@ -472,7 +478,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 7 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(1 * MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1111 * MIN_CHANGE); // we get the bigger coin BOOST_CHECK_EQUAL(setCoinsRet.size(), 1U); @@ -482,7 +488,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 6 / 10); add_coin(MIN_CHANGE * 8 / 10); add_coin(1111 * MIN_CHANGE); - BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK( testWallet.SelectCoinsMinConf(MIN_CHANGE, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE); // we should get the exact amount BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); // in two coins 0.4+0.6 @@ -493,12 +499,12 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) add_coin(MIN_CHANGE * 100); // trying to make 100.01 from these three coins - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 10001 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, MIN_CHANGE * 10105 / 100); // we should get all coins BOOST_CHECK_EQUAL(setCoinsRet.size(), 3U); // but if we try to make 99.9, we should take the bigger of the two small coins to avoid small change - BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(MIN_CHANGE * 9990 / 100, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 101 * MIN_CHANGE); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); } @@ -512,7 +518,7 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) // We only create the wallet once to save time, but we still run the coin selection RUN_TESTS times. for (int i = 0; i < RUN_TESTS; i++) { - BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(2000, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); if (amt - 2000 < MIN_CHANGE) { // needs more than one input: @@ -538,8 +544,8 @@ 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 - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, GroupCoins(vCoins), setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, GroupCoins(vCoins), setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(50 * COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK(!equal_sets(setCoinsRet, setCoinsRet2)); int fails = 0; @@ -547,8 +553,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, GroupCoins(vCoins), setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, GroupCoins(vCoins), setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(COIN, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -570,8 +576,8 @@ BOOST_AUTO_TEST_CASE(knapsack_solver_test) { // selecting 1 from 100 identical coins depends on the shuffle; this test will fail 1% of the time // run the test RANDOM_REPEATS times and only complain if all of them fail - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, GroupCoins(vCoins), setCoinsRet , nValueRet, coin_selection_params, bnb_used)); - BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, GroupCoins(vCoins), setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet , nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(90*CENT, filter_standard, vCoins, setCoinsRet2, nValueRet, coin_selection_params, bnb_used)); if (equal_sets(setCoinsRet, setCoinsRet2)) fails++; } @@ -598,7 +604,7 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset) add_coin(1000 * COIN); add_coin(3 * COIN); - BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, GroupCoins(vCoins), setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(1003 * COIN, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); BOOST_CHECK_EQUAL(nValueRet, 1003 * COIN); BOOST_CHECK_EQUAL(setCoinsRet.size(), 2U); @@ -633,13 +639,13 @@ BOOST_AUTO_TEST_CASE(SelectCoins_test) CAmount target = rand.randrange(balance - 1000) + 1000; // Perform selection - CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0); - CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0); + CoinSelectionParams coin_selection_params_knapsack(false, 34, 148, CFeeRate(0), 0, false); + CoinSelectionParams coin_selection_params_bnb(true, 34, 148, CFeeRate(0), 0, false); CoinSet out_set; CAmount out_value = 0; bool bnb_used = false; - BOOST_CHECK(testWallet.SelectCoinsMinConf(target, filter_standard, GroupCoins(vCoins), out_set, out_value, coin_selection_params_bnb, bnb_used) || - testWallet.SelectCoinsMinConf(target, filter_standard, GroupCoins(vCoins), out_set, out_value, coin_selection_params_knapsack, bnb_used)); + BOOST_CHECK(testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_bnb, bnb_used) || + testWallet.SelectCoinsMinConf(target, filter_standard, vCoins, out_set, out_value, coin_selection_params_knapsack, bnb_used)); BOOST_CHECK_GE(out_value, target); } } diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index 27179839b7..b2eb8e4bca 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -30,8 +30,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_file) std::string filename; std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, filename); - BOOST_CHECK(filename == test_name); - BOOST_CHECK(env->Directory() == datadir); + BOOST_CHECK_EQUAL(filename, test_name); + BOOST_CHECK_EQUAL(env->Directory(), datadir); } BOOST_AUTO_TEST_CASE(getwalletenv_directory) @@ -41,8 +41,8 @@ BOOST_AUTO_TEST_CASE(getwalletenv_directory) std::string filename; std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(datadir, filename); - BOOST_CHECK(filename == expected_name); - BOOST_CHECK(env->Directory() == datadir); + BOOST_CHECK_EQUAL(filename, expected_name); + BOOST_CHECK_EQUAL(env->Directory(), datadir); } BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index aa7f1c83d8..f035a70a20 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include <fs.h> +#include <univalue.h> #include <util/check.h> #include <util/system.h> @@ -37,6 +38,9 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam InitWalletDirTestingSetup::~InitWalletDirTestingSetup() { + gArgs.LockSettings([&](util::Settings& settings) { + settings.forced_settings.erase("walletdir"); + }); fs::current_path(m_cwd); } diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index e70b56c529..45e1b8c4b8 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -19,7 +19,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default) BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); - BOOST_CHECK(walletdir == expected_path); + BOOST_CHECK_EQUAL(walletdir, expected_path); } BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom) @@ -29,7 +29,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom) BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]); - BOOST_CHECK(walletdir == expected_path); + BOOST_CHECK_EQUAL(walletdir, expected_path); } BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_does_not_exist) @@ -69,7 +69,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing) BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); - BOOST_CHECK(walletdir == expected_path); + BOOST_CHECK_EQUAL(walletdir, expected_path); } BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) @@ -79,7 +79,7 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) BOOST_CHECK(result == true); fs::path walletdir = gArgs.GetArg("-walletdir", ""); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); - BOOST_CHECK(walletdir == expected_path); + BOOST_CHECK_EQUAL(walletdir, expected_path); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 918946f9e7..db80745db0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2357,13 +2357,12 @@ const CTxOut& CWallet::FindNonChangeParentOutput(const CTransaction& tx, int out return ptx->vout[n]; } -bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<OutputGroup> groups, +bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const { setCoinsRet.clear(); nValueRet = 0; - std::vector<OutputGroup> utxo_pool; if (coin_selection_params.use_bnb) { // Get long term estimate FeeCalculation feeCalc; @@ -2371,35 +2370,27 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil temp.m_confirm_target = 1008; CFeeRate long_term_feerate = GetMinimumFeeRate(*this, temp, &feeCalc); - // Calculate cost of change - CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); + // Get the feerate for effective value. + // When subtracting the fee from the outputs, we want the effective feerate to be 0 + CFeeRate effective_feerate{0}; + if (!coin_selection_params.m_subtract_fee_outputs) { + effective_feerate = coin_selection_params.effective_fee; + } - // Filter by the min conf specs and add to utxo_pool and calculate effective value - for (OutputGroup& group : groups) { - if (!group.EligibleForSpending(eligibility_filter)) continue; + std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, effective_feerate, long_term_feerate, eligibility_filter, true /* positive_only */); - if (coin_selection_params.m_subtract_fee_outputs) { - // Set the effective feerate to 0 as we don't want to use the effective value since the fees will be deducted from the output - group.SetFees(CFeeRate(0) /* effective_feerate */, long_term_feerate); - } else { - group.SetFees(coin_selection_params.effective_fee, long_term_feerate); - } + // Calculate cost of change + CAmount cost_of_change = GetDiscardRate(*this).GetFee(coin_selection_params.change_spend_size) + coin_selection_params.effective_fee.GetFee(coin_selection_params.change_output_size); - OutputGroup pos_group = group.GetPositiveOnlyGroup(); - if (pos_group.effective_value > 0) utxo_pool.push_back(pos_group); - } // Calculate the fees for things that aren't inputs CAmount not_input_fees = coin_selection_params.effective_fee.GetFee(coin_selection_params.tx_noinputs_size); bnb_used = true; - return SelectCoinsBnB(utxo_pool, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); + return SelectCoinsBnB(groups, nTargetValue, cost_of_change, setCoinsRet, nValueRet, not_input_fees); } else { - // Filter by the min conf specs and add to utxo_pool - for (const OutputGroup& group : groups) { - if (!group.EligibleForSpending(eligibility_filter)) continue; - utxo_pool.push_back(group); - } + std::vector<OutputGroup> groups = GroupOutputs(coins, !coin_selection_params.m_avoid_partial_spends, CFeeRate(0), CFeeRate(0), eligibility_filter, false /* positive_only */); + bnb_used = false; - return KnapsackSolver(nTargetValue, utxo_pool, setCoinsRet, nValueRet); + return KnapsackSolver(nTargetValue, groups, setCoinsRet, nValueRet); } } @@ -2482,16 +2473,14 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm // explicitly shuffling the outputs before processing Shuffle(vCoins.begin(), vCoins.end(), FastRandomContext()); } - std::vector<OutputGroup> groups = GroupOutputs(vCoins, !coin_control.m_avoid_partial_spends, max_ancestors); - bool res = value_to_select <= 0 || - SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || - SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || - (m_spend_zero_conf_change && !fRejectLongChains && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max()), groups, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); + SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::min((size_t)4, max_ancestors/3), std::min((size_t)4, max_descendants/3)), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors-1, max_descendants-1, true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)) || + (m_spend_zero_conf_change && !fRejectLongChains && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, std::numeric_limits<uint64_t>::max(), std::numeric_limits<uint64_t>::max(), true /* include_partial_groups */), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used)); // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset util::insert(setCoinsRet, setPresetCoins); @@ -2782,6 +2771,7 @@ bool CWallet::CreateTransactionInternal( std::vector<COutput> vAvailableCoins; AvailableCoins(vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0); CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy + coin_selection_params.m_avoid_partial_spends = coin_control.m_avoid_partial_spends; // Create change script that will be used if we need change // TODO: pass in scriptChange instead of reservedest so @@ -3780,7 +3770,7 @@ std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, cons // 2. Path to an existing directory. // 3. Path to a symlink to a directory. // 4. For backwards compatibility, the name of a data file in -walletdir. - const fs::path& wallet_path = fs::absolute(name, GetWalletDir()); + const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), name); fs::file_type path_type = fs::symlink_status(wallet_path).type(); if (!(path_type == fs::file_not_found || path_type == fs::directory_file || (path_type == fs::symlink_file && fs::is_directory(wallet_path)) || @@ -4192,51 +4182,90 @@ bool CWalletTx::IsImmatureCoinBase() const return GetBlocksToMaturity() > 0; } -std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const { - std::vector<OutputGroup> groups; - std::map<CTxDestination, OutputGroup> gmap; - std::set<CTxDestination> full_groups; +std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool separate_coins, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate, const CoinEligibilityFilter& filter, bool positive_only) const +{ + std::vector<OutputGroup> groups_out; - for (const auto& output : outputs) { - if (output.fSpendable) { - CTxDestination dst; - CInputCoin input_coin = output.GetInputCoin(); + if (separate_coins) { + // Single coin means no grouping. Each COutput gets its own OutputGroup. + for (const COutput& output : outputs) { + // Skip outputs we cannot spend + if (!output.fSpendable) continue; size_t ancestors, descendants; chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); - if (!single_coin && ExtractDestination(output.tx->tx->vout[output.i].scriptPubKey, dst)) { - auto it = gmap.find(dst); - if (it != gmap.end()) { - // Limit output groups to no more than OUTPUT_GROUP_MAX_ENTRIES - // number of entries, to protect against inadvertently creating - // a too-large transaction when using -avoidpartialspends to - // prevent breaking consensus or surprising users with a very - // high amount of fees. - if (it->second.m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) { - groups.push_back(it->second); - it->second = OutputGroup{}; - full_groups.insert(dst); - } - it->second.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); - } else { - gmap[dst].Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); - } - } else { - groups.emplace_back(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants); - } + CInputCoin input_coin = output.GetInputCoin(); + + // Make an OutputGroup containing just this output + OutputGroup group{effective_feerate, long_term_feerate}; + group.Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants, positive_only); + + // Check the OutputGroup's eligibility. Only add the eligible ones. + if (positive_only && group.effective_value <= 0) continue; + if (group.m_outputs.size() > 0 && group.EligibleForSpending(filter)) groups_out.push_back(group); + } + return groups_out; + } + + // We want to combine COutputs that have the same scriptPubKey into single OutputGroups + // except when there are more than OUTPUT_GROUP_MAX_ENTRIES COutputs grouped in an OutputGroup. + // To do this, we maintain a map where the key is the scriptPubKey and the value is a vector of OutputGroups. + // For each COutput, we check if the scriptPubKey is in the map, and if it is, the COutput's CInputCoin is added + // to the last OutputGroup in the vector for the scriptPubKey. When the last OutputGroup has + // OUTPUT_GROUP_MAX_ENTRIES CInputCoins, a new OutputGroup is added to the end of the vector. + std::map<CScript, std::vector<OutputGroup>> spk_to_groups_map; + for (const auto& output : outputs) { + // Skip outputs we cannot spend + if (!output.fSpendable) continue; + + size_t ancestors, descendants; + chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); + CInputCoin input_coin = output.GetInputCoin(); + CScript spk = input_coin.txout.scriptPubKey; + + std::vector<OutputGroup>& groups = spk_to_groups_map[spk]; + + if (groups.size() == 0) { + // No OutputGroups for this scriptPubKey yet, add one + groups.emplace_back(effective_feerate, long_term_feerate); } + + // Get the last OutputGroup in the vector so that we can add the CInputCoin to it + // A pointer is used here so that group can be reassigned later if it is full. + OutputGroup* group = &groups.back(); + + // Check if this OutputGroup is full. We limit to OUTPUT_GROUP_MAX_ENTRIES when using -avoidpartialspends + // to avoid surprising users with very high fees. + if (group->m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) { + // The last output group is full, add a new group to the vector and use that group for the insertion + groups.emplace_back(effective_feerate, long_term_feerate); + group = &groups.back(); + } + + // Add the input_coin to group + group->Insert(input_coin, output.nDepth, output.tx->IsFromMe(ISMINE_ALL), ancestors, descendants, positive_only); } - if (!single_coin) { - for (auto& it : gmap) { - auto& group = it.second; - if (full_groups.count(it.first) > 0) { - // Make this unattractive as we want coin selection to avoid it if possible - group.m_ancestors = max_ancestors - 1; + + // Now we go through the entire map and pull out the OutputGroups + for (const auto& spk_and_groups_pair: spk_to_groups_map) { + const std::vector<OutputGroup>& groups_per_spk= spk_and_groups_pair.second; + + // Go through the vector backwards. This allows for the first item we deal with being the partial group. + for (auto group_it = groups_per_spk.rbegin(); group_it != groups_per_spk.rend(); group_it++) { + const OutputGroup& group = *group_it; + + // Don't include partial groups if there are full groups too and we don't want partial groups + if (group_it == groups_per_spk.rbegin() && groups_per_spk.size() > 1 && !filter.m_include_partial_groups) { + continue; } - groups.push_back(group); + + // Check the OutputGroup's eligibility. Only add the eligible ones. + if (positive_only && group.effective_value <= 0) continue; + if (group.m_outputs.size() > 0 && group.EligibleForSpending(filter)) groups_out.push_back(group); } } - return groups; + + return groups_out; } bool CWallet::IsCrypted() const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e3eae1dd95..4fc4466604 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -610,8 +610,16 @@ struct CoinSelectionParams size_t tx_noinputs_size = 0; //! Indicate that we are subtracting the fee from outputs bool m_subtract_fee_outputs = false; - - CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size) : use_bnb(use_bnb), change_output_size(change_output_size), change_spend_size(change_spend_size), effective_fee(effective_fee), tx_noinputs_size(tx_noinputs_size) {} + bool m_avoid_partial_spends = false; + + CoinSelectionParams(bool use_bnb, size_t change_output_size, size_t change_spend_size, CFeeRate effective_fee, size_t tx_noinputs_size, bool avoid_partial) : + use_bnb(use_bnb), + change_output_size(change_output_size), + change_spend_size(change_spend_size), + effective_fee(effective_fee), + tx_noinputs_size(tx_noinputs_size), + m_avoid_partial_spends(avoid_partial) + {} CoinSelectionParams() {} }; @@ -818,7 +826,7 @@ public: * completion the coin set and corresponding actual target value is * assembled */ - bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<OutputGroup> groups, + bool SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibilityFilter& eligibility_filter, std::vector<COutput> coins, std::set<CInputCoin>& setCoinsRet, CAmount& nValueRet, const CoinSelectionParams& coin_selection_params, bool& bnb_used) const; bool IsSpent(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); @@ -827,7 +835,7 @@ public: bool IsSpentKey(const uint256& hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void SetSpentKeyState(WalletBatch& batch, const uint256& hash, unsigned int n, bool used, std::set<CTxDestination>& tx_destinations) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const; + std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool separate_coins, const CFeeRate& effective_feerate, const CFeeRate& long_term_feerate, const CoinEligibilityFilter& filter, bool positive_only) const; bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index a1bb7343f4..b2cb0bf479 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -103,10 +103,8 @@ static void WalletShowInfo(CWallet* wallet_instance) tfm::format(std::cout, "Address Book: %zu\n", wallet_instance->m_address_book.size()); } -bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& name) +bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) { - fs::path path = fs::absolute(name, GetWalletDir()); - if (args.IsArgSet("-format") && command != "createfromdump") { tfm::format(std::cerr, "The -format option can only be used with the \"createfromdump\" command.\n"); return false; @@ -119,6 +117,12 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n"); return false; } + if (command == "create" && !args.IsArgSet("-wallet")) { + tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n"); + return false; + } + const std::string name = args.GetArg("-wallet", ""); + const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), name); if (command == "create") { DatabaseOptions options; diff --git a/src/wallet/wallettool.h b/src/wallet/wallettool.h index f544a6f727..f4516bb5bc 100644 --- a/src/wallet/wallettool.h +++ b/src/wallet/wallettool.h @@ -10,7 +10,7 @@ namespace WalletTool { void WalletShowInfo(CWallet* wallet_instance); -bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command, const std::string& file); +bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command); } // namespace WalletTool |