diff --git a/src/qt/coincontroldialog.cpp b/src/qt/coincontroldialog.cpp index c19420beb5..7d8ef6571f 100644 --- a/src/qt/coincontroldialog.cpp +++ b/src/qt/coincontroldialog.cpp @@ -512,7 +512,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) nBytes -= 34; // Fee - nPayFee = CWallet::GetMinimumFee(nBytes, coinControl->nConfirmTarget, ::mempool, ::feeEstimator, nullptr /* FeeCalculation */, false /* ignoreGlobalPayTxFee */, conservative_estimate); + nPayFee = CWallet::GetMinimumFee(nBytes, *coinControl, ::mempool, ::feeEstimator, nullptr /* FeeCalculation */); if (nPayAmount > 0) { @@ -587,7 +587,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog) if (payTxFee.GetFeePerK() > 0) dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), payTxFee.GetFeePerK()) / 1000; else { - dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(coinControl->nConfirmTarget, NULL, ::mempool, conservative_estimate).GetFeePerK()) / 1000; + dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(*coinControl->m_confirm_target, NULL, ::mempool, conservative_estimate).GetFeePerK()) / 1000; } QString toolTip4 = tr("Can vary +/- %1 satoshi(s) per input.").arg(dFeeVary); diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 1a07d466bf..92478fcac6 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -274,11 +274,11 @@ void SendCoinsDialog::on_sendButton_clicked() CCoinControl ctrl; if (model->getOptionsModel()->getCoinControlFeatures()) ctrl = *CoinControlDialog::coinControl; - if (ui->radioSmartFee->isChecked()) - ctrl.nConfirmTarget = getConfTargetForIndex(ui->confTargetSelector->currentIndex()); - else - ctrl.nConfirmTarget = 0; - + if (ui->radioSmartFee->isChecked()) { + ctrl.m_confirm_target = getConfTargetForIndex(ui->confTargetSelector->currentIndex()); + } else { + ctrl.m_confirm_target = boost::none; + } ctrl.signalRbf = ui->optInRBF->isChecked(); prepareStatus = model->prepareTransaction(currentTransaction, ctrl); @@ -848,9 +848,9 @@ void SendCoinsDialog::coinControlUpdateLabels() CoinControlDialog::payAmounts.clear(); CoinControlDialog::fSubtractFeeFromAmount = false; if (ui->radioSmartFee->isChecked()) { - CoinControlDialog::coinControl->nConfirmTarget = getConfTargetForIndex(ui->confTargetSelector->currentIndex()); + CoinControlDialog::coinControl->m_confirm_target = getConfTargetForIndex(ui->confTargetSelector->currentIndex()); } else { - CoinControlDialog::coinControl->nConfirmTarget = model->getDefaultConfirmTarget(); + CoinControlDialog::coinControl->m_confirm_target = boost::none; } CoinControlDialog::coinControl->signalRbf = ui->optInRBF->isChecked(); diff --git a/src/qt/walletmodel.cpp b/src/qt/walletmodel.cpp index 3f90860cc1..ba0e1da0c7 100644 --- a/src/qt/walletmodel.cpp +++ b/src/qt/walletmodel.cpp @@ -24,6 +24,7 @@ #include "sync.h" #include "ui_interface.h" #include "util.h" // for GetBoolArg +#include "wallet/coincontrol.h" #include "wallet/feebumper.h" #include "wallet/wallet.h" #include "wallet/walletdb.h" // for BackupWallet @@ -667,8 +668,10 @@ bool WalletModel::bumpFee(uint256 hash) { std::unique_ptr feeBump; { + CCoinControl coin_control; + coin_control.signalRbf = true; LOCK2(cs_main, wallet->cs_wallet); - feeBump.reset(new CFeeBumper(wallet, hash, nTxConfirmTarget, false, 0, true, FeeEstimateMode::UNSET)); + feeBump.reset(new CFeeBumper(wallet, hash, coin_control, 0)); } if (feeBump->getResult() != BumpFeeResult::OK) { diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index bdd01bec12..40c8b764b9 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -10,6 +10,8 @@ #include "primitives/transaction.h" #include "wallet/wallet.h" +#include + /** Coin Control Features. */ class CCoinControl { @@ -19,12 +21,12 @@ public: bool fAllowOtherInputs; //! Includes watch only addresses which match the ISMINE_WATCH_SOLVABLE criteria bool fAllowWatchOnly; - //! Override estimated feerate + //! Override automatic min/max checks on fee, m_feerate must be set if true bool fOverrideFeeRate; - //! Feerate to use if overrideFeeRate is true - CFeeRate nFeeRate; - //! Override the default confirmation target, 0 = use default - int nConfirmTarget; + //! Override the default payTxFee if set + boost::optional m_feerate; + //! Override the default confirmation target if set + boost::optional m_confirm_target; //! Signal BIP-125 replace by fee. bool signalRbf; //! Fee estimation mode to control arguments to estimateSmartFee @@ -41,9 +43,9 @@ public: fAllowOtherInputs = false; fAllowWatchOnly = false; setSelected.clear(); - nFeeRate = CFeeRate(0); + m_feerate = boost::none; fOverrideFeeRate = false; - nConfirmTarget = 0; + m_confirm_target = boost::none; signalRbf = fWalletRbf; m_fee_mode = FeeEstimateMode::UNSET; } diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 607ecf4182..4bfd8726a5 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "consensus/validation.h" +#include "wallet/coincontrol.h" #include "wallet/feebumper.h" #include "wallet/wallet.h" #include "policy/fees.h" @@ -66,7 +67,7 @@ bool CFeeBumper::preconditionChecks(const CWallet *pWallet, const CWalletTx& wtx return true; } -CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConfirmTarget, bool ignoreGlobalPayTxFee, CAmount totalFee, bool newTxReplaceable, FeeEstimateMode fee_mode) +CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoinControl& coin_control, CAmount totalFee) : txid(std::move(txidIn)), nOldFee(0), @@ -165,8 +166,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConf nNewFee = totalFee; nNewFeeRate = CFeeRate(totalFee, maxNewTxSize); } else { - bool conservative_estimate = CalculateEstimateType(fee_mode, newTxReplaceable); - nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, ::feeEstimator, nullptr /* FeeCalculation */, ignoreGlobalPayTxFee, conservative_estimate); + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, coin_control, mempool, ::feeEstimator, nullptr /* FeeCalculation */); nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize); // New fee rate must be at least old rate + minimum incremental relay rate @@ -221,7 +221,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConf } // Mark new tx not replaceable, if requested. - if (!newTxReplaceable) { + if (!coin_control.signalRbf) { for (auto& input : mtx.vin) { if (input.nSequence < 0xfffffffe) input.nSequence = 0xfffffffe; } diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h index 11e2f5f953..3d64e53c15 100644 --- a/src/wallet/feebumper.h +++ b/src/wallet/feebumper.h @@ -10,6 +10,7 @@ class CWallet; class CWalletTx; class uint256; +class CCoinControl; enum class FeeEstimateMode; enum class BumpFeeResult @@ -25,7 +26,7 @@ enum class BumpFeeResult class CFeeBumper { public: - CFeeBumper(const CWallet *pWalletIn, const uint256 txidIn, int newConfirmTarget, bool ignoreGlobalPayTxFee, CAmount totalFee, bool newTxReplaceable, FeeEstimateMode fee_mode); + CFeeBumper(const CWallet *pWalletIn, const uint256 txidIn, const CCoinControl& coin_control, CAmount totalFee); BumpFeeResult getResult() const { return currentResult; } const std::vector& getErrors() const { return vErrors; } CAmount getOldFee() const { return nOldFee; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 1916908922..c636fa8114 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -460,7 +460,7 @@ UniValue sendtoaddress(const JSONRPCRequest& request) } if (request.params.size() > 6 && !request.params[6].isNull()) { - coin_control.nConfirmTarget = request.params[6].get_int(); + coin_control.m_confirm_target = request.params[6].get_int(); } if (request.params.size() > 7 && !request.params[7].isNull()) { @@ -981,7 +981,7 @@ UniValue sendmany(const JSONRPCRequest& request) } if (request.params.size() > 6 && !request.params[6].isNull()) { - coin_control.nConfirmTarget = request.params[6].get_int(); + coin_control.m_confirm_target = request.params[6].get_int(); } if (request.params.size() > 7 && !request.params[7].isNull()) { @@ -2730,13 +2730,9 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) RPCTypeCheck(request.params, {UniValue::VSTR}); CCoinControl coinControl; - coinControl.destChange = CNoDestination(); int changePosition = -1; - coinControl.fAllowWatchOnly = false; // include watching bool lockUnspents = false; bool reserveChangeKey = true; - coinControl.nFeeRate = CFeeRate(0); - coinControl.fOverrideFeeRate = false; UniValue subtractFeeFromOutputs; std::set setSubtractFeeFromOutputs; @@ -2788,7 +2784,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) if (options.exists("feeRate")) { - coinControl.nFeeRate = CFeeRate(AmountFromValue(options["feeRate"])); + coinControl.m_feerate = CFeeRate(AmountFromValue(options["feeRate"])); coinControl.fOverrideFeeRate = true; } @@ -2799,7 +2795,7 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) coinControl.signalRbf = options["replaceable"].get_bool(); } if (options.exists("conf_target")) { - coinControl.nConfirmTarget = options["conf_target"].get_int(); + coinControl.m_confirm_target = options["conf_target"].get_int(); } if (options.exists("estimate_mode")) { if (!FeeModeFromString(options["estimate_mode"].get_str(), coinControl.m_fee_mode)) { @@ -2905,11 +2901,9 @@ UniValue bumpfee(const JSONRPCRequest& request) hash.SetHex(request.params[0].get_str()); // optional parameters - bool ignoreGlobalPayTxFee = false; - int newConfirmTarget = nTxConfirmTarget; CAmount totalFee = 0; - bool replaceable = true; - FeeEstimateMode fee_mode = FeeEstimateMode::UNSET; + CCoinControl coin_control; + coin_control.signalRbf = true; if (request.params.size() > 1) { UniValue options = request.params[1]; RPCTypeCheckObj(options, @@ -2924,14 +2918,11 @@ UniValue bumpfee(const JSONRPCRequest& request) if (options.exists("confTarget") && options.exists("totalFee")) { throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget and totalFee options should not both be set. Please provide either a confirmation target for fee estimation or an explicit total fee for the transaction."); } else if (options.exists("confTarget")) { - // If the user has explicitly set a confTarget in this rpc call, - // then override the default logic that uses the global payTxFee - // instead of the confirmation target. - ignoreGlobalPayTxFee = true; - newConfirmTarget = options["confTarget"].get_int(); - if (newConfirmTarget <= 0) { // upper-bound will be checked by estimatefee/smartfee + int target = options["confTarget"].get_int(); + if (target <= 0) { // FIXME: Check upper bound too throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid confTarget (cannot be <= 0)"); } + coin_control.m_confirm_target = target; } else if (options.exists("totalFee")) { totalFee = options["totalFee"].get_int64(); if (totalFee <= 0) { @@ -2940,10 +2931,10 @@ UniValue bumpfee(const JSONRPCRequest& request) } if (options.exists("replaceable")) { - replaceable = options["replaceable"].get_bool(); + coin_control.signalRbf = options["replaceable"].get_bool(); } if (options.exists("estimate_mode")) { - if (!FeeModeFromString(options["estimate_mode"].get_str(), fee_mode)) { + if (!FeeModeFromString(options["estimate_mode"].get_str(), coin_control.m_fee_mode)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid estimate_mode parameter"); } } @@ -2952,7 +2943,7 @@ UniValue bumpfee(const JSONRPCRequest& request) LOCK2(cs_main, pwallet->cs_wallet); EnsureWalletIsUnlocked(pwallet); - CFeeBumper feeBump(pwallet, hash, newConfirmTarget, ignoreGlobalPayTxFee, totalFee, replaceable, fee_mode); + CFeeBumper feeBump(pwallet, hash, coin_control, totalFee); BumpFeeResult res = feeBump.getResult(); if (res != BumpFeeResult::OK) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index f69ae5268c..f010ff0ca1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2721,17 +2721,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT vin.scriptWitness.SetNull(); } - // Allow to override the default confirmation target over the CoinControl instance - int currentConfirmationTarget = nTxConfirmTarget; - if (coin_control.nConfirmTarget > 0) - currentConfirmationTarget = coin_control.nConfirmTarget; - - // Allow to override the default fee estimate mode over the CoinControl instance - bool conservative_estimate = CalculateEstimateType(coin_control.m_fee_mode, coin_control.signalRbf); - - CAmount nFeeNeeded = GetMinimumFee(nBytes, currentConfirmationTarget, ::mempool, ::feeEstimator, &feeCalc, false /* ignoreGlobalPayTxFee */, conservative_estimate); - if (coin_control.fOverrideFeeRate) - nFeeNeeded = coin_control.nFeeRate.GetFee(nBytes); + CAmount nFeeNeeded = GetMinimumFee(nBytes, coin_control, ::mempool, ::feeEstimator, &feeCalc); // If we made it here and we aren't even able to meet the relay fee on the next pass, give up // because we must be at the maximum allowed fee. @@ -2756,7 +2746,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CWalletT // new inputs. We now know we only need the smaller fee // (because of reduced tx size) and so we should add a // change output. Only try this once. - CAmount fee_needed_for_change = GetMinimumFee(change_prototype_size, currentConfirmationTarget, ::mempool, ::feeEstimator, nullptr, false /* ignoreGlobalPayTxFee */, conservative_estimate); + CAmount fee_needed_for_change = GetMinimumFee(change_prototype_size, coin_control, ::mempool, ::feeEstimator, nullptr); CAmount minimum_value_for_change = GetDustThreshold(change_prototype_txout, ::dustRelayFee); CAmount max_excess_fee = fee_needed_for_change + minimum_value_for_change; if (nFeeRet > nFeeNeeded + max_excess_fee && nChangePosInOut == -1 && nSubtractFeeFromAmount == 0 && pick_new_inputs) { @@ -2932,33 +2922,52 @@ CAmount CWallet::GetRequiredFee(unsigned int nTxBytes) return std::max(minTxFee.GetFee(nTxBytes), ::minRelayTxFee.GetFee(nTxBytes)); } -CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc, bool ignoreGlobalPayTxFee, bool conservative_estimate) +CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc) { - // payTxFee is the user-set global for desired feerate - CAmount nFeeNeeded = payTxFee.GetFee(nTxBytes); - // User didn't set: use -txconfirmtarget to estimate... - if (nFeeNeeded == 0 || ignoreGlobalPayTxFee) { - nFeeNeeded = estimator.estimateSmartFee(nConfirmTarget, feeCalc, pool, conservative_estimate).GetFee(nTxBytes); - // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee - if (nFeeNeeded == 0) { - nFeeNeeded = fallbackFee.GetFee(nTxBytes); - if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; - } - } else { + /* User control of how to calculate fee uses the following parameter precedence: + 1. coin_control.m_feerate + 2. coin_control.m_confirm_target + 3. payTxFee (user-set global variable) + 4. nTxConfirmTarget (user-set global variable) + The first parameter that is set is used. + */ + CAmount fee_needed; + if (coin_control.m_feerate) { // 1. + fee_needed = coin_control.m_feerate->GetFee(nTxBytes); + if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; + // Allow to override automatic min/max check over coin control instance + if (coin_control.fOverrideFeeRate) return fee_needed; + } + else if (!coin_control.m_confirm_target && ::payTxFee != CFeeRate(0)) { // 3. TODO: remove magic value of 0 for global payTxFee + fee_needed = ::payTxFee.GetFee(nTxBytes); if (feeCalc) feeCalc->reason = FeeReason::PAYTXFEE; } + else { // 2. or 4. + // We will use smart fee estimation + unsigned int target = coin_control.m_confirm_target ? *coin_control.m_confirm_target : ::nTxConfirmTarget; + // Allow to override the default fee estimate mode over the CoinControl instance + bool conservative_estimate = CalculateEstimateType(coin_control.m_fee_mode, coin_control.signalRbf); + + fee_needed = estimator.estimateSmartFee(target, feeCalc, pool, conservative_estimate).GetFee(nTxBytes); + if (fee_needed == 0) { + // if we don't have enough data for estimateSmartFee, then use fallbackFee + fee_needed = fallbackFee.GetFee(nTxBytes); + if (feeCalc) feeCalc->reason = FeeReason::FALLBACK; + } + } + // prevent user from paying a fee below minRelayTxFee or minTxFee - CAmount requiredFee = GetRequiredFee(nTxBytes); - if (requiredFee > nFeeNeeded) { - nFeeNeeded = requiredFee; + CAmount required_fee = GetRequiredFee(nTxBytes); + if (required_fee > fee_needed) { + fee_needed = required_fee; if (feeCalc) feeCalc->reason = FeeReason::REQUIRED; } // But always obey the maximum - if (nFeeNeeded > maxTxFee) { - nFeeNeeded = maxTxFee; + if (fee_needed > maxTxFee) { + fee_needed = maxTxFee; if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE; } - return nFeeNeeded; + return fee_needed; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index d8717ea179..3c866776fb 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -964,7 +964,7 @@ public: * Estimate the minimum fee considering user set parameters * and the required fee */ - static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc, bool ignoreGlobalPayTxFee, bool conservative_estimate); + static CAmount GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_control, const CTxMemPool& pool, const CBlockPolicyEstimator& estimator, FeeCalculation *feeCalc); /** * Return the minimum required fee taking into account the * floating relay fee and user set minimum transaction fee