diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/coinselection.cpp | 3 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 6 | ||||
-rw-r--r-- | src/wallet/test/coinselector_tests.cpp | 7 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 53 | ||||
-rw-r--r-- | src/wallet/wallet.h | 2 |
5 files changed, 47 insertions, 24 deletions
diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp index 5bbb2c0ad0..079a5d3d53 100644 --- a/src/wallet/coinselection.cpp +++ b/src/wallet/coinselection.cpp @@ -106,6 +106,9 @@ bool SelectCoinsBnB(std::vector<OutputGroup>& utxo_pool, const CAmount& target_v best_selection = curr_selection; best_selection.resize(utxo_pool.size()); best_waste = curr_waste; + if (best_waste == 0) { + break; + } } curr_waste -= (curr_value - actual_target); // Remove the excess value as we will be selecting different coins now backtrack = true; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d24d92a178..ae3134b89a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2325,7 +2325,8 @@ static UniValue settxfee(const JSONRPCRequest& request) } RPCHelpMan{"settxfee", - "\nSet the transaction fee per kB for this wallet. Overrides the global -paytxfee command line parameter.\n", + "\nSet the transaction fee per kB for this wallet. Overrides the global -paytxfee command line parameter.\n" + "Can be deactivated by passing 0 as the fee. In that case automatic fee selection will be used by default.\n", { {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The transaction fee in " + CURRENCY_UNIT + "/kB"}, }, @@ -2343,12 +2344,15 @@ static UniValue settxfee(const JSONRPCRequest& request) CAmount nAmount = AmountFromValue(request.params[0]); CFeeRate tx_fee_rate(nAmount, 1000); + CFeeRate max_tx_fee_rate(pwallet->m_default_max_tx_fee, 1000); if (tx_fee_rate == CFeeRate(0)) { // automatic selection } else if (tx_fee_rate < pwallet->chain().relayMinFee()) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be less than min relay tx fee (%s)", pwallet->chain().relayMinFee().ToString())); } else if (tx_fee_rate < pwallet->m_min_fee) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be less than wallet min fee (%s)", pwallet->m_min_fee.ToString())); + } else if (tx_fee_rate > max_tx_fee_rate) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be more than wallet max tx fee (%s)", max_tx_fee_rate.ToString())); } pwallet->m_pay_tx_fee = tx_fee_rate; diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index d3ecd7e2cb..66f4542cf9 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -176,8 +176,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) selection.clear(); // Select 5 Cent - add_coin(3 * CENT, 3, actual_selection); - add_coin(2 * CENT, 2, actual_selection); + add_coin(4 * CENT, 4, actual_selection); + add_coin(1 * CENT, 1, actual_selection); BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 5 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); BOOST_CHECK(equal_sets(selection, actual_selection)); BOOST_CHECK_EQUAL(value_ret, 5 * CENT); @@ -204,9 +204,8 @@ BOOST_AUTO_TEST_CASE(bnb_search_test) // Select 10 Cent add_coin(5 * CENT, 5, utxo_pool); + add_coin(5 * CENT, 5, actual_selection); add_coin(4 * CENT, 4, actual_selection); - add_coin(3 * CENT, 3, actual_selection); - add_coin(2 * CENT, 2, actual_selection); add_coin(1 * CENT, 1, actual_selection); BOOST_CHECK(SelectCoinsBnB(GroupCoins(utxo_pool), 10 * CENT, 0.5 * CENT, selection, value_ret, not_input_fees)); BOOST_CHECK(equal_sets(selection, actual_selection)); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 519f022cc8..feb0563409 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2372,6 +2372,13 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm ++it; } + unsigned int limit_ancestor_count = 0; + unsigned int limit_descendant_count = 0; + chain().getPackageLimits(limit_ancestor_count, limit_descendant_count); + size_t max_ancestors = (size_t)std::max<int64_t>(1, limit_ancestor_count); + size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count); + bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); + // form groups from remaining coins; note that preset coins will not // automatically have their associated (same address) coins included if (coin_control.m_avoid_partial_spends && vCoins.size() > OUTPUT_GROUP_MAX_ENTRIES) { @@ -2380,14 +2387,7 @@ 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); - - unsigned int limit_ancestor_count; - unsigned int limit_descendant_count; - chain().getPackageLimits(limit_ancestor_count, limit_descendant_count); - size_t max_ancestors = (size_t)std::max<int64_t>(1, limit_ancestor_count); - size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count); - bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS); + 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) || @@ -4184,32 +4184,49 @@ bool CWalletTx::IsImmatureCoinBase() const return GetBlocksToMaturity() > 0; } -std::vector<OutputGroup> CWallet::GroupOutputs(const std::vector<COutput>& outputs, bool single_coin) const { +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; - CTxDestination dst; + std::set<CTxDestination> full_groups; + for (const auto& output : outputs) { if (output.fSpendable) { + CTxDestination dst; CInputCoin input_coin = output.GetInputCoin(); size_t ancestors, descendants; chain().getTransactionAncestry(output.tx->GetHash(), ancestors, descendants); if (!single_coin && ExtractDestination(output.tx->tx->vout[output.i].scriptPubKey, dst)) { - // Limit output groups to no more than 10 entries, to protect - // against inadvertently creating a too-large transaction - // when using -avoidpartialspends - if (gmap[dst].m_outputs.size() >= OUTPUT_GROUP_MAX_ENTRIES) { - groups.push_back(gmap[dst]); - gmap.erase(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); } - 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); } } } if (!single_coin) { - for (const auto& it : gmap) groups.push_back(it.second); + 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; + } + groups.push_back(group); + } } return groups; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 577a739d89..638c8562c9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -830,7 +830,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; + std::vector<OutputGroup> GroupOutputs(const std::vector<COutput>& outputs, bool single_coin, const size_t max_ancestors) const; bool IsLockedCoin(uint256 hash, unsigned int n) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void LockCoin(const COutPoint& output) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); |