Merge #12336: Remove deprecated rpc options

db1cbcc [RPC] Remove deprecated addmultisigaddress return format (John Newbery)
cb28a0b [RPC] Remove deprecated createmultisig object (John Newbery)
ed45c82 [tests] Remove test for deprecated createmultsig option (John Newbery)
d066a1c [rpc] Remove deprecated getmininginfo RPC option (John Newbery)
c6f09c2 [rpc] remove deprecated estimatefee RPC (John Newbery)
a8e437a [tests] Remove estimatefee from rpc_deprecated.py test (John Newbery)
a5623b1 [tests] Remove tests for deprecated estimatefee RPC (John Newbery)
d119f2e [tests] Fix style warnings in feature_fee_estimation.py (John Newbery)

Pull request description:

  There were some RPC/RPC options deprecated in v0.16. Those can now be removed from master since v0.16 has been branched.

  - `estimatefee` RPC has been removed. The `feature_fee_estimation.py` test has been updated to remove the RPC, but doesn't yet have good coverage of the replacement RPC `estimatesmartfee`. Improving the test coverage should be done in a new PR. (#11031)
  - the `errors` field returned by `getmininginfo` has been deprecated and replaced by a `warning` field. (#10858)
  - providing addresses as inputs to `createmultisig` has been deprecated. Users should use `addmultisigaddress` instead (#11415)
  - The return format from `addmultisigaddress` has changed (#11415)

  `getwitnessaddress` was also deprecated in v0.16 and can be removed, but many tests are using that RPC, so it's a larger job to remove. It should be removed in a separate PR (possibly after #11739 and #11398 have been merged and the segwit test code tidied up)

Tree-SHA512: 8ffaa5f6094131339b9e9e468e8b141de4b144697d2271efa2992b80b12eb97849ade3da8df5c1c9400ed4c04e6a029926550a3e5846d2029b644f9e84ac7124

2 files changed, 80 insertions, 103 deletions

diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py
index 68453e50f4..e1263414bd 100755
--- a/test/functional/feature_fee_estimation.py
+++ b/test/functional/feature_fee_estimation.py
@@ -3,39 +3,47 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test fee estimation code."""
+from decimal import Decimal
+import random
 
-from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import *
-from test_framework.script import CScript, OP_1, OP_DROP, OP_2, OP_HASH160, OP_EQUAL, hash160, OP_TRUE
 from test_framework.mininode import CTransaction, CTxIn, CTxOut, COutPoint, ToHex, COIN
+from test_framework.script import CScript, OP_1, OP_DROP, OP_2, OP_HASH160, OP_EQUAL, hash160, OP_TRUE
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_greater_than,
+    assert_greater_than_or_equal,
+    connect_nodes,
+    satoshi_round,
+    sync_blocks,
+    sync_mempools,
+)
 
 # Construct 2 trivial P2SH's and the ScriptSigs that spend them
 # So we can create many transactions without needing to spend
 # time signing.
-redeem_script_1 = CScript([OP_1, OP_DROP])
-redeem_script_2 = CScript([OP_2, OP_DROP])
-P2SH_1 = CScript([OP_HASH160, hash160(redeem_script_1), OP_EQUAL])
-P2SH_2 = CScript([OP_HASH160, hash160(redeem_script_2), OP_EQUAL])
+REDEEM_SCRIPT_1 = CScript([OP_1, OP_DROP])
+REDEEM_SCRIPT_2 = CScript([OP_2, OP_DROP])
+P2SH_1 = CScript([OP_HASH160, hash160(REDEEM_SCRIPT_1), OP_EQUAL])
+P2SH_2 = CScript([OP_HASH160, hash160(REDEEM_SCRIPT_2), OP_EQUAL])
 
 # Associated ScriptSig's to spend satisfy P2SH_1 and P2SH_2
-SCRIPT_SIG = [CScript([OP_TRUE, redeem_script_1]), CScript([OP_TRUE, redeem_script_2])]
-
-global log
+SCRIPT_SIG = [CScript([OP_TRUE, REDEEM_SCRIPT_1]), CScript([OP_TRUE, REDEEM_SCRIPT_2])]
 
 def small_txpuzzle_randfee(from_node, conflist, unconflist, amount, min_fee, fee_increment):
-    """
-    Create and send a transaction with a random fee.
+    """Create and send a transaction with a random fee.
+
     The transaction pays to a trivial P2SH script, and assumes that its inputs
     are of the same form.
     The function takes a list of confirmed outputs and unconfirmed outputs
     and attempts to use the confirmed list first for its inputs.
     It adds the newly created outputs to the unconfirmed list.
-    Returns (raw transaction, fee)
-    """
+    Returns (raw transaction, fee)."""
+
     # It's best to exponentially distribute our random fees
     # because the buckets are exponentially spaced.
     # Exponentially distributed from 1-128 * fee_increment
-    rand_fee = float(fee_increment)*(1.1892**random.randint(0,28))
+    rand_fee = float(fee_increment) * (1.1892 ** random.randint(0, 28))
     # Total fee ranges from min_fee to min_fee + 127*fee_increment
     fee = min_fee - fee_increment + satoshi_round(rand_fee)
     tx = CTransaction()
@@ -50,95 +58,69 @@ def small_txpuzzle_randfee(from_node, conflist, unconflist, amount, min_fee, fee
             total_in += t["amount"]
             tx.vin.append(CTxIn(COutPoint(int(t["txid"], 16), t["vout"]), b""))
         if total_in <= amount + fee:
-            raise RuntimeError("Insufficient funds: need %d, have %d"%(amount+fee, total_in))
-    tx.vout.append(CTxOut(int((total_in - amount - fee)*COIN), P2SH_1))
-    tx.vout.append(CTxOut(int(amount*COIN), P2SH_2))
+            raise RuntimeError("Insufficient funds: need %d, have %d" % (amount + fee, total_in))
+    tx.vout.append(CTxOut(int((total_in - amount - fee) * COIN), P2SH_1))
+    tx.vout.append(CTxOut(int(amount * COIN), P2SH_2))
     # These transactions don't need to be signed, but we still have to insert
     # the ScriptSig that will satisfy the ScriptPubKey.
     for inp in tx.vin:
         inp.scriptSig = SCRIPT_SIG[inp.prevout.n]
     txid = from_node.sendrawtransaction(ToHex(tx), True)
-    unconflist.append({ "txid" : txid, "vout" : 0 , "amount" : total_in - amount - fee})
-    unconflist.append({ "txid" : txid, "vout" : 1 , "amount" : amount})
+    unconflist.append({"txid": txid, "vout": 0, "amount": total_in - amount - fee})
+    unconflist.append({"txid": txid, "vout": 1, "amount": amount})
 
     return (ToHex(tx), fee)
 
-def split_inputs(from_node, txins, txouts, initial_split = False):
-    """
-    We need to generate a lot of inputs so we can generate a ton of transactions.
+def split_inputs(from_node, txins, txouts, initial_split=False):
+    """Generate a lot of inputs so we can generate a ton of transactions.
+
     This function takes an input from txins, and creates and sends a transaction
     which splits the value into 2 outputs which are appended to txouts.
     Previously this was designed to be small inputs so they wouldn't have
-    a high coin age when the notion of priority still existed.
-    """
+    a high coin age when the notion of priority still existed."""
+
     prevtxout = txins.pop()
     tx = CTransaction()
     tx.vin.append(CTxIn(COutPoint(int(prevtxout["txid"], 16), prevtxout["vout"]), b""))
 
-    half_change = satoshi_round(prevtxout["amount"]/2)
-    rem_change = prevtxout["amount"] - half_change  - Decimal("0.00001000")
-    tx.vout.append(CTxOut(int(half_change*COIN), P2SH_1))
-    tx.vout.append(CTxOut(int(rem_change*COIN), P2SH_2))
+    half_change = satoshi_round(prevtxout["amount"] / 2)
+    rem_change = prevtxout["amount"] - half_change - Decimal("0.00001000")
+    tx.vout.append(CTxOut(int(half_change * COIN), P2SH_1))
+    tx.vout.append(CTxOut(int(rem_change * COIN), P2SH_2))
 
     # If this is the initial split we actually need to sign the transaction
     # Otherwise we just need to insert the proper ScriptSig
-    if (initial_split) :
+    if (initial_split):
         completetx = from_node.signrawtransaction(ToHex(tx))["hex"]
-    else :
+    else:
         tx.vin[0].scriptSig = SCRIPT_SIG[prevtxout["vout"]]
         completetx = ToHex(tx)
     txid = from_node.sendrawtransaction(completetx, True)
-    txouts.append({ "txid" : txid, "vout" : 0 , "amount" : half_change})
-    txouts.append({ "txid" : txid, "vout" : 1 , "amount" : rem_change})
+    txouts.append({"txid": txid, "vout": 0, "amount": half_change})
+    txouts.append({"txid": txid, "vout": 1, "amount": rem_change})
 
-def check_estimates(node, fees_seen, max_invalid, print_estimates = True):
-    """
-    This function calls estimatefee and verifies that the estimates
-    meet certain invariants.
-    """
-    all_estimates = [ node.estimatefee(i) for i in range(1,26) ]
-    if print_estimates:
-        log.info([str(all_estimates[e-1]) for e in [1,2,3,6,15,25]])
-    delta = 1.0e-6 # account for rounding error
-    last_e = max(fees_seen)
-    for e in [x for x in all_estimates if x >= 0]:
-        # Estimates should be within the bounds of what transactions fees actually were:
-        if float(e)+delta < min(fees_seen) or float(e)-delta > max(fees_seen):
+def check_estimates(node, fees_seen, max_invalid):
+    """Call estimatesmartfee and verify that the estimates meet certain invariants."""
+
+    delta = 1.0e-6  # account for rounding error
+    last_feerate = float(max(fees_seen))
+    all_smart_estimates = [node.estimatesmartfee(i) for i in range(1, 26)]
+    for i, e in enumerate(all_smart_estimates):  # estimate is for i+1
+        feerate = float(e["feerate"])
+        assert_greater_than(feerate, 0)
+
+        if feerate + delta < min(fees_seen) or feerate - delta > max(fees_seen):
             raise AssertionError("Estimated fee (%f) out of range (%f,%f)"
-                                 %(float(e), min(fees_seen), max(fees_seen)))
-        # Estimates should be monotonically decreasing
-        if float(e)-delta > last_e:
+                                 % (feerate, min(fees_seen), max(fees_seen)))
+        if feerate - delta > last_feerate:
             raise AssertionError("Estimated fee (%f) larger than last fee (%f) for lower number of confirms"
-                                 %(float(e),float(last_e)))
-        last_e = e
-    valid_estimate = False
-    invalid_estimates = 0
-    for i,e in enumerate(all_estimates): # estimate is for i+1
-        if e >= 0:
-            valid_estimate = True
-            if i >= 13:  # for n>=14 estimatesmartfee(n/2) should be at least as high as estimatefee(n)
-                assert(node.estimatesmartfee((i+1)//2)["feerate"] > float(e) - delta)
+                                 % (feerate, last_feerate))
+        last_feerate = feerate
 
+        if i == 0:
+            assert_equal(e["blocks"], 2)
         else:
-            invalid_estimates += 1
-
-            # estimatesmartfee should still be valid
-            approx_estimate = node.estimatesmartfee(i+1)["feerate"]
-            answer_found = node.estimatesmartfee(i+1)["blocks"]
-            assert(approx_estimate > 0)
-            assert(answer_found > i+1)
-
-            # Once we're at a high enough confirmation count that we can give an estimate
-            # We should have estimates for all higher confirmation counts
-            if valid_estimate:
-                raise AssertionError("Invalid estimate appears at higher confirm count than valid estimate")
-
-    # Check on the expected number of different confirmation counts
-    # that we might not have valid estimates for
-    if invalid_estimates > max_invalid:
-        raise AssertionError("More than (%d) invalid estimates"%(max_invalid))
-    return all_estimates
-
+            assert_greater_than_or_equal(i + 1, e["blocks"])
 
 class EstimateFeeTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -151,7 +133,7 @@ class EstimateFeeTest(BitcoinTestFramework):
         which we will use to generate our transactions.
         """
         self.add_nodes(3, extra_args=[["-maxorphantx=1000", "-whitelist=127.0.0.1"],
-                                      ["-blockmaxsize=17000", "-maxorphantx=1000", "-deprecatedrpc=estimatefee"],
+                                      ["-blockmaxsize=17000", "-maxorphantx=1000"],
                                       ["-blockmaxsize=8000", "-maxorphantx=1000"]])
         # Use node0 to mine blocks for input splitting
         # Node1 mines small blocks but that are bigger than the expected transaction rate.
@@ -160,7 +142,6 @@ class EstimateFeeTest(BitcoinTestFramework):
         # Node2 is a stingy miner, that
         # produces too small blocks (room for only 55 or so transactions)
 
-
     def transact_and_mine(self, numblocks, mining_node):
         min_fee = Decimal("0.00001")
         # We will now mine numblocks blocks generating on average 100 transactions between each block
@@ -169,14 +150,14 @@ class EstimateFeeTest(BitcoinTestFramework):
         # resorting to tx's that depend on the mempool when those run out
         for i in range(numblocks):
             random.shuffle(self.confutxo)
-            for j in range(random.randrange(100-50,100+50)):
-                from_index = random.randint(1,2)
+            for j in range(random.randrange(100 - 50, 100 + 50)):
+                from_index = random.randint(1, 2)
                 (txhex, fee) = small_txpuzzle_randfee(self.nodes[from_index], self.confutxo,
                                                       self.memutxo, Decimal("0.005"), min_fee, min_fee)
                 tx_kbytes = (len(txhex) // 2) / 1000.0
-                self.fees_per_kb.append(float(fee)/tx_kbytes)
+                self.fees_per_kb.append(float(fee) / tx_kbytes)
             sync_mempools(self.nodes[0:3], wait=.1)
-            mined = mining_node.getblock(mining_node.generate(1)[0],True)["tx"]
+            mined = mining_node.getblock(mining_node.generate(1)[0], True)["tx"]
             sync_blocks(self.nodes[0:3], wait=.1)
             # update which txouts are confirmed
             newmem = []
@@ -191,10 +172,6 @@ class EstimateFeeTest(BitcoinTestFramework):
         self.log.info("This test is time consuming, please be patient")
         self.log.info("Splitting inputs so we can generate tx's")
 
-        # Make log handler available to helper functions
-        global log
-        log = self.log
-
         # Start node0
         self.start_node(0)
         self.txouts = []
@@ -210,13 +187,13 @@ class EstimateFeeTest(BitcoinTestFramework):
         # Use txouts to monitor the available utxo, since these won't be tracked in wallet
         reps = 0
         while (reps < 5):
-            #Double txouts to txouts2
-            while (len(self.txouts)>0):
+            # Double txouts to txouts2
+            while (len(self.txouts) > 0):
                 split_inputs(self.nodes[0], self.txouts, self.txouts2)
             while (len(self.nodes[0].getrawmempool()) > 0):
                 self.nodes[0].generate(1)
-            #Double txouts2 to txouts
-            while (len(self.txouts2)>0):
+            # Double txouts2 to txouts
+            while (len(self.txouts2) > 0):
                 split_inputs(self.nodes[0], self.txouts2, self.txouts)
             while (len(self.nodes[0].getrawmempool()) > 0):
                 self.nodes[0].generate(1)
@@ -235,7 +212,7 @@ class EstimateFeeTest(BitcoinTestFramework):
 
         self.fees_per_kb = []
         self.memutxo = []
-        self.confutxo = self.txouts # Start with the set of confirmed txouts after splitting
+        self.confutxo = self.txouts  # Start with the set of confirmed txouts after splitting
         self.log.info("Will output estimates for 1/2/3/6/15/25 blocks")
 
         for i in range(2):
diff --git a/test/functional/rpc_deprecated.py b/test/functional/rpc_deprecated.py
index d6f25158ef..90183474bb 100755
--- a/test/functional/rpc_deprecated.py
+++ b/test/functional/rpc_deprecated.py
@@ -4,24 +4,24 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test deprecation of RPC calls."""
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_raises_rpc_error
 
 class DeprecatedRpcTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
         self.setup_clean_chain = True
-        self.extra_args = [[], ["-deprecatedrpc=estimatefee", "-deprecatedrpc=createmultisig"]]
+        self.extra_args = [[], []]
 
     def run_test(self):
-        self.log.info("estimatefee: Shows deprecated message")
-        assert_raises_rpc_error(-32, 'estimatefee is deprecated', self.nodes[0].estimatefee, 1)
-
-        self.log.info("Using -deprecatedrpc=estimatefee bypasses the error")
-        self.nodes[1].estimatefee(1)
-
-        self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses")
-        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()])
-        self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()])
+        # This test should be used to verify correct behaviour of deprecated
+        # RPC methods with and without the -deprecatedrpc flags. For example:
+        #
+        # self.log.info("Make sure that -deprecatedrpc=createmultisig allows it to take addresses")
+        # assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, [self.nodes[0].getnewaddress()])
+        # self.nodes[1].createmultisig(1, [self.nodes[1].getnewaddress()])
+        #
+        # There are currently no deprecated RPC methods in master, so this
+        # test is currently empty.
+        pass
 
 if __name__ == '__main__':
     DeprecatedRpcTest().main()