diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -3,3 +3,4 @@ This release includes the following features and fixes: + - Deprecate nblocks parameter in `estimatefee`. See `bitcoin-cli help estimatefee` for more info. diff --git a/src/qt/sendcoinsdialog.cpp b/src/qt/sendcoinsdialog.cpp --- a/src/qt/sendcoinsdialog.cpp +++ b/src/qt/sendcoinsdialog.cpp @@ -782,9 +782,7 @@ return; } - int nBlocksToConfirm = - getConfTargetForIndex(ui->confTargetSelector->currentIndex()); - CFeeRate feeRate = g_mempool.estimateFee(nBlocksToConfirm); + CFeeRate feeRate = g_mempool.estimateFee(); // not enough data => minfee if (feeRate <= CFeeRate(Amount::zero())) { ui->labelSmartFee->setText( @@ -805,8 +803,7 @@ "/kB"); ui->labelSmartFee2->hide(); ui->labelFeeEstimation->setText( - tr("Estimated to begin confirmation within %n block(s).", "", - nBlocksToConfirm)); + tr("Estimated to begin confirmation by next block.")); } updateFeeMinimizedLabel(); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -109,7 +109,6 @@ {"pruneblockchain", 0, "height"}, {"keypoolrefill", 0, "newsize"}, {"getrawmempool", 0, "verbose"}, - {"estimatefee", 0, "nblocks"}, {"prioritisetransaction", 1, "priority_delta"}, {"prioritisetransaction", 2, "fee_delta"}, {"setban", 2, "bantime"}, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -774,41 +774,29 @@ static UniValue estimatefee(const Config &config, const JSONRPCRequest &request) { - if (request.fHelp || request.params.size() != 1) { + if (request.fHelp || request.params.size() > 1) { throw std::runtime_error( - "estimatefee nblocks\n" + "estimatefee\n" "\nEstimates the approximate fee per kilobyte needed for a " - "transaction to begin\n" - "confirmation within nblocks blocks.\n" - "\nArguments:\n" - "1. nblocks (numeric, required)\n" + "transaction\n" "\nResult:\n" "n (numeric) estimated fee-per-kilobyte\n" - "\n" - "A negative value is returned if not enough transactions and " - "blocks\n" - "have been observed to make an estimate.\n" - "-1 is always returned for nblocks == 1 as it is impossible to " - "calculate\n" - "a fee that is high enough to get reliably included in the next " - "block.\n" "\nExample:\n" + - HelpExampleCli("estimatefee", "6")); + HelpExampleCli("estimatefee", "")); } - RPCTypeCheck(request.params, {UniValue::VNUM}); - - int nBlocks = request.params[0].get_int(); - if (nBlocks < 1) { - nBlocks = 1; - } - - CFeeRate feeRate = g_mempool.estimateFee(nBlocks); - if (feeRate == CFeeRate(Amount::zero())) { - return -1.0; + if ((request.params.size() == 1) && + !IsDeprecatedRPCEnabled(gArgs, "estimatefee")) { + // FIXME: Remove this message in 0.20 + throw JSONRPCError( + RPC_METHOD_DEPRECATED, + "estimatefee with the nblocks argument is no longer supported\n" + "Please call estimatefee with no arguments instead.\n" + "\nExample:\n" + + HelpExampleCli("estimatefee", "")); } - return ValueFromAmount(feeRate.GetFeePerK()); + return ValueFromAmount(g_mempool.estimateFee().GetFeePerK()); } // clang-format off @@ -823,7 +811,7 @@ {"generating", "generatetoaddress", generatetoaddress, {"nblocks", "address", "maxtries"}}, - {"util", "estimatefee", estimatefee, {"nblocks"}}, + {"util", "estimatefee", estimatefee, {}}, }; // clang-format on diff --git a/src/test/policyestimator_tests.cpp b/src/test/policyestimator_tests.cpp --- a/src/test/policyestimator_tests.cpp +++ b/src/test/policyestimator_tests.cpp @@ -56,11 +56,8 @@ // Check that the estimate is above the rolling minimum fee. This should // be true since we have not trimmed the mempool. - BOOST_CHECK(CFeeRate(Amount::zero()) == mpool.estimateFee(1)); - BOOST_CHECK(mpool.GetMinFee(1) <= mpool.estimateFee(2)); - BOOST_CHECK(mpool.GetMinFee(1) <= mpool.estimateFee(3)); - BOOST_CHECK(mpool.GetMinFee(1) <= mpool.estimateFee(4)); - BOOST_CHECK(mpool.GetMinFee(1) <= mpool.estimateFee(5)); + BOOST_CHECK(CFeeRate(Amount::zero()) == mpool.estimateFee()); + BOOST_CHECK(mpool.GetMinFee(1) <= mpool.estimateFee()); // Check that estimateFee returns the minimum rolling fee even when the // mempool grows very quickly and no blocks have been mined. @@ -98,10 +95,8 @@ CFeeRate(10000 * DEFAULT_BLOCK_MIN_TX_FEE_PER_KB, CTransaction(tx).GetTotalSize())); - for (int i = 1; i < 10; i++) { - BOOST_CHECK_MESSAGE(mpool.estimateFee(i) == mpool.GetMinFee(1), - "Confirm blocks has failed on iteration " << i); - } + BOOST_CHECK_MESSAGE(mpool.estimateFee() == mpool.GetMinFee(1), + "Confirm blocks has failed"); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/txmempool.h b/src/txmempool.h --- a/src/txmempool.h +++ b/src/txmempool.h @@ -743,7 +743,7 @@ TxMempoolInfo info(const uint256 &hash) const; std::vector infoAll() const; - CFeeRate estimateFee(int nBlocks) const; + CFeeRate estimateFee() const; size_t DynamicMemoryUsage() const; diff --git a/src/txmempool.cpp b/src/txmempool.cpp --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -950,7 +950,7 @@ return GetInfo(i); } -CFeeRate CTxMemPool::estimateFee(int nBlocks) const { +CFeeRate CTxMemPool::estimateFee() const { LOCK(cs); uint64_t maxMempoolSize = diff --git a/src/wallet/fees.cpp b/src/wallet/fees.cpp --- a/src/wallet/fees.cpp +++ b/src/wallet/fees.cpp @@ -18,7 +18,7 @@ Amount nFeeNeeded = targetFee; // User didn't set: use -txconfirmtarget to estimate... if (nFeeNeeded == Amount::zero()) { - nFeeNeeded = pool.estimateFee(nConfirmTarget).GetFeeCeiling(nTxBytes); + nFeeNeeded = pool.estimateFee().GetFeeCeiling(nTxBytes); // ... unless we don't have enough mempool data for estimatefee, then // use fallbackFee. if (nFeeNeeded == Amount::zero()) { diff --git a/test/functional/rpc_estimatefee.py b/test/functional/rpc_estimatefee.py new file mode 100755 --- /dev/null +++ b/test/functional/rpc_estimatefee.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +# Copyright (c) 2019 The Bitcoin developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from decimal import Decimal +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_raises_rpc_error + + +class EstimateFeeTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 3 + self.extra_args = [[], ["-minrelaytxfee=0.001"], + ["-deprecatedrpc=estimatefee"]] + + def run_test(self): + for i in range(5): + self.nodes[0].generate(1) + + # estimatefee is 0.00001 by default, regardless of block contents + assert_equal(self.nodes[0].estimatefee(), Decimal('0.00001')) + assert_equal(self.nodes[2].estimatefee(), Decimal('0.00001')) + + # estimatefee may be different for nodes that set it in their config + assert_equal(self.nodes[1].estimatefee(), Decimal('0.001')) + + # nblocks arg is no longer supported. Make sure the error message + # indicates this. + assert_raises_rpc_error(-32, "estimatefee with the nblocks argument is no longer supported", + self.nodes[0].estimatefee, 1) + + # When marked as deprecated, estimatfee with nblocks set succeeds + assert_equal(self.nodes[2].estimatefee(1), Decimal('0.00001')) + + +if __name__ == '__main__': + EstimateFeeTest().main()