From 995b9f385b935e4e9b9fa46e82f642204cc85cba Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 5 Jan 2016 13:10:19 -0500 Subject: [PATCH 1/3] Always respect GetRequiredFee for wallet txs --- src/wallet/wallet.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 444bd88f8b289..244bc37e766e8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2223,14 +2223,9 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarge if (nFeeNeeded == 0) { int estimateFoundTarget = nConfirmTarget; nFeeNeeded = pool.estimateSmartFee(nConfirmTarget, &estimateFoundTarget).GetFee(nTxBytes); - // ... unless we don't have enough mempool data for our desired target - // so we make sure we're paying at least minTxFee - if (nFeeNeeded == 0 || (unsigned int)estimateFoundTarget > nConfirmTarget) - nFeeNeeded = std::max(nFeeNeeded, GetRequiredFee(nTxBytes)); - } - // prevent user from paying a non-sense fee (like 1 satoshi): 0 < fee < minRelayFee - if (nFeeNeeded < ::minRelayTxFee.GetFee(nTxBytes)) - nFeeNeeded = ::minRelayTxFee.GetFee(nTxBytes); + } + // prevent user from paying a fee below minRelayTxFee or minTxFee + nFeeNeeded = std::max(nFeeNeeded, GetRequiredFee(nTxBytes)); // But always obey the maximum if (nFeeNeeded > maxTxFee) nFeeNeeded = maxTxFee; From e420a1b15e3be8c9d862173d9d554563405b34a7 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 5 Jan 2016 13:11:34 -0500 Subject: [PATCH 2/3] Add sane fallback for fee estimation Add new commandline option "-fallbackfee" to use when fee estimation does not have sufficient data. --- src/init.cpp | 11 +++++++++++ src/qt/sendcoinsdialog.cpp | 6 ++++-- src/wallet/wallet.cpp | 9 +++++++++ src/wallet/wallet.h | 3 +++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index c768ca75be769..ba85a7972e521 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -393,6 +393,8 @@ std::string HelpMessage(HelpMessageMode mode) strUsage += HelpMessageGroup(_("Wallet options:")); strUsage += HelpMessageOpt("-disablewallet", _("Do not load the wallet and disable wallet RPC calls")); strUsage += HelpMessageOpt("-keypool=", strprintf(_("Set key pool size to (default: %u)"), DEFAULT_KEYPOOL_SIZE)); + strUsage += HelpMessageOpt("-fallbackfee=", strprintf(_("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data (default: %s)"), + CURRENCY_UNIT, FormatMoney(DEFAULT_FALLBACK_FEE))); strUsage += HelpMessageOpt("-mintxfee=", strprintf(_("Fees (in %s/kB) smaller than this are considered zero fee for transaction creation (default: %s)"), CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE))); strUsage += HelpMessageOpt("-paytxfee=", strprintf(_("Fee (in %s/kB) to add to transactions you send (default: %s)"), @@ -947,6 +949,15 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) else return InitError(strprintf(_("Invalid amount for -mintxfee=: '%s'"), mapArgs["-mintxfee"])); } + if (mapArgs.count("-fallbackfee")) + { + CAmount nFeePerK = 0; + if (!ParseMoney(mapArgs["-fallbackfee"], nFeePerK)) + return InitError(strprintf(_("Invalid amount for -fallbackfee=: '%s'"), mapArgs["-fallbackfee"])); + if (nFeePerK > nHighTransactionFeeWarning) + InitWarning(_("-fallbackfee is set very high! This is the transaction fee you may pay when fee estimates are not available.")); + CWallet::fallbackFee = CFeeRate(nFeePerK); + } if (mapArgs.count("-paytxfee")) { CAmount nFeePerK = 0; diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp index 31c9028c4b5af..c834c3b564618 100644 --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -640,13 +640,15 @@ void SendCoinsDialog::updateSmartFeeLabel() CFeeRate feeRate = mempool.estimateSmartFee(nBlocksToConfirm, &estimateFoundAtBlocks); if (feeRate <= CFeeRate(0)) // not enough data => minfee { - ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), CWallet::GetRequiredFee(1000)) + "/kB"); + ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), + std::max(CWallet::fallbackFee.GetFeePerK(), CWallet::GetRequiredFee(1000))) + "/kB"); ui->labelSmartFee2->show(); // (Smart fee not initialized yet. This usually takes a few blocks...) ui->labelFeeEstimation->setText(""); } else { - ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), feeRate.GetFeePerK()) + "/kB"); + ui->labelSmartFee->setText(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), + std::max(feeRate.GetFeePerK(), CWallet::GetRequiredFee(1000))) + "/kB"); ui->labelSmartFee2->hide(); ui->labelFeeEstimation->setText(tr("Estimated to begin confirmation within %n block(s).", "", estimateFoundAtBlocks)); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 244bc37e766e8..581c688fc7775 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -47,6 +47,12 @@ bool fSendFreeTransactions = DEFAULT_SEND_FREE_TRANSACTIONS; * Override with -mintxfee */ CFeeRate CWallet::minTxFee = CFeeRate(DEFAULT_TRANSACTION_MINFEE); +/** + * If fee estimation does not have enough data to provide estimates, use this fee instead. + * Has no effect if not using fee estimation + * Override with -fallbackfee + */ +CFeeRate CWallet::fallbackFee = CFeeRate(DEFAULT_FALLBACK_FEE); /** @defgroup mapWallet * @@ -2223,6 +2229,9 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarge if (nFeeNeeded == 0) { int estimateFoundTarget = nConfirmTarget; nFeeNeeded = pool.estimateSmartFee(nConfirmTarget, &estimateFoundTarget).GetFee(nTxBytes); + // ... unless we don't have enough mempool data for estimatefee, then use fallbackFee + if (nFeeNeeded == 0) + nFeeNeeded = fallbackFee.GetFee(nTxBytes); } // prevent user from paying a fee below minRelayTxFee or minTxFee nFeeNeeded = std::max(nFeeNeeded, GetRequiredFee(nTxBytes)); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 53a2b96690a0a..ce57f4dae140c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -41,6 +41,8 @@ static const unsigned int DEFAULT_KEYPOOL_SIZE = 100; static const CAmount DEFAULT_TRANSACTION_FEE = 0; //! -paytxfee will warn if called with a higher fee than this amount (in satoshis) per KB static const CAmount nHighTransactionFeeWarning = 0.01 * COIN; +//! -fallbackfee default +static const CAmount DEFAULT_FALLBACK_FEE = 20000; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; //! -maxtxfee default @@ -666,6 +668,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool AddAccountingEntry(const CAccountingEntry&, CWalletDB & pwalletdb); static CFeeRate minTxFee; + static CFeeRate fallbackFee; /** * Estimate the minimum fee considering user set parameters * and the required fee From bebe58b748532157958f9055e4517e280684006c Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Tue, 5 Jan 2016 17:47:04 -0500 Subject: [PATCH 3/3] SQUASHME: Fix rpc tests that assumed fallback to minRelayTxFee --- qa/rpc-tests/fundrawtransaction.py | 9 +++++++++ qa/rpc-tests/mempool_limit.py | 2 ++ 2 files changed, 11 insertions(+) diff --git a/qa/rpc-tests/fundrawtransaction.py b/qa/rpc-tests/fundrawtransaction.py index d6493dbb8a8fb..dda9166151b27 100755 --- a/qa/rpc-tests/fundrawtransaction.py +++ b/qa/rpc-tests/fundrawtransaction.py @@ -30,6 +30,11 @@ def run_test(self): print "Mining blocks..." min_relay_tx_fee = self.nodes[0].getnetworkinfo()['relayfee'] + # This test is not meant to test fee estimation and we'd like + # to be sure all txs are sent at a consistent desired feerate + for node in self.nodes: + node.settxfee(min_relay_tx_fee) + # if the fee's positive delta is higher than this value tests will fail, # neg. delta always fail the tests. # The size of the signature of every input may be at most 2 bytes larger @@ -447,6 +452,10 @@ def run_test(self): wait_bitcoinds() self.nodes = start_nodes(4, self.options.tmpdir) + # This test is not meant to test fee estimation and we'd like + # to be sure all txs are sent at a consistent desired feerate + for node in self.nodes: + node.settxfee(min_relay_tx_fee) connect_nodes_bi(self.nodes,0,1) connect_nodes_bi(self.nodes,1,2) diff --git a/qa/rpc-tests/mempool_limit.py b/qa/rpc-tests/mempool_limit.py index 3ba17ac4f1c73..a8cf6360ee1db 100755 --- a/qa/rpc-tests/mempool_limit.py +++ b/qa/rpc-tests/mempool_limit.py @@ -33,7 +33,9 @@ def run_test(self): inputs = [{ "txid" : us0["txid"], "vout" : us0["vout"]}] outputs = {self.nodes[0].getnewaddress() : 0.0001} tx = self.nodes[0].createrawtransaction(inputs, outputs) + self.nodes[0].settxfee(self.relayfee) # specifically fund this tx with low fee txF = self.nodes[0].fundrawtransaction(tx) + self.nodes[0].settxfee(0) # return to automatic fee selection txFS = self.nodes[0].signrawtransaction(txF['hex']) txid = self.nodes[0].sendrawtransaction(txFS['hex']) self.nodes[0].lockunspent(True, [us0])