Have KnapsackSolver actually use effective values

Although the CreateTransaction loop currently remains, it should be
largely unused. KnapsackSolver will now account for transaction fees
when doing its selection.

In the previous commit, SelectCoinsMinConf was refactored to have some
calculations become shared for KnapsackSolver and SelectCoinsBnB. In
this commit, KnapsackSolver will now use the not_input_fees and
effective_feerate so that it include the fee for non-input things
(excluding a change output) so that the algorithm will select enough to
cover those fees. This is necessary for selecting on effective values.

Additionally, the OutputGroups
created for KnapsackSolver will actually have their effective values
calculated and set, and KnapsackSolver will do its selection on those
effective values.

Lastly, SelectCoins is modified to use the same value for preselected
inputs for BnB and KnapsackSolver. While it will still use the real
value when subtracting the fee from outputs, this behavior will be
the same regardless of the algo used for selecting additional inputs.

1 files changed, 5 insertions, 5 deletions

diff --git a/src/wallet/coinselection.cpp b/src/wallet/coinselection.cpp
index db770f6a45..af88bf85dd 100644
--- a/src/wallet/coinselection.cpp
+++ b/src/wallet/coinselection.cpp
@@ -227,14 +227,14 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group
     Shuffle(groups.begin(), groups.end(), FastRandomContext());
 
     for (const OutputGroup& group : groups) {
-        if (group.m_value == nTargetValue) {
+        if (group.effective_value == nTargetValue) {
             util::insert(setCoinsRet, group.m_outputs);
             nValueRet += group.m_value;
             return true;
-        } else if (group.m_value < nTargetValue + MIN_CHANGE) {
+        } else if (group.effective_value < nTargetValue + MIN_CHANGE) {
             applicable_groups.push_back(group);
-            nTotalLower += group.m_value;
-        } else if (!lowest_larger || group.m_value < lowest_larger->m_value) {
+            nTotalLower += group.effective_value;
+        } else if (!lowest_larger || group.effective_value < lowest_larger->effective_value) {
             lowest_larger = group;
         }
     }
@@ -267,7 +267,7 @@ bool KnapsackSolver(const CAmount& nTargetValue, std::vector<OutputGroup>& group
     // If we have a bigger coin and (either the stochastic approximation didn't find a good solution,
     //                                   or the next bigger coin is closer), return the bigger coin
     if (lowest_larger &&
-        ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || lowest_larger->m_value <= nBest)) {
+        ((nBest != nTargetValue && nBest < nTargetValue + MIN_CHANGE) || lowest_larger->effective_value <= nBest)) {
         util::insert(setCoinsRet, lowest_larger->m_outputs);
         nValueRet += lowest_larger->m_value;
     } else {