More economical fee estimates for RBF and RPC options to control #10589

Merged
merged 5 commits into from Jul 11, 2017
View
@@ -36,6 +36,20 @@ std::string StringForFeeReason(FeeReason reason) {
return reason_string->second;
}
+bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode) {
+ static const std::map<std::string, FeeEstimateMode> fee_modes = {
+ {"UNSET", FeeEstimateMode::UNSET},
+ {"ECONOMICAL", FeeEstimateMode::ECONOMICAL},
+ {"CONSERVATIVE", FeeEstimateMode::CONSERVATIVE},
+ };
+ auto mode = fee_modes.find(mode_string);
+
+ if (mode == fee_modes.end()) return false;
+
+ fee_estimate_mode = mode->second;
+ return true;
+}
+
/**
* We will instantiate an instance of this class to track transactions that were
* included in a block. We will lump transactions into a bucket according to their
View
@@ -90,6 +90,15 @@ enum class FeeReason {
std::string StringForFeeReason(FeeReason reason);
+/* Used to determine type of fee estimation requested */
+enum class FeeEstimateMode {
+ UNSET, //! Use default settings based on other criteria
+ ECONOMICAL, //! Force estimateSmartFee to use non-conservative estimates
+ CONSERVATIVE, //! Force estimateSmartFee to use conservative estimates
+};
+
+bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode);
+
/* Used to return detailed information about a feerate bucket */
struct EstimatorBucket
{
@@ -197,7 +206,7 @@ class CBlockPolicyEstimator
* the closest target where one can be given. 'conservative' estimates are
* valid over longer time horizons also.
*/
- CFeeRate estimateSmartFee(int confTarget, FeeCalculation *feeCalc, const CTxMemPool& pool, bool conservative = true) const;
+ CFeeRate estimateSmartFee(int confTarget, FeeCalculation *feeCalc, const CTxMemPool& pool, bool conservative) const;
/** Return a specific fee estimate calculation with a given success
* threshold and time horizon, and optionally return detailed data about
@@ -490,6 +490,8 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
else nBytesInputs += 148;
}
+ bool conservative_estimate = CalculateEstimateType(FeeEstimateMode::UNSET, coinControl->signalRbf);
+
// calculation
if (nQuantity > 0)
{
@@ -510,7 +512,7 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
nBytes -= 34;
// Fee
- nPayFee = CWallet::GetMinimumFee(nBytes, coinControl->nConfirmTarget, ::mempool, ::feeEstimator);
+ nPayFee = CWallet::GetMinimumFee(nBytes, coinControl->nConfirmTarget, ::mempool, ::feeEstimator, nullptr /* FeeCalculation */, false /* ignoreGlobalPayTxFee */, conservative_estimate);
if (nPayAmount > 0)
{
@@ -585,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).GetFeePerK()) / 1000;
+ dFeeVary = (double)std::max(CWallet::GetRequiredFee(1000), ::feeEstimator.estimateSmartFee(coinControl->nConfirmTarget, NULL, ::mempool, conservative_estimate).GetFeePerK()) / 1000;
}
QString toolTip4 = tr("Can vary +/- %1 satoshi(s) per input.").arg(dFeeVary);
@@ -166,6 +166,8 @@ void SendCoinsDialog::setModel(WalletModel *_model)
connect(ui->checkBoxMinimumFee, SIGNAL(stateChanged(int)), this, SLOT(updateFeeSectionControls()));
connect(ui->checkBoxMinimumFee, SIGNAL(stateChanged(int)), this, SLOT(updateGlobalFeeVariables()));
connect(ui->checkBoxMinimumFee, SIGNAL(stateChanged(int)), this, SLOT(coinControlUpdateLabels()));
+ connect(ui->optInRBF, SIGNAL(stateChanged(int)), this, SLOT(updateSmartFeeLabel()));
+ connect(ui->optInRBF, SIGNAL(stateChanged(int)), this, SLOT(coinControlUpdateLabels()));
@TheBlueMatt

TheBlueMatt Jul 9, 2017

Contributor

Cant comment there, but I believe you also need to move the default setting of optInRBF up a bunch here (about 10 lines below this (set default rbf checkbox state, or does that setting automagically result in calling these registrations?).

@morcos

morcos Jul 10, 2017

Contributor

Not sure if I understand your question (or how QT works) but I'm pretty sure it's correct as is. Calling setCheckState below will trigger the signal if the checkbox changed from its initial value.

@promag

promag Jul 10, 2017

Contributor

Either way, what about make an explicit call to coinControlUpdateLabels()?

@morcos

morcos Jul 10, 2017

Contributor

I don't think this is necessary. It's analogous to setting the default sliderSmartFee done in the same section of code.

ui->customFee->setSingleStep(CWallet::GetRequiredFee(1000));
updateFeeSectionControls();
updateMinFeeLabel();
@@ -652,7 +654,8 @@ void SendCoinsDialog::updateSmartFeeLabel()
int nBlocksToConfirm = ui->sliderSmartFee->maximum() - ui->sliderSmartFee->value() + 2;
FeeCalculation feeCalc;
- CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocksToConfirm, &feeCalc, ::mempool);
+ bool conservative_estimate = CalculateEstimateType(FeeEstimateMode::UNSET, ui->optInRBF->isChecked());
+ CFeeRate feeRate = ::feeEstimator.estimateSmartFee(nBlocksToConfirm, &feeCalc, ::mempool, conservative_estimate);
if (feeRate <= CFeeRate(0)) // not enough data => minfee
{
ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(),
@@ -827,6 +830,7 @@ void SendCoinsDialog::coinControlUpdateLabels()
} else {
CoinControlDialog::coinControl->nConfirmTarget = model->getDefaultConfirmTarget();
}
+ CoinControlDialog::coinControl->signalRbf = ui->optInRBF->isChecked();
for(int i = 0; i < ui->entries->count(); ++i)
{
View
@@ -19,6 +19,7 @@
#include "keystore.h"
#include "validation.h"
#include "net.h" // for g_connman
+#include "policy/fees.h"
#include "policy/rbf.h"
#include "sync.h"
#include "ui_interface.h"
@@ -667,7 +668,7 @@ bool WalletModel::bumpFee(uint256 hash)
std::unique_ptr<CFeeBumper> feeBump;
{
LOCK2(cs_main, wallet->cs_wallet);
- feeBump.reset(new CFeeBumper(wallet, hash, nTxConfirmTarget, false, 0, true));
+ feeBump.reset(new CFeeBumper(wallet, hash, nTxConfirmTarget, false, 0, true, FeeEstimateMode::UNSET));
}
if (feeBump->getResult() != BumpFeeResult::OK)
{
View
@@ -37,6 +37,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "getnetworkhashps", 1, "height" },
{ "sendtoaddress", 1, "amount" },
{ "sendtoaddress", 4, "subtractfeefromamount" },
+ { "sendtoaddress", 5 , "replaceable" },
+ { "sendtoaddress", 6 , "conf_target" },
{ "settxfee", 0, "amount" },
{ "getreceivedbyaddress", 1, "minconf" },
{ "getreceivedbyaccount", 1, "minconf" },
@@ -69,6 +71,8 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "sendmany", 1, "amounts" },
{ "sendmany", 2, "minconf" },
{ "sendmany", 4, "subtractfeefrom" },
+ { "sendmany", 5 , "replaceable" },
+ { "sendmany", 6 , "conf_target" },
{ "addmultisigaddress", 0, "nrequired" },
{ "addmultisigaddress", 1, "keys" },
{ "createmultisig", 0, "nrequired" },
@@ -184,8 +184,8 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
mpool.TrimToSize(1);
BOOST_CHECK(mpool.GetMinFee(1).GetFeePerK() > feeV[5]);
for (int i = 1; i < 10; i++) {
- BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool).GetFeePerK() >= feeEst.estimateRawFee(i, 0.85, FeeEstimateHorizon::MED_HALFLIFE).GetFeePerK());
- BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool).GetFeePerK() >= mpool.GetMinFee(1).GetFeePerK());
+ BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool, true).GetFeePerK() >= feeEst.estimateRawFee(i, 0.85, FeeEstimateHorizon::MED_HALFLIFE).GetFeePerK());
+ BOOST_CHECK(feeEst.estimateSmartFee(i, NULL, mpool, true).GetFeePerK() >= mpool.GetMinFee(1).GetFeePerK());
}
}
View
@@ -6,6 +6,7 @@
#define BITCOIN_WALLET_COINCONTROL_H
#include "policy/feerate.h"
+#include "policy/fees.h"
#include "primitives/transaction.h"
#include "wallet/wallet.h"
@@ -26,6 +27,8 @@ class CCoinControl
int nConfirmTarget;
//! Signal BIP-125 replace by fee.
bool signalRbf;
+ //! Fee estimation mode to control arguments to estimateSmartFee
+ FeeEstimateMode m_fee_mode;
@promag

promag Jul 10, 2017

Contributor

m_fee_estimate_mode?

CCoinControl()
{
@@ -42,6 +45,7 @@ class CCoinControl
fOverrideFeeRate = false;
nConfirmTarget = 0;
signalRbf = fWalletRbf;
+ m_fee_mode = FeeEstimateMode::UNSET;
}
bool HasSelected() const
View
@@ -66,7 +66,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)
+CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConfirmTarget, bool ignoreGlobalPayTxFee, CAmount totalFee, bool newTxReplaceable, FeeEstimateMode fee_mode)
:
txid(std::move(txidIn)),
nOldFee(0),
@@ -165,7 +165,8 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConf
nNewFee = totalFee;
nNewFeeRate = CFeeRate(totalFee, maxNewTxSize);
} else {
- nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, ::feeEstimator, nullptr, ignoreGlobalPayTxFee);
+ bool conservative_estimate = CalculateEstimateType(fee_mode, newTxReplaceable);
+ nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, ::feeEstimator, nullptr /* FeeCalculation */, ignoreGlobalPayTxFee, conservative_estimate);
nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize);
// New fee rate must be at least old rate + minimum incremental relay rate
View
@@ -10,6 +10,7 @@
class CWallet;
class CWalletTx;
class uint256;
+enum class FeeEstimateMode;
enum class BumpFeeResult
{
@@ -24,7 +25,7 @@ enum class BumpFeeResult
class CFeeBumper
{
public:
- CFeeBumper(const CWallet *pWalletIn, const uint256 txidIn, int newConfirmTarget, bool ignoreGlobalPayTxFee, CAmount totalFee, bool newTxReplaceable);
+ CFeeBumper(const CWallet *pWalletIn, const uint256 txidIn, int newConfirmTarget, bool ignoreGlobalPayTxFee, CAmount totalFee, bool newTxReplaceable, FeeEstimateMode fee_mode);
BumpFeeResult getResult() const { return currentResult; }
const std::vector<std::string>& getErrors() const { return vErrors; }
CAmount getOldFee() const { return nOldFee; }
Oops, something went wrong.