From c2612273ef0b3ef127ba03c87f2a3303f0b9d299 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 29 Jul 2021 03:54:17 +0000 Subject: [PATCH 01/16] RPC: Support specifying different types for param aliases --- src/rpc/util.cpp | 22 +++++++++++++++------- src/rpc/util.h | 20 ++++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index a11366bd47..3ccb1032c4 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -793,9 +793,15 @@ UniValue RPCHelpMan::GetArgMap() const for (int i{0}; i < int(m_args.size()); ++i) { const auto& arg = m_args.at(i); std::vector arg_names = SplitString(arg.m_names, '|'); + RPCArg::Type argtype = arg.m_type; + size_t arg_num = 0; for (const auto& arg_name : arg_names) { - push_back_arg_info(m_name, i, arg_name, arg.m_type); - if (arg.m_type == RPCArg::Type::OBJ_NAMED_PARAMS) { + if (!arg.m_type_per_name.empty()) { + argtype = arg.m_type_per_name.at(arg_num++); + } + + push_back_arg_info(m_name, i, arg_name, argtype); + if (argtype == RPCArg::Type::OBJ_NAMED_PARAMS) { for (const auto& inner : arg.m_inner) { std::vector inner_names = SplitString(inner.m_names, '|'); for (const std::string& inner_name : inner_names) { @@ -846,13 +852,15 @@ UniValue RPCArg::MatchesType(const UniValue& request) const { if (m_opts.skip_type_check) return true; if (IsOptional() && request.isNull()) return true; - const auto exp_type{ExpectedType(m_type)}; - if (!exp_type) return true; // nothing to check + for (auto type : m_type_per_name.empty() ? std::vector{m_type} : m_type_per_name) { + const auto exp_type{ExpectedType(type)}; + if (!exp_type) return true; // nothing to check - if (*exp_type != request.getType()) { - return strprintf("JSON value of type %s is not of expected type %s", uvTypeName(request.getType()), uvTypeName(*exp_type)); + if (*exp_type == request.getType()) { + return true; + } } - return true; + return strprintf("JSON value of type %s is not of expected type %s", uvTypeName(request.getType()), uvTypeName(*ExpectedType(m_type))); } std::string RPCArg::GetFirstName() const diff --git a/src/rpc/util.h b/src/rpc/util.h index 392540ffad..5a5f638b66 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -202,6 +203,7 @@ struct RPCArg { const std::string m_names; //!< The name of the arg (can be empty for inner args, can contain multiple aliases separated by | for named request arguments) const Type m_type; + const std::vector m_type_per_name; const std::vector m_inner; //!< Only used for arrays or dicts const Fallback m_fallback; const std::string m_description; @@ -222,6 +224,24 @@ struct RPCArg { CHECK_NONFATAL(type != Type::ARR && type != Type::OBJ && type != Type::OBJ_NAMED_PARAMS && type != Type::OBJ_USER_KEYS); } + RPCArg( + std::string name, + std::vector types, + Fallback fallback, + std::string description, + std::vector inner = {}, + RPCArgOptions opts = {}) + : m_names{std::move(name)}, + m_type{types.at(0)}, + m_type_per_name{std::move(types)}, + m_inner{std::move(inner)}, + m_fallback{std::move(fallback)}, + m_description{std::move(description)}, + m_opts{std::move(opts)} + { + CHECK_NONFATAL(m_type_per_name.size() == size_t(std::count(m_names.begin(), m_names.end(), '|')) + 1); + } + RPCArg( std::string name, Type type, From c7560421eb615d20d05567003af853943f531ebd Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Sat, 6 Mar 2021 20:30:17 +0100 Subject: [PATCH 02/16] Introduce fee histogram in getmempoolinfo RPC command Co-authored-by: Jonas Schnelli Co-authored-by: Jon Atack Github-Pull: #21422 Rebased-From: d242aa52de7e09ee756e56fe1d6f8941ffd48f06 --- src/rest.cpp | 3 +- src/rpc/client.cpp | 1 + src/rpc/mempool.cpp | 103 +++++++++++++++++++++++++++++++++++++++++--- src/rpc/mempool.h | 9 +++- 4 files changed, 108 insertions(+), 8 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index c42bc8e40c..5153b1733b 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -33,6 +33,7 @@ #include #include +#include #include #include @@ -686,7 +687,7 @@ static bool rest_mempool(const std::any& context, HTTPRequest* req, const std::s } str_json = MempoolToJSON(*mempool, verbose, mempool_sequence).write() + "\n"; } else { - str_json = MempoolInfoToJSON(*mempool).write() + "\n"; + str_json = MempoolInfoToJSON(*mempool, std::nullopt).write() + "\n"; } req->WriteHeader("Content-Type", "application/json"); diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index b866fa484b..c9bf70cdb0 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -250,6 +250,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getblockstats", 1, "stats" }, { "pruneblockchain", 0, "height" }, { "keypoolrefill", 0, "newsize" }, + { "getmempoolinfo", 0, "fee_histogram" }, { "getrawmempool", 0, "verbose" }, { "getrawmempool", 1, "mempool_sequence" }, { "estimatesmartfee", 0, "conf_target" }, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index d61898260b..e4f60d05fb 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -25,6 +26,7 @@ #include #include +#include #include using node::DumpMempool; @@ -664,7 +666,7 @@ static RPCHelpMan gettxspendingprevout() }; } -UniValue MempoolInfoToJSON(const CTxMemPool& pool) +UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional& histogram_floors) { // Make sure this call is atomic in the pool. LOCK(pool.cs); @@ -680,14 +682,64 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) ret.pushKV("incrementalrelayfee", ValueFromAmount(pool.m_opts.incremental_relay_feerate.GetFeePerK())); ret.pushKV("unbroadcastcount", uint64_t{pool.GetUnbroadcastTxs().size()}); ret.pushKV("fullrbf", pool.m_opts.full_rbf); + + if (histogram_floors) { + const MempoolHistogramFeeRates& floors{histogram_floors.value()}; + + std::vector sizes(floors.size(), 0); + std::vector count(floors.size(), 0); + std::vector fees(floors.size(), 0); + + for (const CTxMemPoolEntry& e : pool.mapTx) { + const CAmount fee{e.GetFee()}; + const uint32_t size{uint32_t(e.GetTxSize())}; + const CAmount fee_rate{CFeeRate{fee, size}.GetFee(1)}; + + // Distribute fee rates + for (size_t i = floors.size(); i-- > 0;) { + if (fee_rate >= floors[i]) { + sizes[i] += size; + ++count[i]; + fees[i] += fee; + break; + } + } + } + + // Track total amount of available fees in fee rate groups + CAmount total_fees = 0; + UniValue groups(UniValue::VOBJ); + for (size_t i = 0; i < floors.size(); ++i) { + UniValue info_sub(UniValue::VOBJ); + info_sub.pushKV("size", sizes.at(i)); + info_sub.pushKV("count", count.at(i)); + info_sub.pushKV("fees", fees.at(i)); + info_sub.pushKV("from", floors.at(i)); + + total_fees += fees.at(i); + groups.pushKV(ToString(floors.at(i)), info_sub); + } + + UniValue info(UniValue::VOBJ); + info.pushKV("fee_rate_groups", groups); + info.pushKV("total_fees", total_fees); + ret.pushKV("fee_histogram", info); + } + return ret; } static RPCHelpMan getmempoolinfo() { return RPCHelpMan{"getmempoolinfo", - "Returns details on the active state of the TX memory pool.", - {}, + "Returns details on the active state of the TX memory pool.\n", + { + {"fee_histogram", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Fee statistics grouped by fee rate ranges", + { + {"fee_rate", RPCArg::Type::NUM, RPCArg::Optional::NO, "Fee rate (in " + CURRENCY_ATOM + "/vB) to group the fees by"}, + }, + }, + }, RPCResult{ RPCResult::Type::OBJ, "", "", { @@ -702,14 +754,53 @@ static RPCHelpMan getmempoolinfo() {RPCResult::Type::NUM, "incrementalrelayfee", "minimum fee rate increment for mempool limiting or replacement in " + CURRENCY_UNIT + "/kvB"}, {RPCResult::Type::NUM, "unbroadcastcount", "Current number of transactions that haven't passed initial broadcast yet"}, {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection"}, + {RPCResult::Type::OBJ, "fee_histogram", /*optional=*/true, "", + { + {RPCResult::Type::OBJ_DYN, "fee_rate_groups", "", + { + {RPCResult::Type::OBJ, "", "Fee rate group named by its lower bound (in " + CURRENCY_ATOM + "/vB), identical to the \"from\" field below", + { + {RPCResult::Type::NUM, "size", "Cumulative size of all transactions in the fee rate group (in vBytes)"}, + {RPCResult::Type::NUM, "count", "Number of transactions in the fee rate group"}, + {RPCResult::Type::NUM, "fees", "Cumulative fees of all transactions in the fee rate group (in " + CURRENCY_ATOM + ")"}, + {RPCResult::Type::NUM, "from", "Group contains transactions with fee rates equal or greater than this value (in " + CURRENCY_ATOM + "/vB)"}, + }}}}, + {RPCResult::Type::NUM, "total_fees", "Total available fees in mempool (in " + CURRENCY_ATOM + ")"}, + }}, }}, RPCExamples{ - HelpExampleCli("getmempoolinfo", "") - + HelpExampleRpc("getmempoolinfo", "") + HelpExampleCli("getmempoolinfo", "") + + HelpExampleCli("getmempoolinfo", R"("[0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 12, 14, 17, 20, 25, 30, 40, 50, 60, 70, 80, 100, 120, 140, 170, 200]")") + + HelpExampleRpc("getmempoolinfo", "") + + HelpExampleRpc("getmempoolinfo", R"([0, 1, 2, 3, 4, 5, 6, 7, 8, 10, 12, 14, 17, 20, 25, 30, 40, 50, 60, 70, 80, 100, 120, 140, 170, 200])") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - return MempoolInfoToJSON(EnsureAnyMemPool(request.context)); + MempoolHistogramFeeRates histogram_floors; + std::optional histogram_floors_opt = std::nullopt; + + if (!request.params[0].isNull()) { + const UniValue histogram_floors_univalue = request.params[0].get_array(); + + if (histogram_floors_univalue.empty()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid number of parameters"); + } + + for (size_t i = 0; i < histogram_floors_univalue.size(); ++i) { + int64_t value = histogram_floors_univalue[i].getInt(); + + if (value < 0) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Non-negative values are expected"); + } else if (i > 0 && histogram_floors.back() >= value) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Strictly increasing values are expected"); + } + + histogram_floors.push_back(value); + } + histogram_floors_opt = std::optional(std::move(histogram_floors)); + } + + return MempoolInfoToJSON(EnsureAnyMemPool(request.context), histogram_floors_opt); }, }; } diff --git a/src/rpc/mempool.h b/src/rpc/mempool.h index 229d7d52dd..5feb6089af 100644 --- a/src/rpc/mempool.h +++ b/src/rpc/mempool.h @@ -5,11 +5,18 @@ #ifndef BITCOIN_RPC_MEMPOOL_H #define BITCOIN_RPC_MEMPOOL_H +#include + +#include +#include + class CTxMemPool; class UniValue; +typedef std::vector MempoolHistogramFeeRates; + /** Mempool information to JSON */ -UniValue MempoolInfoToJSON(const CTxMemPool& pool); +UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional& histogram_floors); /** Mempool to JSON */ UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose = false, bool include_mempool_sequence = false); From 1d566c067c62a7d5819eff31a01ce6cd29c7e7df Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Sat, 6 Mar 2021 22:54:05 +0100 Subject: [PATCH 03/16] test: Add mempool fee histogram test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit: https://github.com/bitcoin/bitcoin/commit/0b6ba66238c377116bc6c21e19cffbf1b6dfc788 Co-authored-by: João Barbosa Co-authored-by: Jon Atack Github-Pull: #21422 Rebased-From: c5e53d0d21ff166ee9ab21f043d839aa112fcf96 --- test/functional/mempool_fee_histogram.py | 164 +++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 165 insertions(+) create mode 100755 test/functional/mempool_fee_histogram.py diff --git a/test/functional/mempool_fee_histogram.py b/test/functional/mempool_fee_histogram.py new file mode 100755 index 0000000000..fbda9610d4 --- /dev/null +++ b/test/functional/mempool_fee_histogram.py @@ -0,0 +1,164 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test mempool fee histogram.""" + +from decimal import Decimal + +from test_framework.blocktools import COINBASE_MATURITY +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + assert_greater_than, + assert_greater_than_or_equal, +) + +class MempoolFeeHistogramTest(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser) + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + node = self.nodes[0] + self.generate(self.nodes[0], COINBASE_MATURITY + 2, sync_fun=self.no_op) + + # We have two UTXOs (utxo_1 and utxo_2) and we create three changeless transactions: + # - tx1 (5 sat/vB): spending utxo_1 + # - tx2 (14 sat/vB): spending output from tx1 + # - tx3 (6 sat/vB): spending utxo_2 and the output from tx2 + + self.log.info("Test getmempoolinfo does not return fee histogram by default") + assert ("fee_histogram" not in node.getmempoolinfo()) + + self.log.info("Test getmempoolinfo returns empty fee histogram when mempool is empty") + info = node.getmempoolinfo([1, 2, 3]) + + (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) + assert_equal(0, non_empty_groups) + assert_equal(3, empty_groups) + assert_equal(0, total_fees) + + for i in ['1', '2', '3']: + assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['size']) + assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['count']) + assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['fees']) + assert_equal(int(i), info['fee_histogram']['fee_rate_groups'][i]['from']) + + self.log.info("Test that we have two spendable UTXOs and lock the second one") + utxos = node.listunspent() + assert_equal(2, len(utxos)) + node.lockunspent(False, [{"txid": utxos[1]["txid"], "vout": utxos[1]["vout"]}]) + + self.log.info("Send tx1 transaction with 5 sat/vB fee rate") + node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("50.0"), fee_rate=5, subtractfeefromamount=True) + + self.log.info("Test fee rate histogram when mempool contains 1 transaction (tx1: 5 sat/vB)") + info = node.getmempoolinfo([1, 3, 5, 10]) + (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) + assert_equal(1, non_empty_groups) + assert_equal(3, empty_groups) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) + assert_equal(total_fees, info['fee_histogram']['total_fees']) + + assert_equal(0, info['fee_histogram']['fee_rate_groups']['1']['size']) + assert_equal(0, info['fee_histogram']['fee_rate_groups']['1']['count']) + assert_equal(0, info['fee_histogram']['fee_rate_groups']['1']['fees']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['1']['from']) + + assert_equal(0, info['fee_histogram']['fee_rate_groups']['3']['size']) + assert_equal(0, info['fee_histogram']['fee_rate_groups']['3']['count']) + assert_equal(0, info['fee_histogram']['fee_rate_groups']['3']['fees']) + assert_equal(3, info['fee_histogram']['fee_rate_groups']['3']['from']) + + assert_equal(188, info['fee_histogram']['fee_rate_groups']['5']['size']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) + assert_equal(940, info['fee_histogram']['fee_rate_groups']['5']['fees']) + assert_equal(5, info['fee_histogram']['fee_rate_groups']['5']['from']) + + assert_equal(0, info['fee_histogram']['fee_rate_groups']['10']['size']) + assert_equal(0, info['fee_histogram']['fee_rate_groups']['10']['count']) + assert_equal(0, info['fee_histogram']['fee_rate_groups']['10']['fees']) + assert_equal(10, info['fee_histogram']['fee_rate_groups']['10']['from']) + + self.log.info("Send tx2 transaction with 14 sat/vB fee rate (spends tx1 UTXO)") + node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("25.0"), fee_rate=14, subtractfeefromamount=True) + + self.log.info("Test fee rate histogram when mempool contains 2 transactions (tx1: 5 sat/vB, tx2: 14 sat/vB)") + info = node.getmempoolinfo([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]) + + # Verify that tx1 and tx2 are reported in 5 sat/vB and 14 sat/vB in fee rate groups respectively + (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) + assert_equal(2, non_empty_groups) + assert_equal(13, empty_groups) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['14']['count']) + assert_equal(total_fees, info['fee_histogram']['total_fees']) + + # Unlock the second UTXO which we locked + node.lockunspent(True, [{"txid": utxos[1]["txid"], "vout": utxos[1]["vout"]}]) + + self.log.info("Send tx3 transaction with 6 sat/vB fee rate (spends all available UTXOs)") + node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("99.9"), fee_rate=6, subtractfeefromamount=True) + + self.log.info("Test fee rate histogram when mempool contains 3 transactions (tx1: 5 sat/vB, tx2: 14 sat/vB, tx3: 6 sat/vB)") + info = node.getmempoolinfo([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]) + + # Verify that each of 5, 6 and 14 sat/vB fee rate groups contain one transaction + (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) + assert_equal(3, non_empty_groups) + assert_equal(12, empty_groups) + + for i in ['1', '2', '3', '4', '7', '8', '9', '10', '11', '12', '13', '15']: + assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['size']) + assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['count']) + assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['fees']) + assert_equal(int(i), info['fee_histogram']['fee_rate_groups'][i]['from']) + + assert_equal(188, info['fee_histogram']['fee_rate_groups']['5']['size']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) + assert_equal(940, info['fee_histogram']['fee_rate_groups']['5']['fees']) + assert_equal(5, info['fee_histogram']['fee_rate_groups']['5']['from']) + + assert_equal(356, info['fee_histogram']['fee_rate_groups']['6']['size']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['6']['count']) + assert_equal(2136, info['fee_histogram']['fee_rate_groups']['6']['fees']) + assert_equal(6, info['fee_histogram']['fee_rate_groups']['6']['from']) + + assert_equal(141, info['fee_histogram']['fee_rate_groups']['14']['size']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['14']['count']) + assert_equal(1974, info['fee_histogram']['fee_rate_groups']['14']['fees']) + assert_equal(14, info['fee_histogram']['fee_rate_groups']['14']['from']) + + assert_equal(total_fees, info['fee_histogram']['total_fees']) + + def histogram_stats(self, histogram): + total_fees = 0 + empty_count = 0 + non_empty_count = 0 + + for key, bin in histogram['fee_rate_groups'].items(): + assert_equal(int(key), bin['from']) + if bin['fees'] > 0: + assert_greater_than(bin['count'], 0) + else: + assert_equal(bin['count'], 0) + assert_greater_than_or_equal(bin['fees'], 0) + assert_greater_than_or_equal(bin['size'], 0) + total_fees += bin['fees'] + + if bin['count'] == 0: + empty_count += 1 + else: + non_empty_count += 1 + + return (non_empty_count, empty_count, total_fees) + +if __name__ == '__main__': + MempoolFeeHistogramTest(__file__).main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index b85bf1c668..087dbb6588 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -304,6 +304,7 @@ BASE_SCRIPTS = [ 'p2p_initial_headers_sync.py', 'feature_nulldummy.py', 'mempool_accept.py', + 'mempool_fee_histogram.py', 'mempool_expiry.py', 'wallet_import_with_label.py --legacy-wallet', 'wallet_importdescriptors.py --descriptors', From ecd567ae6367f36fbe1928c0e94cdd36cbae539d Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Tue, 16 Apr 2019 12:55:59 +0200 Subject: [PATCH 04/16] RPC/blockchain: Consider ancestor, descendant, and combined fee rates for histogram in getmempoolinfo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test changes from: https://github.com/bitcoin/bitcoin/commit/0b6ba66238c377116bc6c21e19cffbf1b6dfc788 Co-authored-by: João Barbosa Co-authored-by: Jon Atack --- src/rpc/mempool.cpp | 13 ++++++++- test/functional/mempool_fee_histogram.py | 37 ++++++++++++------------ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index e4f60d05fb..3ad1ffbf21 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -693,7 +693,18 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional 0;) { diff --git a/test/functional/mempool_fee_histogram.py b/test/functional/mempool_fee_histogram.py index fbda9610d4..133e188777 100755 --- a/test/functional/mempool_fee_histogram.py +++ b/test/functional/mempool_fee_histogram.py @@ -93,12 +93,11 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): self.log.info("Test fee rate histogram when mempool contains 2 transactions (tx1: 5 sat/vB, tx2: 14 sat/vB)") info = node.getmempoolinfo([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]) - # Verify that tx1 and tx2 are reported in 5 sat/vB and 14 sat/vB in fee rate groups respectively + # Verify that both tx1 and tx2 are reported in 8 sat/vB fee rate group (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) - assert_equal(2, non_empty_groups) - assert_equal(13, empty_groups) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['14']['count']) + assert_equal(1, non_empty_groups) + assert_equal(14, empty_groups) + assert_equal(2, info['fee_histogram']['fee_rate_groups']['8']['count']) assert_equal(total_fees, info['fee_histogram']['total_fees']) # Unlock the second UTXO which we locked @@ -110,31 +109,31 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): self.log.info("Test fee rate histogram when mempool contains 3 transactions (tx1: 5 sat/vB, tx2: 14 sat/vB, tx3: 6 sat/vB)") info = node.getmempoolinfo([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]) - # Verify that each of 5, 6 and 14 sat/vB fee rate groups contain one transaction + # Verify that each of 6, 7 and 8 sat/vB fee rate groups contain one transaction (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) assert_equal(3, non_empty_groups) assert_equal(12, empty_groups) - for i in ['1', '2', '3', '4', '7', '8', '9', '10', '11', '12', '13', '15']: + for i in ['1', '2', '3', '4', '5', '9', '10', '11', '12', '13', '14', '15']: assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['size']) assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['count']) assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['fees']) assert_equal(int(i), info['fee_histogram']['fee_rate_groups'][i]['from']) - assert_equal(188, info['fee_histogram']['fee_rate_groups']['5']['size']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) - assert_equal(940, info['fee_histogram']['fee_rate_groups']['5']['fees']) - assert_equal(5, info['fee_histogram']['fee_rate_groups']['5']['from']) - - assert_equal(356, info['fee_histogram']['fee_rate_groups']['6']['size']) + assert_equal(188, info['fee_histogram']['fee_rate_groups']['6']['size']) assert_equal(1, info['fee_histogram']['fee_rate_groups']['6']['count']) - assert_equal(2136, info['fee_histogram']['fee_rate_groups']['6']['fees']) - assert_equal(6, info['fee_histogram']['fee_rate_groups']['6']['from']) + assert_equal(940, info['fee_histogram']['fee_rate_groups']['6']['fees']) + assert_equal(5, info['fee_histogram']['fee_rate_groups']['6']['from']) - assert_equal(141, info['fee_histogram']['fee_rate_groups']['14']['size']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['14']['count']) - assert_equal(1974, info['fee_histogram']['fee_rate_groups']['14']['fees']) - assert_equal(14, info['fee_histogram']['fee_rate_groups']['14']['from']) + assert_equal(356, info['fee_histogram']['fee_rate_groups']['7']['size']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['7']['count']) + assert_equal(2136, info['fee_histogram']['fee_rate_groups']['7']['fees']) + assert_equal(6, info['fee_histogram']['fee_rate_groups']['7']['from']) + + assert_equal(141, info['fee_histogram']['fee_rate_groups']['8']['size']) + assert_equal(1, info['fee_histogram']['fee_rate_groups']['8']['count']) + assert_equal(1974, info['fee_histogram']['fee_rate_groups']['8']['fees']) + assert_equal(14, info['fee_histogram']['fee_rate_groups']['8']['from']) assert_equal(total_fees, info['fee_histogram']['total_fees']) From eb93c8bf4b3a1e731ab3d09b03622213e12bfbf8 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Tue, 1 Mar 2022 07:26:16 +0000 Subject: [PATCH 05/16] Bugfix: QA: Ensure mempool_fee_histogram can adapt to feerate rounding correctly Caution: This implementation is for a post-#22949 codebase --- test/functional/mempool_fee_histogram.py | 89 ++++++++++++++---------- 1 file changed, 54 insertions(+), 35 deletions(-) diff --git a/test/functional/mempool_fee_histogram.py b/test/functional/mempool_fee_histogram.py index 133e188777..71acbc6893 100755 --- a/test/functional/mempool_fee_histogram.py +++ b/test/functional/mempool_fee_histogram.py @@ -7,13 +7,28 @@ from decimal import Decimal from test_framework.blocktools import COINBASE_MATURITY +from test_framework.messages import ( + COIN, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_greater_than, assert_greater_than_or_equal, + ceildiv, ) +def get_actual_fee_rate(fee_in_satoshis, vsize): + fee_rate = ceildiv(fee_in_satoshis, vsize) + return str(fee_rate) + +def get_tx_details(node, txid): + info = node.gettransaction(txid=txid) + info.update(node.getrawtransaction(txid=txid, verbose=True)) + info['fee'] = int(-info['fee'] * COIN) # convert to satoshis + info['feerate'] = get_actual_fee_rate(info['fee'], info['vsize']) + return info + class MempoolFeeHistogramTest(BitcoinTestFramework): def add_options(self, parser): self.add_wallet_options(parser) @@ -57,14 +72,15 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): node.lockunspent(False, [{"txid": utxos[1]["txid"], "vout": utxos[1]["vout"]}]) self.log.info("Send tx1 transaction with 5 sat/vB fee rate") - node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("50.0"), fee_rate=5, subtractfeefromamount=True) + tx1_txid = node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("50.0"), fee_rate=5, subtractfeefromamount=True) + tx1_info = get_tx_details(node, tx1_txid) - self.log.info("Test fee rate histogram when mempool contains 1 transaction (tx1: 5 sat/vB)") + self.log.info(f"Test fee rate histogram when mempool contains 1 transaction (tx1: {tx1_info['feerate']} sat/vB)") info = node.getmempoolinfo([1, 3, 5, 10]) (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) assert_equal(1, non_empty_groups) assert_equal(3, empty_groups) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) + assert_equal(1, info['fee_histogram']['fee_rate_groups'][tx1_info['feerate']]['count']) assert_equal(total_fees, info['fee_histogram']['total_fees']) assert_equal(0, info['fee_histogram']['fee_rate_groups']['1']['size']) @@ -88,54 +104,57 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): assert_equal(10, info['fee_histogram']['fee_rate_groups']['10']['from']) self.log.info("Send tx2 transaction with 14 sat/vB fee rate (spends tx1 UTXO)") - node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("25.0"), fee_rate=14, subtractfeefromamount=True) + tx2_txid = node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("25.0"), fee_rate=14, subtractfeefromamount=True) + tx2_info = get_tx_details(node, tx2_txid) - self.log.info("Test fee rate histogram when mempool contains 2 transactions (tx1: 5 sat/vB, tx2: 14 sat/vB)") + self.log.info(f"Test fee rate histogram when mempool contains 2 transactions (tx1: {tx1_info['feerate']} sat/vB, tx2: {tx2_info['feerate']} sat/vB)") info = node.getmempoolinfo([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]) - # Verify that both tx1 and tx2 are reported in 8 sat/vB fee rate group + # Verify that both tx1 and tx2 are reported in 9 sat/vB fee rate group (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) + tx1p2_feerate = get_actual_fee_rate(tx1_info['fee'] + tx2_info['fee'], tx1_info['vsize'] + tx2_info['vsize']) assert_equal(1, non_empty_groups) assert_equal(14, empty_groups) - assert_equal(2, info['fee_histogram']['fee_rate_groups']['8']['count']) + assert_equal(2, info['fee_histogram']['fee_rate_groups'][tx1p2_feerate]['count']) assert_equal(total_fees, info['fee_histogram']['total_fees']) # Unlock the second UTXO which we locked node.lockunspent(True, [{"txid": utxos[1]["txid"], "vout": utxos[1]["vout"]}]) self.log.info("Send tx3 transaction with 6 sat/vB fee rate (spends all available UTXOs)") - node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("99.9"), fee_rate=6, subtractfeefromamount=True) + tx3_txid = node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("99.9"), fee_rate=6, subtractfeefromamount=True) + tx3_info = get_tx_details(node, tx3_txid) - self.log.info("Test fee rate histogram when mempool contains 3 transactions (tx1: 5 sat/vB, tx2: 14 sat/vB, tx3: 6 sat/vB)") + self.log.info(f"Test fee rate histogram when mempool contains 3 transactions (tx1: {tx1_info['feerate']} sat/vB, tx2: {tx2_info['feerate']} sat/vB, tx3: {tx3_info['feerate']} sat/vB)") info = node.getmempoolinfo([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]) - # Verify that each of 6, 7 and 8 sat/vB fee rate groups contain one transaction - (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) - assert_equal(3, non_empty_groups) - assert_equal(12, empty_groups) + # Verify that each of 6, 8 and 9 sat/vB fee rate groups contain one transaction + # tx1 should be grouped with tx2 + tx3 (descendants) + # tx2 should be grouped with tx1 (ancestors only) + # tx3 should be alone + expected_histogram = { + 'fee_rate_groups': dict( ( + (str(n), { + 'from': n, + 'count': 0, + 'fees': 0, + 'size': 0, + }) for n in range(1, 16) + ) ), + 'total_fees': tx1_info['fee'] + tx2_info['fee'] + tx3_info['fee'], + } + expected_frg = expected_histogram['fee_rate_groups'] + tx1p2p3_feerate = get_actual_fee_rate(expected_histogram['total_fees'], tx1_info['vsize'] + tx2_info['vsize'] + tx3_info['vsize']) + def inc_expected(feerate, txinfo): + this_frg = expected_frg[feerate] + this_frg['count'] += 1 + this_frg['fees'] += txinfo['fee'] + this_frg['size'] += txinfo['vsize'] + inc_expected(tx1p2p3_feerate, tx1_info) + inc_expected(tx1p2_feerate, tx2_info) + inc_expected(tx3_info['feerate'], tx3_info) - for i in ['1', '2', '3', '4', '5', '9', '10', '11', '12', '13', '14', '15']: - assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['size']) - assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['count']) - assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['fees']) - assert_equal(int(i), info['fee_histogram']['fee_rate_groups'][i]['from']) - - assert_equal(188, info['fee_histogram']['fee_rate_groups']['6']['size']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['6']['count']) - assert_equal(940, info['fee_histogram']['fee_rate_groups']['6']['fees']) - assert_equal(5, info['fee_histogram']['fee_rate_groups']['6']['from']) - - assert_equal(356, info['fee_histogram']['fee_rate_groups']['7']['size']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['7']['count']) - assert_equal(2136, info['fee_histogram']['fee_rate_groups']['7']['fees']) - assert_equal(6, info['fee_histogram']['fee_rate_groups']['7']['from']) - - assert_equal(141, info['fee_histogram']['fee_rate_groups']['8']['size']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['8']['count']) - assert_equal(1974, info['fee_histogram']['fee_rate_groups']['8']['fees']) - assert_equal(14, info['fee_histogram']['fee_rate_groups']['8']['from']) - - assert_equal(total_fees, info['fee_histogram']['total_fees']) + assert_equal(expected_histogram, info['fee_histogram']) def histogram_stats(self, histogram): total_fees = 0 From b9b320c02aaac8fb764cd92857edd76ce80409cd Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 6 Mar 2021 20:30:17 +0100 Subject: [PATCH 06/16] RPC/blockchain: getmempoolinfo: Enable specifying with_fee_histogram as a boolean to use a sensible default set of fee rate levels --- src/rpc/client.cpp | 1 + src/rpc/mempool.cpp | 8 ++++++-- src/rpc/mempool.h | 9 +++++++++ test/functional/mempool_fee_histogram.py | 22 ++++++++++++++++++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index c9bf70cdb0..2e1305b77e 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -251,6 +251,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "pruneblockchain", 0, "height" }, { "keypoolrefill", 0, "newsize" }, { "getmempoolinfo", 0, "fee_histogram" }, + { "getmempoolinfo", 0, "with_fee_histogram" }, { "getrawmempool", 0, "verbose" }, { "getrawmempool", 1, "mempool_sequence" }, { "estimatesmartfee", 0, "conf_target" }, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 3ad1ffbf21..7a98e7b826 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -745,7 +745,7 @@ static RPCHelpMan getmempoolinfo() return RPCHelpMan{"getmempoolinfo", "Returns details on the active state of the TX memory pool.\n", { - {"fee_histogram", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Fee statistics grouped by fee rate ranges", + {"fee_histogram|with_fee_histogram", {RPCArg::Type::ARR, RPCArg::Type::BOOL}, RPCArg::Optional::OMITTED, "Fee statistics grouped by fee rate ranges", { {"fee_rate", RPCArg::Type::NUM, RPCArg::Optional::NO, "Fee rate (in " + CURRENCY_ATOM + "/vB) to group the fees by"}, }, @@ -790,7 +790,11 @@ static RPCHelpMan getmempoolinfo() MempoolHistogramFeeRates histogram_floors; std::optional histogram_floors_opt = std::nullopt; - if (!request.params[0].isNull()) { + if (request.params[0].isBool()) { + if (request.params[0].isTrue()) { + histogram_floors_opt = MempoolInfoToJSON_const_histogram_floors; + } + } else if (!request.params[0].isNull()) { const UniValue histogram_floors_univalue = request.params[0].get_array(); if (histogram_floors_univalue.empty()) { diff --git a/src/rpc/mempool.h b/src/rpc/mempool.h index 5feb6089af..c775590b24 100644 --- a/src/rpc/mempool.h +++ b/src/rpc/mempool.h @@ -15,6 +15,15 @@ class UniValue; typedef std::vector MempoolHistogramFeeRates; +/* TODO: define log scale formular for dynamically creating the + * feelimits but with the property of not constantly changing + * (and thus screw up client implementations) */ +static const MempoolHistogramFeeRates MempoolInfoToJSON_const_histogram_floors{ + 1, 2, 3, 4, 5, 6, 7, 8, 10, + 12, 14, 17, 20, 25, 30, 40, 50, 60, 70, 80, 100, + 120, 140, 170, 200, 250, 300, 400, 500, 600, 700, 800, 1000, + 1200, 1400, 1700, 2000, 2500, 3000, 4000, 5000, 6000, 7000, 8000, 10000}; + /** Mempool information to JSON */ UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional& histogram_floors); diff --git a/test/functional/mempool_fee_histogram.py b/test/functional/mempool_fee_histogram.py index 71acbc6893..84f85f13c5 100755 --- a/test/functional/mempool_fee_histogram.py +++ b/test/functional/mempool_fee_histogram.py @@ -156,6 +156,28 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): assert_equal(expected_histogram, info['fee_histogram']) + self.log.info("Test fee rate histogram with default groups") + info = node.getmempoolinfo(with_fee_histogram=True) + + # Verify that the 6 sat/vB fee rate group has one transaction, and the 8-9 sat/vB fee rate group has two + for collapse_n in (9, 11, 13, 15): + for field in ('count', 'size', 'fees'): + expected_frg[str(collapse_n - 1)][field] += expected_frg[str(collapse_n)][field] + del expected_frg[str(collapse_n)] + + for new_n in (17, 20, 25) + tuple(range(30, 90, 10)) + (100, 120, 140, 170, 200, 250) + tuple(range(300, 900, 100)) + (1000, 1200, 1400, 1700, 2000, 2500) + tuple(range(3000, 9000, 1000)) + (10000,): + assert(info['fee_histogram']['fee_rate_groups'][str(new_n)] == { + 'from': new_n, + 'count': 0, + 'fees': 0, + 'size': 0, + }) + del info['fee_histogram']['fee_rate_groups'][str(new_n)] + assert_equal(expected_histogram, info['fee_histogram']) + + self.log.info("Test getmempoolinfo(with_fee_histogram=False) does not return fee histogram") + assert('fee_histogram' not in node.getmempoolinfo(with_fee_histogram=False)) + def histogram_stats(self, histogram): total_fees = 0 empty_count = 0 From 64e50cbb7dfca6c1954ee66052cf3bc795bf797d Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 9 Nov 2022 22:03:10 +0000 Subject: [PATCH 07/16] Add mempool/fee_histogram option to rest API --- src/rest.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rest.cpp b/src/rest.cpp index 5153b1733b..11f5f2703c 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -651,8 +651,8 @@ static bool rest_mempool(const std::any& context, HTTPRequest* req, const std::s std::string param; const RESTResponseFormat rf = ParseDataFormat(param, str_uri_part); - if (param != "contents" && param != "info") { - return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/mempool/.json"); + if (param != "contents" && param != "info" && param != "info/with_fee_histogram") { + return RESTERR(req, HTTP_BAD_REQUEST, "Invalid URI format. Expected /rest/mempool/.json"); } const CTxMemPool* mempool = GetMemPool(context, req); @@ -686,6 +686,8 @@ static bool rest_mempool(const std::any& context, HTTPRequest* req, const std::s return RESTERR(req, HTTP_BAD_REQUEST, "Verbose results cannot contain mempool sequence values. (hint: set \"verbose=false\")"); } str_json = MempoolToJSON(*mempool, verbose, mempool_sequence).write() + "\n"; + } else if (param == "info/with_fee_histogram") { + str_json = MempoolInfoToJSON(*mempool, MempoolInfoToJSON_const_histogram_floors).write() + "\n"; } else { str_json = MempoolInfoToJSON(*mempool, std::nullopt).write() + "\n"; } From 521f921d832f68e9e730c1627c5f116493c53f1f Mon Sep 17 00:00:00 2001 From: Kiminuo Date: Sat, 6 Mar 2021 20:30:17 +0100 Subject: [PATCH 08/16] RPC/mempool: Add "to" (end of range) field to fee histogram Co-authored-by: Jonas Schnelli Co-authored-by: Jon Atack Github-Pull: #21422 Rebased-From: 0b87ba9bc3a2ada2839af0e1af868fcd5ddb9155 --- src/rpc/mempool.cpp | 7 +++++++ test/functional/mempool_fee_histogram.py | 14 +++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 7a98e7b826..88931d6705 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -727,6 +727,12 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional frinfo['from'] + del frinfo['to'] + assert_equal(frinfo, { 'from': new_n, 'count': 0, 'fees': 0, @@ -191,6 +201,8 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): assert_equal(bin['count'], 0) assert_greater_than_or_equal(bin['fees'], 0) assert_greater_than_or_equal(bin['size'], 0) + if bin['to'] is not None: + assert_greater_than_or_equal(bin['to'], bin['from']) total_fees += bin['fees'] if bin['count'] == 0: From 68f7f13ad00deea9604ea3dad83b9cc9e2fe873c Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 2 Apr 2020 15:10:19 +0000 Subject: [PATCH 09/16] RPC/blockchain: getmempoolinfo: Return fee_histogram in older format (only) --- src/rpc/mempool.cpp | 34 +++----- test/functional/mempool_fee_histogram.py | 105 ++++++++++++----------- 2 files changed, 67 insertions(+), 72 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 88931d6705..90de1591f5 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -719,26 +719,17 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional::max() : floors[i + 1]); total_fees += fees.at(i); - groups.pushKV(ToString(floors.at(i)), info_sub); + info.pushKV(ToString(floors[i]), info_sub); } - - UniValue info(UniValue::VOBJ); - info.pushKV("fee_rate_groups", groups); info.pushKV("total_fees", total_fees); ret.pushKV("fee_histogram", info); } @@ -773,18 +764,17 @@ static RPCHelpMan getmempoolinfo() {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection"}, {RPCResult::Type::OBJ, "fee_histogram", /*optional=*/true, "", { - {RPCResult::Type::OBJ_DYN, "fee_rate_groups", "", - { - {RPCResult::Type::OBJ, "", "Fee rate group named by its lower bound (in " + CURRENCY_ATOM + "/vB), identical to the \"from\" field below", + {RPCResult::Type::OBJ, "", "Fee rate group named by its lower bound (in " + CURRENCY_ATOM + "/vB), identical to the \"from_feerate\" field below", { - {RPCResult::Type::NUM, "size", "Cumulative size of all transactions in the fee rate group (in vBytes)"}, + {RPCResult::Type::NUM, "sizes", "Cumulative size of all transactions in the fee rate group (in vBytes)"}, {RPCResult::Type::NUM, "count", "Number of transactions in the fee rate group"}, {RPCResult::Type::NUM, "fees", "Cumulative fees of all transactions in the fee rate group (in " + CURRENCY_ATOM + ")"}, - {RPCResult::Type::NUM, "from", "Group contains transactions with fee rates equal or greater than this value (in " + CURRENCY_ATOM + "/vB)"}, - {RPCResult::Type::ANY, "to", "Group contains transactions with fee rates equal or less than this value (in " + CURRENCY_ATOM + "/vB)"}, - }}}}, + {RPCResult::Type::NUM, "from_feerate", "Group contains transactions with fee rates equal or greater than this value (in " + CURRENCY_ATOM + "/vB)"}, + {RPCResult::Type::ANY, "to_feerate", "Group contains transactions with fee rates equal or less than this value (in " + CURRENCY_ATOM + "/vB)"}, + }}, + {RPCResult::Type::ELISION, "", ""}, {RPCResult::Type::NUM, "total_fees", "Total available fees in mempool (in " + CURRENCY_ATOM + ")"}, - }}, + }, /*skip_type_check=*/ true}, }}, RPCExamples{ HelpExampleCli("getmempoolinfo", "") + diff --git a/test/functional/mempool_fee_histogram.py b/test/functional/mempool_fee_histogram.py index 9c2de2bfd0..7b90a3ecaf 100755 --- a/test/functional/mempool_fee_histogram.py +++ b/test/functional/mempool_fee_histogram.py @@ -61,10 +61,10 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): assert_equal(0, total_fees) for i in ['1', '2', '3']: - assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['size']) - assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['count']) - assert_equal(0, info['fee_histogram']['fee_rate_groups'][i]['fees']) - assert_equal(int(i), info['fee_histogram']['fee_rate_groups'][i]['from']) + assert_equal(0, info['fee_histogram'][i]['sizes']) + assert_equal(0, info['fee_histogram'][i]['count']) + assert_equal(0, info['fee_histogram'][i]['fees']) + assert_equal(int(i), info['fee_histogram'][i]['from_feerate']) self.log.info("Test that we have two spendable UTXOs and lock the second one") utxos = node.listunspent() @@ -80,28 +80,28 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): (non_empty_groups, empty_groups, total_fees) = self.histogram_stats(info['fee_histogram']) assert_equal(1, non_empty_groups) assert_equal(3, empty_groups) - assert_equal(1, info['fee_histogram']['fee_rate_groups'][tx1_info['feerate']]['count']) + assert_equal(1, info['fee_histogram'][tx1_info['feerate']]['count']) assert_equal(total_fees, info['fee_histogram']['total_fees']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['1']['size']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['1']['count']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['1']['fees']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['1']['from']) + assert_equal(0, info['fee_histogram']['1']['sizes']) + assert_equal(0, info['fee_histogram']['1']['count']) + assert_equal(0, info['fee_histogram']['1']['fees']) + assert_equal(1, info['fee_histogram']['1']['from_feerate']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['3']['size']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['3']['count']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['3']['fees']) - assert_equal(3, info['fee_histogram']['fee_rate_groups']['3']['from']) + assert_equal(0, info['fee_histogram']['3']['sizes']) + assert_equal(0, info['fee_histogram']['3']['count']) + assert_equal(0, info['fee_histogram']['3']['fees']) + assert_equal(3, info['fee_histogram']['3']['from_feerate']) - assert_equal(188, info['fee_histogram']['fee_rate_groups']['5']['size']) - assert_equal(1, info['fee_histogram']['fee_rate_groups']['5']['count']) - assert_equal(940, info['fee_histogram']['fee_rate_groups']['5']['fees']) - assert_equal(5, info['fee_histogram']['fee_rate_groups']['5']['from']) + assert_equal(188, info['fee_histogram']['5']['sizes']) + assert_equal(1, info['fee_histogram']['5']['count']) + assert_equal(940, info['fee_histogram']['5']['fees']) + assert_equal(5, info['fee_histogram']['5']['from_feerate']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['10']['size']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['10']['count']) - assert_equal(0, info['fee_histogram']['fee_rate_groups']['10']['fees']) - assert_equal(10, info['fee_histogram']['fee_rate_groups']['10']['from']) + assert_equal(0, info['fee_histogram']['10']['sizes']) + assert_equal(0, info['fee_histogram']['10']['count']) + assert_equal(0, info['fee_histogram']['10']['fees']) + assert_equal(10, info['fee_histogram']['10']['from_feerate']) self.log.info("Send tx2 transaction with 14 sat/vB fee rate (spends tx1 UTXO)") tx2_txid = node.sendtoaddress(address=node.getnewaddress(), amount=Decimal("25.0"), fee_rate=14, subtractfeefromamount=True) @@ -115,7 +115,7 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): tx1p2_feerate = get_actual_fee_rate(tx1_info['fee'] + tx2_info['fee'], tx1_info['vsize'] + tx2_info['vsize']) assert_equal(1, non_empty_groups) assert_equal(14, empty_groups) - assert_equal(2, info['fee_histogram']['fee_rate_groups'][tx1p2_feerate]['count']) + assert_equal(2, info['fee_histogram'][tx1p2_feerate]['count']) assert_equal(total_fees, info['fee_histogram']['total_fees']) # Unlock the second UTXO which we locked @@ -132,26 +132,27 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): # tx1 should be grouped with tx2 + tx3 (descendants) # tx2 should be grouped with tx1 (ancestors only) # tx3 should be alone - expected_histogram = { - 'fee_rate_groups': dict( ( + expected_histogram = dict( + tuple( (str(n), { - 'from': n, - 'to': n, + 'from_feerate': n, + 'to_feerate': n + 1, 'count': 0, 'fees': 0, - 'size': 0, + 'sizes': 0, }) for n in range(1, 16) - ) ), - 'total_fees': tx1_info['fee'] + tx2_info['fee'] + tx3_info['fee'], - } - expected_frg = expected_histogram['fee_rate_groups'] - expected_frg['15']['to'] = None + ) + ( + ('total_fees', tx1_info['fee'] + tx2_info['fee'] + tx3_info['fee']), + ) + ) + expected_frg = expected_histogram + expected_frg['15']['to_feerate'] = 9223372036854775807 tx1p2p3_feerate = get_actual_fee_rate(expected_histogram['total_fees'], tx1_info['vsize'] + tx2_info['vsize'] + tx3_info['vsize']) def inc_expected(feerate, txinfo): this_frg = expected_frg[feerate] this_frg['count'] += 1 this_frg['fees'] += txinfo['fee'] - this_frg['size'] += txinfo['vsize'] + this_frg['sizes'] += txinfo['vsize'] inc_expected(tx1p2p3_feerate, tx1_info) inc_expected(tx1p2_feerate, tx2_info) inc_expected(tx3_info['feerate'], tx3_info) @@ -163,26 +164,23 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): # Verify that the 6 sat/vB fee rate group has one transaction, and the 8-9 sat/vB fee rate group has two for collapse_n in (9, 11, 13, 15): - for field in ('count', 'size', 'fees'): + for field in ('count', 'sizes', 'fees'): expected_frg[str(collapse_n - 1)][field] += expected_frg[str(collapse_n)][field] - expected_frg[str(collapse_n - 1)]['to'] += 1 + expected_frg[str(collapse_n - 1)]['to_feerate'] += 1 del expected_frg[str(collapse_n)] - expected_frg['14']['to'] += 1 # 16 is also skipped + expected_frg['14']['to_feerate'] += 1 # 16 is also skipped for new_n in (17, 20, 25) + tuple(range(30, 90, 10)) + (100, 120, 140, 170, 200, 250) + tuple(range(300, 900, 100)) + (1000, 1200, 1400, 1700, 2000, 2500) + tuple(range(3000, 9000, 1000)) + (10000,): - frinfo = info['fee_histogram']['fee_rate_groups'][str(new_n)] - if new_n == 10000: - assert frinfo['to'] is None - else: - assert frinfo['to'] > frinfo['from'] - del frinfo['to'] + frinfo = info['fee_histogram'][str(new_n)] + assert frinfo['to_feerate'] > frinfo['from_feerate'] + del frinfo['to_feerate'] assert_equal(frinfo, { - 'from': new_n, + 'from_feerate': new_n, 'count': 0, 'fees': 0, - 'size': 0, + 'sizes': 0, }) - del info['fee_histogram']['fee_rate_groups'][str(new_n)] + del info['fee_histogram'][str(new_n)] assert_equal(expected_histogram, info['fee_histogram']) self.log.info("Test getmempoolinfo(with_fee_histogram=False) does not return fee histogram") @@ -193,16 +191,23 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): empty_count = 0 non_empty_count = 0 - for key, bin in histogram['fee_rate_groups'].items(): - assert_equal(int(key), bin['from']) + for key, bin in histogram.items(): + if key == 'total_fees': + continue + assert_equal(int(key), bin['from_feerate']) if bin['fees'] > 0: assert_greater_than(bin['count'], 0) else: assert_equal(bin['count'], 0) assert_greater_than_or_equal(bin['fees'], 0) - assert_greater_than_or_equal(bin['size'], 0) - if bin['to'] is not None: - assert_greater_than_or_equal(bin['to'], bin['from']) + assert_greater_than_or_equal(bin['sizes'], 0) + if bin['to_feerate'] is not None: + assert_greater_than_or_equal(bin['to_feerate'], bin['from_feerate']) + for next_key in sorted((*(int(a) for a in histogram.keys() if a != 'total_fees'), 0x7fffffffffffffff)): + if int(next_key) <= int(key): + continue + assert_equal(bin['to_feerate'], int(next_key)) + break total_fees += bin['fees'] if bin['count'] == 0: From 446ba480453b2f1c468ae43f084363f0726c0f04 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 9 Nov 2022 22:17:52 +0000 Subject: [PATCH 10/16] Bugfix: RPC/blockchain: Correct type of "to_feerate" result in getmempoolinfo fee histogram --- src/rpc/mempool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 90de1591f5..69f085dd77 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -770,7 +770,7 @@ static RPCHelpMan getmempoolinfo() {RPCResult::Type::NUM, "count", "Number of transactions in the fee rate group"}, {RPCResult::Type::NUM, "fees", "Cumulative fees of all transactions in the fee rate group (in " + CURRENCY_ATOM + ")"}, {RPCResult::Type::NUM, "from_feerate", "Group contains transactions with fee rates equal or greater than this value (in " + CURRENCY_ATOM + "/vB)"}, - {RPCResult::Type::ANY, "to_feerate", "Group contains transactions with fee rates equal or less than this value (in " + CURRENCY_ATOM + "/vB)"}, + {RPCResult::Type::NUM, "to_feerate", /*optional=*/true, "Group contains transactions with fee rates equal or less than this value (in " + CURRENCY_ATOM + "/vB)"}, }}, {RPCResult::Type::ELISION, "", ""}, {RPCResult::Type::NUM, "total_fees", "Total available fees in mempool (in " + CURRENCY_ATOM + ")"}, From 40f27a5e7b746105c9d8d2e3ca05d46b71a5748b Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 10 Nov 2022 03:31:29 +0000 Subject: [PATCH 11/16] RPC/mempool: Skip result type checks for fee_histogram in getmempoolinfo fee_histogram contains both dynamic-named group objects as well as a numeric total_fees, which the check cannot handle Long-term, the result is expected to change to avoid this, so just skip it for now --- src/rpc/mempool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 69f085dd77..c68ef6b8be 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -762,7 +762,7 @@ static RPCHelpMan getmempoolinfo() {RPCResult::Type::NUM, "incrementalrelayfee", "minimum fee rate increment for mempool limiting or replacement in " + CURRENCY_UNIT + "/kvB"}, {RPCResult::Type::NUM, "unbroadcastcount", "Current number of transactions that haven't passed initial broadcast yet"}, {RPCResult::Type::BOOL, "fullrbf", "True if the mempool accepts RBF without replaceability signaling inspection"}, - {RPCResult::Type::OBJ, "fee_histogram", /*optional=*/true, "", + {RPCResult::Type::OBJ_DYN, "fee_histogram", /*optional=*/true, "", { {RPCResult::Type::OBJ, "", "Fee rate group named by its lower bound (in " + CURRENCY_ATOM + "/vB), identical to the \"from_feerate\" field below", { From d10e35ab13d2d891a2d5726a4ac900d3e2b85ae1 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Fri, 10 Jun 2022 14:34:47 +0000 Subject: [PATCH 12/16] Bugfix: QA: Ensure mempool_fee_histogram expected feerates rounded down As of #22949, fees are rounded up based on feerate, but going the opposite direction from fee to feerate still must round down. --- test/functional/mempool_fee_histogram.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/mempool_fee_histogram.py b/test/functional/mempool_fee_histogram.py index 7b90a3ecaf..64a2f9c2c1 100755 --- a/test/functional/mempool_fee_histogram.py +++ b/test/functional/mempool_fee_histogram.py @@ -15,11 +15,11 @@ from test_framework.util import ( assert_equal, assert_greater_than, assert_greater_than_or_equal, - ceildiv, ) def get_actual_fee_rate(fee_in_satoshis, vsize): - fee_rate = ceildiv(fee_in_satoshis, vsize) + # NOTE: Must round down, unlike ceildiv/get_fee + fee_rate = fee_in_satoshis // vsize return str(fee_rate) def get_tx_details(node, txid): From 9ebe86025c51c7dd354f23531104fd9ab5eaa798 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 5 Jul 2023 22:31:46 +0000 Subject: [PATCH 13/16] Bugfix: RPC/blockchain: Actually round feerates down for getmempoolinfo fee histograms --- src/rpc/mempool.cpp | 10 ++++++---- test/functional/mempool_fee_histogram.py | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index c68ef6b8be..3a4e198647 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -697,12 +697,14 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional Date: Thu, 10 Nov 2022 04:05:53 +0000 Subject: [PATCH 14/16] QA: interface_rest: Check /mempool/info/with_fee_histogram matches RPC --- test/functional/interface_rest.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index ba6e960476..31f6e6c491 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -345,6 +345,9 @@ class RESTTest (BitcoinTestFramework): for obj in [json_obj, mempool_info]: obj.pop("unbroadcastcount") assert_equal(json_obj, mempool_info) + json_obj = self.test_rest_request("/mempool/info/with_fee_histogram") + mempool_info = self.nodes[0].getmempoolinfo(with_fee_histogram=True) + assert_equal(json_obj, mempool_info) # Check that there are our submitted transactions in the TX memory pool json_obj = self.test_rest_request("/mempool/contents") From 514352ba269a5dfb5e150fd1d94b5a8c2c9adb5a Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Thu, 7 Mar 2024 05:38:24 +0000 Subject: [PATCH 15/16] RPC/Mempool: Avoid extra decrement of unsigned below 0 when building fee histogram --- src/rpc/mempool.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 3a4e198647..baebdbca8e 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -709,7 +709,8 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool, const std::optional 0;) { + for (size_t i = floors.size(); i > 0;) { + --i; if (fee_rate >= floors[i]) { sizes[i] += size; ++count[i]; From 14b872b852ad301d6e74c7dd7ee7977d1f88e295 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 4 May 2024 03:41:10 +0000 Subject: [PATCH 16/16] Bugfix: QA: mempool_fee_histogram: Compare to actual vsize/fee rather than hard-coding a particular constant --- test/functional/mempool_fee_histogram.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/mempool_fee_histogram.py b/test/functional/mempool_fee_histogram.py index f1e6a3f7e8..47ccc16886 100755 --- a/test/functional/mempool_fee_histogram.py +++ b/test/functional/mempool_fee_histogram.py @@ -93,9 +93,9 @@ class MempoolFeeHistogramTest(BitcoinTestFramework): assert_equal(0, info['fee_histogram']['3']['fees']) assert_equal(3, info['fee_histogram']['3']['from_feerate']) - assert_equal(188, info['fee_histogram']['5']['sizes']) + assert_equal(tx1_info['vsize'], info['fee_histogram']['5']['sizes']) assert_equal(1, info['fee_histogram']['5']['count']) - assert_equal(940, info['fee_histogram']['5']['fees']) + assert_equal(tx1_info['fee'], info['fee_histogram']['5']['fees']) assert_equal(5, info['fee_histogram']['5']['from_feerate']) assert_equal(0, info['fee_histogram']['10']['sizes'])