Merge #13812: wallet: sum ancestors rather than taking max in output groups

23fbbb100f63cb621b4b901dac0c0f16d7d74bc7 wallet: sum ancestors rather than taking max in output groups (Karl-Johan Alm)

Pull request description:

  This is pointed out in https://github.com/bitcoin/bitcoin/pull/12257#discussion_r204549758.

  Basically, the ancestors gives an indication as to how many ancestors the resulting transaction will have, which is more precise when summing up the values, rather than taking the maximum, since all the coins in the group will become ancestors if selected.

Tree-SHA512: 0588c4b6059669650614817e041526a2ab89dda8c07fca8e077c7669dca1fed51cd164f7df56340840ab60285d48f3b140dcee64f64bf696b2dd4ab16d556a13

1 files changed, 6 insertions, 6 deletions

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index 1a810e763f..710146ef96 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -299,12 +299,12 @@ void OutputGroup::Insert(const CInputCoin& output, int depth, bool from_me, size
     m_from_me &= from_me;
     m_value += output.effective_value;
     m_depth = std::min(m_depth, depth);
-    // m_ancestors is currently the max ancestor count for all coins in the group; however, this is
-    // not ideal, as a wallet will consider e.g. thirty 2-ancestor coins as having two ancestors,
-    // when in reality it has 60 ancestors.
-    m_ancestors = std::max(m_ancestors, ancestors);
-    // m_descendants is the count as seen from the top ancestor, not the descendants as seen from the
-    // coin itself; thus, this value is accurate
+    // ancestors here express the number of ancestors the new coin will end up having, which is
+    // the sum, rather than the max; this will overestimate in the cases where multiple inputs
+    // have common ancestors
+    m_ancestors += ancestors;
+    // 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 = m_value;
 }