Merge #10942: Eliminate fee overpaying edge case when subtracting fee from recipients

49d903e Eliminate fee overpaying edge case when subtracting fee from recipients (Alex Morcos)

Pull request description:

  I'm not sure if this is the cause of the issue in #10034 , but this was a known edge case.  I just didn't realize how simple the fix is.

  Could use a couple more eyes to make sure nothing silly can go wrong here, but if we all agree it's this simple, we can add this as another 0.15 bug fix.

Tree-SHA512: db1dd1e83363a3c231267b626d3a388893ee70ba1972056fe2c339c5c9e4fbfd30f7fe837c30cc7be884d454797fd4c619b9d631a8d5eeb55cdb07402a83acb3

1 files changed, 8 insertions, 4 deletions

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index cf75cdd744..d236b1de3f 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2789,10 +2789,6 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                     // selected to meet nFeeNeeded result in a transaction that
                     // requires less fee than the prior iteration.
 
-                    // TODO: The case where nSubtractFeeFromAmount > 0 remains
-                    // to be addressed because it requires returning the fee to
-                    // the payees and not the change output.
-
                     // If we have no change and a big enough excess fee, then
                     // try to construct transaction again only without picking
                     // new inputs. We now know we only need the smaller fee
@@ -2819,6 +2815,8 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                 else if (!pick_new_inputs) {
                     // This shouldn't happen, we should have had enough excess
                     // fee to pay for the new output and still meet nFeeNeeded
+                    // Or we should have just subtracted fee from recipients and
+                    // nFeeNeeded should not have changed
                     strFailReason = _("Transaction fee and change calculation failed");
                     return false;
                 }
@@ -2835,6 +2833,12 @@ bool CWallet::CreateTransaction(const std::vector<CRecipient>& vecSend, CWalletT
                     }
                 }
 
+                // If subtracting fee from recipients, we now know what fee we
+                // need to subtract, we have no reason to reselect inputs
+                if (nSubtractFeeFromAmount > 0) {
+                    pick_new_inputs = false;
+                }
+
                 // Include more fee and try again.
                 nFeeRet = nFeeNeeded;
                 continue;