Merge bitcoin#21759: wallet: document coin selection code

6ba8921 refactor + document coin selection strategy (glozow)
58ea324 [docs] add doxygen comments to wallet code (glozow)
0c74716 [docs] format existing comments as doxygen (glozow)

Pull request description:

  I think it would help code review to have more documentation + doxygen comments

ACKs for top commit:
  Xekyo:
    ReACK bitcoin@6ba8921
  achow101:
    ACK 6ba8921

Tree-SHA512: 74a78d9b0e0c1d5659bed566432a5b3511511d8b2432f440565f443da7b8257a1b90e70aa7505a7f8abf618748eeb43d166e84f278bdee3d34ce5d5c37dc573a

src/wallet/coinselection.h

@@ -15,6 +15,7 @@ static constexpr CAmount MIN_CHANGE{COIN / 100};
 //! final minimum change amount after paying for fees
 static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2;
 
+/** A UTXO under consideration for use in funding a new transaction. */
 class CInputCoin {
 public:
  CInputCoin(const CTransactionRef& tx, unsigned int i)
@@ -56,31 +57,58 @@ class CInputCoin {
  }
 };
 
+/** Parameters for filtering which OutputGroups we may use in coin selection.
+ * We start by being very selective and requiring multiple confirmations and
+ * then get more permissive if we cannot fund the transaction. */
 struct CoinEligibilityFilter
 {
+ /** Minimum number of confirmations for outputs that we sent to ourselves.
+ * We may use unconfirmed UTXOs sent from ourselves, e.g. change outputs. */
  const int conf_mine;
+ /** Minimum number of confirmations for outputs received from a different
+ * wallet. We never spend unconfirmed foreign outputs as we cannot rely on these funds yet. */
  const int conf_theirs;
+ /** Maximum number of unconfirmed ancestors aggregated across all UTXOs in an OutputGroup. */
  const uint64_t max_ancestors;
+ /** Maximum number of descendants that a single UTXO in the OutputGroup may have. */
  const uint64_t max_descendants;
- const bool m_include_partial_groups{false}; //! Include partial destination groups when avoid_reuse and there are full groups
+ /** When avoid_reuse=true and there are full groups (OUTPUT_GROUP_MAX_ENTRIES), whether or not to use any partial groups.*/
+ const bool m_include_partial_groups{false};
 
  CoinEligibilityFilter(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) {}
 };
 
+/** A group of UTXOs paid to the same output script. */
 struct OutputGroup
 {
+ /** The list of UTXOs contained in this output group. */
  std::vector<CInputCoin> m_outputs;
+ /** Whether the UTXOs were sent by the wallet to itself. This is relevant because we may want at
+ * least a certain number of confirmations on UTXOs received from outside wallets while trusting
+ * our own UTXOs more. */
  bool m_from_me{true};
+ /** The total value of the UTXOs in sum. */
  CAmount m_value{0};
+ /** The minimum number of confirmations the UTXOs in the group have. Unconfirmed is 0. */
  int m_depth{999};
+ /** The aggregated count of unconfirmed ancestors of all UTXOs in this
+ * group. Not deduplicated and may overestimate when ancestors are shared. */
  size_t m_ancestors{0};
+ /** The maximum count of descendants of a single UTXO in this output group. */
  size_t m_descendants{0};
+ /** The value of the UTXOs after deducting the cost of spending them at the effective feerate. */
  CAmount effective_value{0};
+ /** The fee to spend these UTXOs at the effective feerate. */
  CAmount fee{0};
+ /** The target feerate of the transaction we're trying to build. */
  CFeeRate m_effective_feerate{0};
+ /** The fee to spend these UTXOs at the long term feerate. */
  CAmount long_term_fee{0};
+ /** The feerate for spending a created change output eventually (i.e. not urgently, and thus at
+ * a lower feerate). Calculated using long term fee estimate. This is used to decide whether
+ * it could be economical to create a change output. */
  CFeeRate m_long_term_feerate{0};
 
  OutputGroup() {}

src/wallet/wallet.cpp

@@ -2877,7 +2877,7 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
  }
  }
 
- // remove preset inputs from vCoins
+ // remove preset inputs from vCoins so that Coin Selection doesn't pick them.
  for (std::vector<COutput>::iterator it = vCoins.begin(); it != vCoins.end() && coin_control.HasSelected();)
  {
  if (setPresetCoins.count(it->GetInputCoin()))
@@ -2889,9 +2889,9 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
  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);
+ const size_t max_ancestors = (size_t)std::max<int64_t>(1, limit_ancestor_count);
+ const size_t max_descendants = (size_t)std::max<int64_t>(1, limit_descendant_count);
+ const bool fRejectLongChains = gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS);
 
  // form groups from remaining coins; note that preset coins will not
  // automatically have their associated (same address) coins included
@@ -2901,16 +2901,52 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
  // explicitly shuffling the outputs before processing
  Shuffle(vCoins.begin(), vCoins.end(), FastRandomContext());
  }
- bool res = value_to_select <= 0 ||
- SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType) ||
- SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType) ||
- (m_spend_zero_conf_change && SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) ||
- (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, nCoinType)) ||
- (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, nCoinType)) ||
- (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, nCoinType)) ||
- (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, nCoinType));
+ // 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.
+ const bool res = [&] {
+ // Pre-selected inputs already cover the target amount.
+ if (value_to_select <= 0) return true;
+
+ // If possible, fund the transaction with confirmed UTXOs only. Prefer at least six
+ // confirmations on outputs received from other wallets and only spend confirmed change.
+ if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 6, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true;
+ if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(1, 1, 0), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true;
+
+ // Fall back to using zero confirmation change (but with as few ancestors in the mempool as
+ // possible) if we cannot fund the transaction otherwise. We never spend unconfirmed
+ // outputs received from other wallets.
+ if (m_spend_zero_conf_change) {
+ if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, 2), vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) return true;
+ if (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, nCoinType)) {
+ return true;
+ }
+ if (SelectCoinsMinConf(value_to_select, CoinEligibilityFilter(0, 1, max_ancestors/2, max_descendants/2),
+ vCoins, setCoinsRet, nValueRet, coin_selection_params, bnb_used, nCoinType)) {
+ return true;
+ }
+ // If partial groups are allowed, relax the requirement of spending OutputGroups (groups
+ // of UTXOs sent to the same address, which are obviously controlled by a single wallet)
+ // in their entirety.
+ if (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, nCoinType)) {
+ return true;
+ }
+ // Try with unlimited ancestors/descendants. The transaction will still need to meet
+ // mempool ancestor/descendant policy to be accepted to mempool and broadcasted, but
+ // OutputGroups use heuristics that may overestimate ancestor/descendant counts.
+ if (!fRejectLongChains && 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, nCoinType)) {
+ return true;
+ }
+ }
+ // Coin Selection failed.
+ return false;
+ }();
 
- // because SelectCoinsMinConf clears the setCoinsRet, we now add the possible inputs to the coinset
+ // SelectCoinsMinConf clears setCoinsRet, so add the preset inputs from coin_control to the coinset
  util::insert(setCoinsRet, setPresetCoins);
 
  // add preset inputs to the total value selected