Skip to content

Commit

Permalink
rpc: deprecate fee fields from mempool entries
Browse files Browse the repository at this point in the history 
Unless `-deprecatedrpc=fees` is passed, top level
fee fields are no longer returned for mempool entries.

Add instructions to field help on how to access
deprecated fields, update help text for readability,
and include units. This is important to help
avoid any confusion as users move from deprecated
fields to the fee fields object (credit: jonatack).

This affects `getmempoolentry`, `getrawmempool`,
`getmempoolancestors`, and `getmempooldescendants`

Modify `test/functional/mempool_packages.py` and
`test/functional/rpc_fundrawtransaction.py` tests
to no longer use deprecated fields.

Co-authored-by: jonatack <jon@atack.com>
  • Loading branch information
@josibake @jonatack
josibake and jonatack committed Nov 2, 2021
1 parent db68c16 commit 79f6fca
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 34 deletions.
33 changes: 20 additions & 13 deletions src/rpc/blockchain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,23 +462,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"}}},
Expand All @@ -498,19 +498,26 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
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());
const CTransaction& tx = e.GetTx();
std::set<std::string> setDepends;
Expand Down
25 changes: 9 additions & 16 deletions test/functional/mempool_packages.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def run_test(self):

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)
Expand All @@ -102,22 +102,19 @@ def run_test(self):

# 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)
descendant_count += 1

# 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
Expand Down Expand Up @@ -168,9 +165,8 @@ def run_test(self):
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)
Expand All @@ -182,9 +178,8 @@ def run_test(self):
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)
Expand All @@ -205,11 +200,9 @@ def run_test(self):
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)
Expand Down
10 changes: 5 additions & 5 deletions test/functional/rpc_fundrawtransaction.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ def test_fee_p2pkh(self):

# 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)
Expand All @@ -443,7 +443,7 @@ def test_fee_p2pkh_multi_out(self):

# 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)
Expand All @@ -470,7 +470,7 @@ def test_fee_p2sh(self):

# 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)
Expand Down Expand Up @@ -514,7 +514,7 @@ def test_fee_4of5(self):

# 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)
Expand Down Expand Up @@ -651,7 +651,7 @@ def test_many_inputs_fee(self):

# 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)
Expand Down

0 comments on commit 79f6fca

Please sign in to comment.