From 42f5ce40931402aaa395ef2959deb64e9a9fff02 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 5 Jan 2017 09:10:08 -0500 Subject: [PATCH 1/2] Try to reduce change output to make needed fee in CreateTransaction Once we've picked coins and dummy-signed the transaction to calculate fee, if we don't have sufficient fee, then try to meet the fee by reducing change before resorting to picking new coins. --- src/wallet/wallet.cpp | 12 ++++++++++++ src/wallet/wallet.h | 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ff7a03bc55..bdf6d3c7a6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2537,6 +2537,18 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt if (nFeeRet >= nFeeNeeded) break; // Done, enough fee included. + // Try to reduce change to include necessary fee + if (nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) { + CAmount additionalFeeNeeded = nFeeNeeded - nFeeRet; + vector::iterator change_position = txNew.vout.begin()+nChangePosInOut; + // Only reduce change if remaining amount is still a large enough output. + if (change_position->nValue >= MIN_FINAL_CHANGE + additionalFeeNeeded) { + change_position->nValue -= additionalFeeNeeded; + nFeeRet += additionalFeeNeeded; + break; // Done, able to increase fee from change + } + } + // Include more fee and try again. nFeeRet = nFeeNeeded; continue; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 1d1f84ebb9..b9fa6bb24b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -48,8 +48,10 @@ static const CAmount DEFAULT_TRANSACTION_FEE = 0; static const CAmount DEFAULT_FALLBACK_FEE = 20000; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; -//! minimum change amount +//! target minimum change amount static const CAmount MIN_CHANGE = CENT; +//! final minimum change amount after paying for fees +static const CAmount MIN_FINAL_CHANGE = MIN_CHANGE/2; //! Default for -spendzeroconfchange static const bool DEFAULT_SPEND_ZEROCONF_CHANGE = true; //! Default for -sendfreetransactions From 20449ef09edf8f4f51a3e531d947dd89973c2a31 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 5 Jan 2017 09:12:48 -0500 Subject: [PATCH 2/2] Don't overpay fee if we have selected new coins that result in a smaller transaction. On repeated calls to SelectCoins we try to meet the fee necessary for the last transaction, the new fee required might be smaller, so increase our change by the difference if we can. --- src/wallet/wallet.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index bdf6d3c7a6..2775f4def3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2534,8 +2534,25 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt return false; } - if (nFeeRet >= nFeeNeeded) + if (nFeeRet >= nFeeNeeded) { + // Reduce fee to only the needed amount if we have change + // output to increase. This prevents potential overpayment + // in fees if the coins 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. + // TODO: The case where there is no change output remains + // to be addressed so we avoid creating too small an output. + if (nFeeRet > nFeeNeeded && nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) { + CAmount extraFeePaid = nFeeRet - nFeeNeeded; + vector::iterator change_position = txNew.vout.begin()+nChangePosInOut; + change_position->nValue += extraFeePaid; + nFeeRet -= extraFeePaid; + } break; // Done, enough fee included. + } // Try to reduce change to include necessary fee if (nChangePosInOut != -1 && nSubtractFeeFromAmount == 0) {