Merge bitcoin/bitcoin#22689: rpc: deprecate top-level fee fields in getmempool RPCs

2f9515f37addabde84c79926d7a24b2897a21dd1 rpc: move fees object to match help (josibake)
07ade7db8f919826c5e69bdaf7d54a6ae653175e doc: add release note for fee field deprecation (josibake)
2ee406ce3e9c252734cb391d85044ac389c34279 test: add functional test for deprecatedrpc=fees (josibake)
35d928c63237e31c99215e2d9d84782befd618d5 rpc: 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:
    reACK 2f9515f37addabde84c79926d7a24b2897a21dd1
  glozow:
    utACK 2f9515f37addabde84c79926d7a24b2897a21dd1

Tree-SHA512: b175f4d39d26d96dc5bae26717d3ccfa5842d98ab402065880bfdcf4921b14ca692a8919fe4e9969acbb5c4d6e6d07dd6462a7e0a0a7342556279b381e1a004e

4 files changed, 82 insertions, 21 deletions

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',