mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-01-10 03:47:29 -03:00
Merge bitcoin/bitcoin#22689: rpc: deprecate top-level fee fields in getmempool RPCs
2f9515f37arpc: move fees object to match help (josibake)07ade7db8fdoc: add release note for fee field deprecation (josibake)2ee406ce3etest: add functional test for deprecatedrpc=fees (josibake)35d928c632rpc: deprecate fee fields from mempool entries (josibake) Pull request description: per #22682 , top level fee fields for mempool entries have been deprecated since 0.17 but are still returned. this PR properly deprecates them so that they are no longer returned unless `-deprecatedrpc=fees` is passed. the first commit takes care of deprecation and also updates `test/functional/mempool_packages.py` to only use the `fees` object. the second commit adds a new functional test for `-deprecatedrpc=fees` closes #22682 ## questions for the reviewer * `-deprecatedrpc=fees` made the most sense to me, but happy to change if there is a name that makes more sense * #22682 seems to indicate that after some period of time, the fields will be removed all together. if we have a rough idea of when this will be, i can add a `TODO: fully remove in vXX` comment to `entryToJSON` ## testing to get started on testing, compile, run the tests, and start your node with the deprecated rpcs flag: ```bash ./src/bitcoind -daemon -deprecatedrpc=fees ``` you should see entries with the deprecated fields like so: ```json { "<txid>": { "fees": { "base": 0.00000671, "modified": 0.00000671, "ancestor": 0.00000671, "descendant": 0.00000671 }, "fee": 0.00000671, "modifiedfee": 0.00000671, "descendantfees": 671, "ancestorfees": 671, "vsize": 144, "weight": 573, ... }, ``` you can also check `getmempoolentry` using any of the txid's from the output above. next start the node without the deprecated flag, repeat the commands from above and verify that the deprecated fields are no longer present at the top level, but present in the "fees" object ACKs for top commit: jnewbery: reACK2f9515f37aglozow: utACK2f9515f37aTree-SHA512: b175f4d39d26d96dc5bae26717d3ccfa5842d98ab402065880bfdcf4921b14ca692a8919fe4e9969acbb5c4d6e6d07dd6462a7e0a0a7342556279b381e1a004e
This commit is contained in:
MarcoFalke
commit
4fd0ce75c5
6 changed files with 121 additions and 42 deletions
|
|
@ -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
|
||||
--------
|
||||
|
||||
|
|
|
|||
|
|
@ -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,6 +494,28 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
|
|||
{
|
||||
AssertLockHeld(pool.cs);
|
||||
|
||||
info.pushKV("vsize", (int)e.GetTxSize());
|
||||
info.pushKV("weight", (int)e.GetTxWeight());
|
||||
// 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());
|
||||
if (deprecated_fee_fields_enabled) {
|
||||
info.pushKV("descendantfees", e.GetModFeesWithDescendants());
|
||||
}
|
||||
info.pushKV("ancestorcount", e.GetCountWithAncestors());
|
||||
info.pushKV("ancestorsize", e.GetSizeWithAncestors());
|
||||
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()));
|
||||
|
|
@ -501,19 +523,6 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
|
|||
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()));
|
||||
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());
|
||||
info.pushKV("ancestorcount", e.GetCountWithAncestors());
|
||||
info.pushKV("ancestorsize", e.GetSizeWithAncestors());
|
||||
info.pushKV("ancestorfees", e.GetModFeesWithAncestors());
|
||||
info.pushKV("wtxid", pool.vTxHashes[e.vTxHashesIdx].first.ToString());
|
||||
const CTransaction& tx = e.GetTx();
|
||||
std::set<std::string> setDepends;
|
||||
for (const CTxIn& txin : tx.vin)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
67
test/functional/rpc_mempool_entry_fee_fields_deprecation.py
Executable file
67
test/functional/rpc_mempool_entry_fee_fields_deprecation.py
Executable file
|
|
@ -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()
|
||||
|
|
@ -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',
|
||||
|
|
|
|||
Loading…
Reference in a new issue