Merge #17458: Refactor OutputGroup effective value calculations and filtering to occur within the struct

9adc2f80fc14f11ee2b1f989ee7be71b58481e6f Refactor OutputGroups to handle effective values, fees, and filtering (Andrew Chow)
7d07e864b8846be186648814a5aaf34269f914a3 Use real value when calculating OutputGroup value (Andrew Chow)

Pull request description:

  Currently, the effective values and filtering for positive effective values is done outside of the OutputGroup. We should instead have functions in Outputgroup to do this and call those for each OutputGroup. So this PR does that.

  This makes future changes for effective values in coin selection much easier.

ACKs for top commit:
  instagibbs:
    reACK 9adc2f80fc14f11ee2b1f989ee7be71b58481e6f
  fjahr:
    re-ACK 9adc2f80fc14f11ee2b1f989ee7be71b58481e6f
  meshcollider:
    Light code review ACK 9adc2f80fc14f11ee2b1f989ee7be71b58481e6f

Tree-SHA512: 7445c94b7295b45bcd83a6f8a5c8f6961a89453fcc856335192d4b4a66aec7724513616b04e5111588ab208c89b311055399d6279cd9c4ce452aefb85f04b64a

1 files changed, 8 insertions, 20 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 54f9ff7a1f..9c7b446c23 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2320,27 +2320,15 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const CoinEligibil
         for (OutputGroup& group : groups) {
             if (!group.EligibleForSpending(eligibility_filter)) continue;
 
-            group.fee = 0;
-            group.long_term_fee = 0;
-            group.effective_value = 0;
-            for (auto it = group.m_outputs.begin(); it != group.m_outputs.end(); ) {
-                const CInputCoin& coin = *it;
-                CAmount effective_value = coin.txout.nValue - (coin.m_input_bytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(coin.m_input_bytes));
-                // Only include outputs that are positive effective value (i.e. not dust)
-                if (effective_value > 0) {
-                    group.fee += coin.m_input_bytes < 0 ? 0 : coin_selection_params.effective_fee.GetFee(coin.m_input_bytes);
-                    group.long_term_fee += coin.m_input_bytes < 0 ? 0 : long_term_feerate.GetFee(coin.m_input_bytes);
-                    if (coin_selection_params.m_subtract_fee_outputs) {
-                        group.effective_value += coin.txout.nValue;
-                    } else {
-                        group.effective_value += effective_value;
-                    }
-                    ++it;
-                } else {
-                    it = group.Discard(coin);
-                }
+            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);
             }
-            if (group.effective_value > 0) utxo_pool.push_back(group);
+
+            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);