diff options
| -rw-r--r-- | doc/release-notes.md | 9 | ||||
| -rw-r--r-- | src/rpc/blockchain.cpp | 47 | ||||
| -rwxr-xr-x | test/functional/mempool_packages.py | 25 | ||||
| -rwxr-xr-x | test/functional/rpc_fundrawtransaction.py | 10 | ||||
| -rwxr-xr-x | test/functional/rpc_mempool_entry_fee_fields_deprecation.py | 67 | ||||
| -rwxr-xr-x | test/functional/test_runner.py | 1 |
6 files changed, 119 insertions, 40 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index 3aff266173..230a56d2cd 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -114,6 +114,15 @@ Updated RPCs causes the lock to be written persistently to the wallet database. This allows UTXOs to remain locked even after node restarts or crashes. (#23065) +- The top-level fee fields `fee`, `modifiedfee`, `ancestorfees` and `descendantfees` + returned by RPCs `getmempoolentry`,`getrawmempool(verbose=true)`, + `getmempoolancestors(verbose=true)` and `getmempooldescendants(verbose=true)` + are deprecated and will be removed in the next major version (use + `-deprecated=fees` if needed in this version). The same fee fields can be accessed + through the `fees` object in the result. WARNING: deprecated + fields `ancestorfees` and `descendantfees` are denominated in sats, whereas all + fields in the `fees` object are denominated in BTC. (#22689) + New RPCs -------- diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f3abe97fc9..2011ffb4fd 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -464,23 +464,23 @@ static RPCHelpMan getdifficulty() static std::vector<RPCResult> MempoolEntryDescription() { return { RPCResult{RPCResult::Type::NUM, "vsize", "virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted."}, RPCResult{RPCResult::Type::NUM, "weight", "transaction weight as defined in BIP 141."}, - RPCResult{RPCResult::Type::STR_AMOUNT, "fee", "transaction fee in " + CURRENCY_UNIT + " (DEPRECATED)"}, - RPCResult{RPCResult::Type::STR_AMOUNT, "modifiedfee", "transaction fee with fee deltas used for mining priority (DEPRECATED)"}, + RPCResult{RPCResult::Type::STR_AMOUNT, "fee", "transaction fee, denominated in " + CURRENCY_UNIT + " (DEPRECATED, returned only if config option -deprecatedrpc=fees is passed)"}, + RPCResult{RPCResult::Type::STR_AMOUNT, "modifiedfee", "transaction fee with fee deltas used for mining priority, denominated in " + CURRENCY_UNIT + " (DEPRECATED, returned only if config option -deprecatedrpc=fees is passed)"}, RPCResult{RPCResult::Type::NUM_TIME, "time", "local time transaction entered pool in seconds since 1 Jan 1970 GMT"}, RPCResult{RPCResult::Type::NUM, "height", "block height when transaction entered pool"}, RPCResult{RPCResult::Type::NUM, "descendantcount", "number of in-mempool descendant transactions (including this one)"}, RPCResult{RPCResult::Type::NUM, "descendantsize", "virtual transaction size of in-mempool descendants (including this one)"}, - RPCResult{RPCResult::Type::STR_AMOUNT, "descendantfees", "modified fees (see above) of in-mempool descendants (including this one) (DEPRECATED)"}, + RPCResult{RPCResult::Type::STR_AMOUNT, "descendantfees", "transaction fees of in-mempool descendants (including this one) with fee deltas used for mining priority, denominated in " + CURRENCY_ATOM + "s (DEPRECATED, returned only if config option -deprecatedrpc=fees is passed)"}, RPCResult{RPCResult::Type::NUM, "ancestorcount", "number of in-mempool ancestor transactions (including this one)"}, RPCResult{RPCResult::Type::NUM, "ancestorsize", "virtual transaction size of in-mempool ancestors (including this one)"}, - RPCResult{RPCResult::Type::STR_AMOUNT, "ancestorfees", "modified fees (see above) of in-mempool ancestors (including this one) (DEPRECATED)"}, + RPCResult{RPCResult::Type::STR_AMOUNT, "ancestorfees", "transaction fees of in-mempool ancestors (including this one) with fee deltas used for mining priority, denominated in " + CURRENCY_ATOM + "s (DEPRECATED, returned only if config option -deprecatedrpc=fees is passed)"}, RPCResult{RPCResult::Type::STR_HEX, "wtxid", "hash of serialized transaction, including witness data"}, RPCResult{RPCResult::Type::OBJ, "fees", "", { - RPCResult{RPCResult::Type::STR_AMOUNT, "base", "transaction fee in " + CURRENCY_UNIT}, - RPCResult{RPCResult::Type::STR_AMOUNT, "modified", "transaction fee with fee deltas used for mining priority in " + CURRENCY_UNIT}, - RPCResult{RPCResult::Type::STR_AMOUNT, "ancestor", "modified fees (see above) of in-mempool ancestors (including this one) in " + CURRENCY_UNIT}, - RPCResult{RPCResult::Type::STR_AMOUNT, "descendant", "modified fees (see above) of in-mempool descendants (including this one) in " + CURRENCY_UNIT}, + RPCResult{RPCResult::Type::STR_AMOUNT, "base", "transaction fee, denominated in " + CURRENCY_UNIT}, + RPCResult{RPCResult::Type::STR_AMOUNT, "modified", "transaction fee with fee deltas used for mining priority, denominated in " + CURRENCY_UNIT}, + RPCResult{RPCResult::Type::STR_AMOUNT, "ancestor", "transaction fees of in-mempool ancestors (including this one) with fee deltas used for mining priority, denominated in " + CURRENCY_UNIT}, + RPCResult{RPCResult::Type::STR_AMOUNT, "descendant", "transaction fees of in-mempool descendants (including this one) with fee deltas used for mining priority, denominated in " + CURRENCY_UNIT}, }}, RPCResult{RPCResult::Type::ARR, "depends", "unconfirmed transactions used as inputs for this transaction", {RPCResult{RPCResult::Type::STR_HEX, "transactionid", "parent transaction id"}}}, @@ -494,26 +494,35 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool { AssertLockHeld(pool.cs); - UniValue fees(UniValue::VOBJ); - fees.pushKV("base", ValueFromAmount(e.GetFee())); - fees.pushKV("modified", ValueFromAmount(e.GetModifiedFee())); - fees.pushKV("ancestor", ValueFromAmount(e.GetModFeesWithAncestors())); - fees.pushKV("descendant", ValueFromAmount(e.GetModFeesWithDescendants())); - info.pushKV("fees", fees); - info.pushKV("vsize", (int)e.GetTxSize()); info.pushKV("weight", (int)e.GetTxWeight()); - info.pushKV("fee", ValueFromAmount(e.GetFee())); - info.pushKV("modifiedfee", ValueFromAmount(e.GetModifiedFee())); + // TODO: top-level fee fields are deprecated. deprecated_fee_fields_enabled blocks should be removed in v24 + const bool deprecated_fee_fields_enabled{IsDeprecatedRPCEnabled("fees")}; + if (deprecated_fee_fields_enabled) { + info.pushKV("fee", ValueFromAmount(e.GetFee())); + info.pushKV("modifiedfee", ValueFromAmount(e.GetModifiedFee())); + } info.pushKV("time", count_seconds(e.GetTime())); info.pushKV("height", (int)e.GetHeight()); info.pushKV("descendantcount", e.GetCountWithDescendants()); info.pushKV("descendantsize", e.GetSizeWithDescendants()); - info.pushKV("descendantfees", e.GetModFeesWithDescendants()); + if (deprecated_fee_fields_enabled) { + info.pushKV("descendantfees", e.GetModFeesWithDescendants()); + } info.pushKV("ancestorcount", e.GetCountWithAncestors()); info.pushKV("ancestorsize", e.GetSizeWithAncestors()); - info.pushKV("ancestorfees", e.GetModFeesWithAncestors()); + if (deprecated_fee_fields_enabled) { + info.pushKV("ancestorfees", e.GetModFeesWithAncestors()); + } info.pushKV("wtxid", pool.vTxHashes[e.vTxHashesIdx].first.ToString()); + + UniValue fees(UniValue::VOBJ); + fees.pushKV("base", ValueFromAmount(e.GetFee())); + fees.pushKV("modified", ValueFromAmount(e.GetModifiedFee())); + fees.pushKV("ancestor", ValueFromAmount(e.GetModFeesWithAncestors())); + fees.pushKV("descendant", ValueFromAmount(e.GetModFeesWithDescendants())); + info.pushKV("fees", fees); + const CTransaction& tx = e.GetTx(); std::set<std::string> setDepends; for (const CTxIn& txin : tx.vin) diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index dcbd340f11..068fdc0b65 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -91,7 +91,7 @@ class MempoolPackagesTest(BitcoinTestFramework): assert_equal(ancestor_vsize, sum([mempool[tx]['vsize'] for tx in mempool])) ancestor_count = MAX_ANCESTORS - assert_equal(ancestor_fees, sum([mempool[tx]['fee'] for tx in mempool])) + assert_equal(ancestor_fees, sum([mempool[tx]['fees']['base'] for tx in mempool])) descendants = [] ancestors = list(chain) @@ -102,11 +102,8 @@ class MempoolPackagesTest(BitcoinTestFramework): # Check that the descendant calculations are correct assert_equal(entry['descendantcount'], descendant_count) - descendant_fees += entry['fee'] - assert_equal(entry['modifiedfee'], entry['fee']) - assert_equal(entry['fees']['base'], entry['fee']) - assert_equal(entry['fees']['modified'], entry['modifiedfee']) - assert_equal(entry['descendantfees'], descendant_fees * COIN) + descendant_fees += entry['fees']['base'] + assert_equal(entry['fees']['modified'], entry['fees']['base']) assert_equal(entry['fees']['descendant'], descendant_fees) descendant_vsize += entry['vsize'] assert_equal(entry['descendantsize'], descendant_vsize) @@ -114,10 +111,10 @@ class MempoolPackagesTest(BitcoinTestFramework): # Check that ancestor calculations are correct assert_equal(entry['ancestorcount'], ancestor_count) - assert_equal(entry['ancestorfees'], ancestor_fees * COIN) + assert_equal(entry['fees']['ancestor'], ancestor_fees) assert_equal(entry['ancestorsize'], ancestor_vsize) ancestor_vsize -= entry['vsize'] - ancestor_fees -= entry['fee'] + ancestor_fees -= entry['fees']['base'] ancestor_count -= 1 # Check that parent/child list is correct @@ -168,9 +165,8 @@ class MempoolPackagesTest(BitcoinTestFramework): ancestor_fees = 0 for x in chain: entry = self.nodes[0].getmempoolentry(x) - ancestor_fees += entry['fee'] + ancestor_fees += entry['fees']['base'] assert_equal(entry['fees']['ancestor'], ancestor_fees + Decimal('0.00001')) - assert_equal(entry['ancestorfees'], ancestor_fees * COIN + 1000) # Undo the prioritisetransaction for later tests self.nodes[0].prioritisetransaction(txid=chain[0], fee_delta=-1000) @@ -182,9 +178,8 @@ class MempoolPackagesTest(BitcoinTestFramework): descendant_fees = 0 for x in reversed(chain): entry = self.nodes[0].getmempoolentry(x) - descendant_fees += entry['fee'] + descendant_fees += entry['fees']['base'] assert_equal(entry['fees']['descendant'], descendant_fees + Decimal('0.00001')) - assert_equal(entry['descendantfees'], descendant_fees * COIN + 1000) # Adding one more transaction on to the chain should fail. assert_raises_rpc_error(-26, "too-long-mempool-chain", chain_transaction, self.nodes[0], [txid], [vout], value, fee, 1) @@ -204,11 +199,9 @@ class MempoolPackagesTest(BitcoinTestFramework): descendant_fees = 0 for x in reversed(chain): entry = self.nodes[0].getmempoolentry(x) - descendant_fees += entry['fee'] + descendant_fees += entry['fees']['base'] if (x == chain[-1]): - assert_equal(entry['modifiedfee'], entry['fee'] + Decimal("0.00002")) - assert_equal(entry['fees']['modified'], entry['fee'] + Decimal("0.00002")) - assert_equal(entry['descendantfees'], descendant_fees * COIN + 2000) + assert_equal(entry['fees']['modified'], entry['fees']['base'] + Decimal("0.00002")) assert_equal(entry['fees']['descendant'], descendant_fees + Decimal("0.00002")) # Check that node1's mempool is as expected (-> custom ancestor limit) diff --git a/test/functional/rpc_fundrawtransaction.py b/test/functional/rpc_fundrawtransaction.py index 93970ff40c..17d11a3ceb 100755 --- a/test/functional/rpc_fundrawtransaction.py +++ b/test/functional/rpc_fundrawtransaction.py @@ -415,7 +415,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Create same transaction over sendtoaddress. txId = self.nodes[0].sendtoaddress(self.nodes[1].getnewaddress(), 1.1) - signedFee = self.nodes[0].getmempoolentry(txId)['fee'] + signedFee = self.nodes[0].getmempoolentry(txId)['fees']['base'] # Compare fee. feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) @@ -441,7 +441,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Create same transaction over sendtoaddress. txId = self.nodes[0].sendmany("", outputs) - signedFee = self.nodes[0].getmempoolentry(txId)['fee'] + signedFee = self.nodes[0].getmempoolentry(txId)['fees']['base'] # Compare fee. feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) @@ -468,7 +468,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Create same transaction over sendtoaddress. txId = self.nodes[0].sendtoaddress(mSigObj, 1.1) - signedFee = self.nodes[0].getmempoolentry(txId)['fee'] + signedFee = self.nodes[0].getmempoolentry(txId)['fees']['base'] # Compare fee. feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) @@ -512,7 +512,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Create same transaction over sendtoaddress. txId = self.nodes[0].sendtoaddress(mSigObj, 1.1) - signedFee = self.nodes[0].getmempoolentry(txId)['fee'] + signedFee = self.nodes[0].getmempoolentry(txId)['fees']['base'] # Compare fee. feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) @@ -644,7 +644,7 @@ class RawTransactionsTest(BitcoinTestFramework): # Create same transaction over sendtoaddress. txId = self.nodes[1].sendmany("", outputs) - signedFee = self.nodes[1].getmempoolentry(txId)['fee'] + signedFee = self.nodes[1].getmempoolentry(txId)['fees']['base'] # Compare fee. feeDelta = Decimal(fundedTx['fee']) - Decimal(signedFee) diff --git a/test/functional/rpc_mempool_entry_fee_fields_deprecation.py b/test/functional/rpc_mempool_entry_fee_fields_deprecation.py new file mode 100755 index 0000000000..82761ff7c8 --- /dev/null +++ b/test/functional/rpc_mempool_entry_fee_fields_deprecation.py @@ -0,0 +1,67 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 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 deprecation of fee fields from top level mempool entry object""" + +from test_framework.blocktools import COIN +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal +from test_framework.wallet import MiniWallet + + +def assertions_helper(new_object, deprecated_object, deprecated_fields): + for field in deprecated_fields: + assert field in deprecated_object + assert field not in new_object + + +class MempoolFeeFieldsDeprecationTest(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.extra_args = [[], ["-deprecatedrpc=fees"]] + + def run_test(self): + # we get spendable outputs from the premined chain starting + # at block 76. see BitcoinTestFramework._initialize_chain() for details + self.wallet = MiniWallet(self.nodes[0]) + self.wallet.rescan_utxos() + + # we create the tx on the first node and wait until it syncs to node_deprecated + # thus, any differences must be coming from getmempoolentry or getrawmempool + tx = self.wallet.send_self_transfer(from_node=self.nodes[0]) + self.nodes[1].sendrawtransaction(tx["hex"]) + + deprecated_fields = ["ancestorfees", "descendantfees", "modifiedfee", "fee"] + self.test_getmempoolentry(tx["txid"], deprecated_fields) + self.test_getrawmempool(tx["txid"], deprecated_fields) + self.test_deprecated_fields_match(tx["txid"]) + + def test_getmempoolentry(self, txid, deprecated_fields): + + self.log.info("Test getmempoolentry rpc") + entry = self.nodes[0].getmempoolentry(txid) + deprecated_entry = self.nodes[1].getmempoolentry(txid) + assertions_helper(entry, deprecated_entry, deprecated_fields) + + def test_getrawmempool(self, txid, deprecated_fields): + + self.log.info("Test getrawmempool rpc") + entry = self.nodes[0].getrawmempool(verbose=True)[txid] + deprecated_entry = self.nodes[1].getrawmempool(verbose=True)[txid] + assertions_helper(entry, deprecated_entry, deprecated_fields) + + def test_deprecated_fields_match(self, txid): + + self.log.info("Test deprecated fee fields match new fees object") + entry = self.nodes[0].getmempoolentry(txid) + deprecated_entry = self.nodes[1].getmempoolentry(txid) + + assert_equal(deprecated_entry["fee"], entry["fees"]["base"]) + assert_equal(deprecated_entry["modifiedfee"], entry["fees"]["modified"]) + assert_equal(deprecated_entry["descendantfees"], entry["fees"]["descendant"] * COIN) + assert_equal(deprecated_entry["ancestorfees"], entry["fees"]["ancestor"] * COIN) + + +if __name__ == "__main__": + MempoolFeeFieldsDeprecationTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index ac1eed63ab..8e7e791538 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -314,6 +314,7 @@ BASE_SCRIPTS = [ 'feature_presegwit_node_upgrade.py', 'feature_settings.py', 'rpc_getdescriptorinfo.py', + 'rpc_mempool_entry_fee_fields_deprecation.py', 'rpc_help.py', 'feature_help.py', 'feature_shutdown.py', |