Merge #19294: test: refactor: Inline adjust_bitcoin_conf_for_pre_17

fa41b0a6da pep-8 test/functional/test_framework/util.py (MarcoFalke)
faa841bc97 test: refactor: Inline adjust_bitcoin_conf_for_pre_17 (MarcoFalke)

Pull request description:

  This removes mental and code complexity as well as attack surface for bikeshedding

ACKs for top commit:
  Sjors:
    utACK fa41b0a6da

Tree-SHA512: 6e3c872e66d98ffaa7aecdfd64aa7dd8fbb51815a8fdaba170ce0772b4c3360084d0ebab4a5feac768ab5df50d04528d7daafc51ba07c15445c1ef94fa3efd34

test/functional/feature_backwards_compatibility.py

@@ -26,7 +26,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.descriptors import descsum_create
 
 from test_framework.util import (
-    adjust_bitcoin_conf_for_pre_17,
     assert_equal,
     sync_blocks,
     sync_mempools,
@@ -60,8 +59,6 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
             170100,
             160300,
         ])
-        # adapt bitcoin.conf, because older bitcoind's don't recognize config sections
-        adjust_bitcoin_conf_for_pre_17(self.nodes[5].bitcoinconf)
 
         self.start_nodes()
 

test/functional/mempool_compatibility.py

@@ -16,9 +16,7 @@ Only v0.15.2 is required by this test. The rest is used in other backwards compa
 import os
 
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import (
+
-    adjust_bitcoin_conf_for_pre_17
-)
 
 class MempoolCompatibilityTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -33,7 +31,6 @@ class MempoolCompatibilityTest(BitcoinTestFramework):
             150200, # oldest version supported by the test framework
             None,
         ])
-        adjust_bitcoin_conf_for_pre_17(self.nodes[0].bitcoinconf)
         self.start_nodes()
         self.import_deterministic_coinbase_privkeys()
 

test/functional/test_framework/test_framework.py

@@ -452,7 +452,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         assert_equal(len(binary), num_nodes)
         assert_equal(len(binary_cli), num_nodes)
         for i in range(num_nodes):
-            self.nodes.append(TestNode(
+            test_node_i = TestNode(
                 i,
                 get_datadir_path(self.options.tmpdir, i),
                 chain=self.chain,
@@ -470,7 +470,15 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
                 start_perf=self.options.perf,
                 use_valgrind=self.options.valgrind,
                 descriptors=self.options.descriptors,
-            ))
+            )
+            self.nodes.append(test_node_i)
+            if not test_node_i.version_is_at_least(170000):
+                # adjust conf for pre 17
+                conf_file = test_node_i.bitcoinconf
+                with open(conf_file, 'r', encoding='utf8') as conf:
+                    conf_data = conf.read()
+                with open(conf_file, 'w', encoding='utf8') as conf:
+                    conf.write(conf_data.replace('[regtest]', ''))
 
     def start_node(self, i, *args, **kwargs):
         """Start a bitcoind"""

test/functional/test_framework/util.py

@@ -25,6 +25,7 @@ logger = logging.getLogger("TestFramework.utils")
 # Assert functions
 ##################
 
+
 def assert_approx(v, vexp, vspan=0.00001):
     """Assert that `v` is within `vspan` of `vexp`"""
     if v < vexp - vspan:
@@ -32,6 +33,7 @@ def assert_approx(v, vexp, vspan=0.00001):
     if v > vexp + vspan:
         raise AssertionError("%s > [%s..%s]" % (str(v), str(vexp - vspan), str(vexp + vspan)))
 
+
 def assert_fee_amount(fee, tx_size, fee_per_kB):
     """Assert the fee was in range"""
     target_fee = round(tx_size * fee_per_kB / 1000, 8)
@@ -41,21 +43,26 @@ def assert_fee_amount(fee, tx_size, fee_per_kB):
     if fee > (tx_size + 2) * fee_per_kB / 1000:
         raise AssertionError("Fee of %s BTC too high! (Should be %s BTC)" % (str(fee), str(target_fee)))
 
+
 def assert_equal(thing1, thing2, *args):
     if thing1 != thing2 or any(thing1 != arg for arg in args):
         raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
 
+
 def assert_greater_than(thing1, thing2):
     if thing1 <= thing2:
         raise AssertionError("%s <= %s" % (str(thing1), str(thing2)))
 
+
 def assert_greater_than_or_equal(thing1, thing2):
     if thing1 < thing2:
         raise AssertionError("%s < %s" % (str(thing1), str(thing2)))
 
+
 def assert_raises(exc, fun, *args, **kwds):
     assert_raises_message(exc, None, fun, *args, **kwds)
 
+
 def assert_raises_message(exc, message, fun, *args, **kwds):
     try:
         fun(*args, **kwds)
@@ -71,6 +78,7 @@ def assert_raises_message(exc, message, fun, *args, **kwds):
     else:
         raise AssertionError("No exception raised")
 
+
 def assert_raises_process_error(returncode, output, fun, *args, **kwds):
     """Execute a process and asserts the process return code and output.
 
@@ -95,6 +103,7 @@ def assert_raises_process_error(returncode, output, fun, *args, **kwds):
     else:
         raise AssertionError("No exception raised")
 
+
 def assert_raises_rpc_error(code, message, fun, *args, **kwds):
     """Run an RPC and verify that a specific JSONRPC exception code and message is raised.
 
@@ -113,6 +122,7 @@ def assert_raises_rpc_error(code, message, fun, *args, **kwds):
     """
     assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
 
+
 def try_rpc(code, message, fun, *args, **kwds):
     """Tries to run an rpc command.
 
@@ -134,22 +144,22 @@ def try_rpc(code, message, fun, *args, **kwds):
     else:
         return False
 
+
 def assert_is_hex_string(string):
     try:
         int(string, 16)
     except Exception as e:
-        raise AssertionError(
+        raise AssertionError("Couldn't interpret %r as hexadecimal; raised: %s" % (string, e))
-            "Couldn't interpret %r as hexadecimal; raised: %s" % (string, e))
+
 
 def assert_is_hash_string(string, length=64):
     if not isinstance(string, str):
         raise AssertionError("Expected a string, got type %r" % type(string))
     elif length and len(string) != length:
-        raise AssertionError(
+        raise AssertionError("String of length %d expected; got %d" % (length, len(string)))
-            "String of length %d expected; got %d" % (length, len(string)))
     elif not re.match('[abcdef0-9]+$', string):
-        raise AssertionError(
+        raise AssertionError("String %r contains invalid characters for a hash." % string)
-            "String %r contains invalid characters for a hash." % string)
+
 
 def assert_array_result(object_array, to_match, expected, should_not_find=False):
     """
@@ -180,9 +190,11 @@ def assert_array_result(object_array, to_match, expected, should_not_find=False)
     if num_matched > 0 and should_not_find:
         raise AssertionError("Objects were found %s" % (str(to_match)))
 
+
 # Utility functions
 ###################
 
+
 def check_json_precision():
     """Make sure json library being used does not lose precision converting BTC values"""
     n = Decimal("20000000.00000003")
@@ -190,11 +202,13 @@ def check_json_precision():
     if satoshis != 2000000000000003:
         raise RuntimeError("JSON encode/decode loses precision")
 
+
 def EncodeDecimal(o):
     if isinstance(o, Decimal):
         return str(o)
     raise TypeError(repr(o) + " is not JSON serializable")
 
+
 def count_bytes(hex_string):
     return len(bytearray.fromhex(hex_string))
 
@@ -202,12 +216,15 @@ def count_bytes(hex_string):
 def hex_str_to_bytes(hex_str):
     return unhexlify(hex_str.encode('ascii'))
 
+
 def str_to_b64str(string):
     return b64encode(string.encode('utf-8')).decode('ascii')
 
+
 def satoshi_round(amount):
     return Decimal(amount).quantize(Decimal('0.00000001'), rounding=ROUND_DOWN)
 
+
 def wait_until(predicate, *, attempts=float('inf'), timeout=float('inf'), lock=None, timeout_factor=1.0):
     if attempts == float('inf') and timeout == float('inf'):
         timeout = 60
@@ -235,6 +252,7 @@ def wait_until(predicate, *, attempts=float('inf'), timeout=float('inf'), lock=N
         raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
     raise RuntimeError('Unreachable')
 
+
 # RPC/P2P connection constants and functions
 ############################################
 
@@ -250,6 +268,7 @@ class PortSeed:
     # Must be initialized with a unique integer for each process
     n = None
 
+
 def get_rpc_proxy(url, node_number, *, timeout=None, coveragedir=None):
     """
     Args:
@@ -271,18 +290,20 @@ def get_rpc_proxy(url, node_number, *, timeout=None, coveragedir=None):
     proxy = AuthServiceProxy(url, **proxy_kwargs)
     proxy.url = url  # store URL on proxy for info
 
-    coverage_logfile = coverage.get_filename(
+    coverage_logfile = coverage.get_filename(coveragedir, node_number) if coveragedir else None
-        coveragedir, node_number) if coveragedir else None
 
     return coverage.AuthServiceProxyWrapper(proxy, coverage_logfile)
 
+
 def p2p_port(n):
     assert n <= MAX_NODES
     return PORT_MIN + n + (MAX_NODES * PortSeed.n) % (PORT_RANGE - 1 - MAX_NODES)
 
+
 def rpc_port(n):
     return PORT_MIN + PORT_RANGE + n + (MAX_NODES * PortSeed.n) % (PORT_RANGE - 1 - MAX_NODES)
 
+
 def rpc_url(datadir, i, chain, rpchost):
     rpc_u, rpc_p = get_auth_cookie(datadir, chain)
     host = '127.0.0.1'
@@ -295,9 +316,11 @@ def rpc_url(datadir, i, chain, rpchost):
             host = rpchost
     return "http://%s:%s@%s:%d" % (rpc_u, rpc_p, host, int(port))
 
+
 # Node functions
 ################
 
+
 def initialize_datadir(dirname, n, chain):
     datadir = get_datadir_path(dirname, n)
     if not os.path.isdir(datadir):
@@ -327,21 +350,17 @@ def initialize_datadir(dirname, n, chain):
         os.makedirs(os.path.join(datadir, 'stdout'), exist_ok=True)
     return datadir
 
-def adjust_bitcoin_conf_for_pre_17(conf_file):
-    with open(conf_file,'r', encoding='utf8') as conf:
-        conf_data = conf.read()
-    with open(conf_file, 'w', encoding='utf8') as conf:
-        conf_data_changed = conf_data.replace('[regtest]', '')
-        conf.write(conf_data_changed)
 
 def get_datadir_path(dirname, n):
     return os.path.join(dirname, "node" + str(n))
 
+
 def append_config(datadir, options):
     with open(os.path.join(datadir, "bitcoin.conf"), 'a', encoding='utf8') as f:
         for option in options:
             f.write(option + "\n")
 
+
 def get_auth_cookie(datadir, chain):
     user = None
     password = None
@@ -366,20 +385,24 @@ def get_auth_cookie(datadir, chain):
         raise ValueError("No RPC credentials")
     return user, password
 
+
 # If a cookie file exists in the given datadir, delete it.
 def delete_cookie_file(datadir, chain):
     if os.path.isfile(os.path.join(datadir, chain, ".cookie")):
         logger.debug("Deleting leftover cookie file")
         os.remove(os.path.join(datadir, chain, ".cookie"))
 
+
 def softfork_active(node, key):
     """Return whether a softfork is active."""
     return node.getblockchaininfo()['softforks'][key]['active']
 
+
 def set_node_times(nodes, t):
     for node in nodes:
         node.setmocktime(t)
 
+
 def disconnect_nodes(from_connection, node_num):
     def get_peer_ids():
         result = []
@@ -392,7 +415,7 @@ def disconnect_nodes(from_connection, node_num):
     if not peer_ids:
         logger.warning("disconnect_nodes: {} and {} were not connected".format(
             from_connection.index,
-            node_num
+            node_num,
         ))
         return
     for peer_id in peer_ids:
@@ -408,6 +431,7 @@ def disconnect_nodes(from_connection, node_num):
     # wait to disconnect
     wait_until(lambda: not get_peer_ids(), timeout=5)
 
+
 def connect_nodes(from_connection, node_num):
     ip_port = "127.0.0.1:" + str(p2p_port(node_num))
     from_connection.addnode(ip_port, "onetry")
@@ -479,6 +503,7 @@ def find_output(node, txid, amount, *, blockhash=None):
             return i
     raise RuntimeError("find_output txid %s : %s not found" % (txid, str(amount)))
 
+
 def gather_inputs(from_node, amount_needed, confirmations_required=1):
     """
     Return a random set of unspent txouts that are enough to pay amount_needed
@@ -496,6 +521,7 @@ def gather_inputs(from_node, amount_needed, confirmations_required=1):
         raise RuntimeError("Insufficient funds: need %d, have %d" % (amount_needed, total_in))
     return (total_in, inputs)
 
+
 def make_change(from_node, amount_in, amount_out, fee):
     """
     Create change output(s), return them
@@ -513,6 +539,7 @@ def make_change(from_node, amount_in, amount_out, fee):
         outputs[from_node.getnewaddress()] = change
     return outputs
 
+
 def random_transaction(nodes, amount, min_fee, fee_increment, fee_variants):
     """
     Create a random transaction.
@@ -532,6 +559,7 @@ def random_transaction(nodes, amount, min_fee, fee_increment, fee_variants):
 
     return (txid, signresult["hex"], fee)
 
+
 # Helper to create at least "count" utxos
 # Pass in a fee that is sufficient for relay and mining new transactions.
 def create_confirmed_utxos(fee, node, count):
@@ -564,6 +592,7 @@ def create_confirmed_utxos(fee, node, count):
     assert len(utxos) >= count
     return utxos
 
+
 # Create large OP_RETURN txouts that can be appended to a transaction
 # to make it large (helper for constructing large transactions).
 def gen_return_txouts():
@@ -583,6 +612,7 @@ def gen_return_txouts():
         txouts.append(txout)
     return txouts
 
+
 # Create a spend of each passed-in utxo, splicing in "txouts" to each raw
 # transaction to make it large.  See gen_return_txouts() above.
 def create_lots_of_big_transactions(node, txouts, utxos, num, fee):
@@ -606,6 +636,7 @@ def create_lots_of_big_transactions(node, txouts, utxos, num, fee):
         txids.append(txid)
     return txids
 
+
 def mine_large_block(node, utxos=None):
     # generate a 66k transaction,
     # and 14 of them is close to the 1MB block limit
@@ -619,6 +650,7 @@ def mine_large_block(node, utxos=None):
     create_lots_of_big_transactions(node, txouts, utxos, num, fee=fee)
     node.generate(1)
 
+
 def find_vout_for_address(node, txid, addr):
     """
     Locate the vout index of the given transaction sending to the

test/functional/wallet_upgradewallet.py

@@ -16,7 +16,6 @@ import shutil
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
-    adjust_bitcoin_conf_for_pre_17,
     assert_equal,
     assert_greater_than,
     assert_is_hex_string,
@@ -46,9 +45,6 @@ class UpgradeWalletTest(BitcoinTestFramework):
             160300,
             150200,
         ])
-        # adapt bitcoin.conf, because older bitcoind's don't recognize config sections
-        adjust_bitcoin_conf_for_pre_17(self.nodes[1].bitcoinconf)
-        adjust_bitcoin_conf_for_pre_17(self.nodes[2].bitcoinconf)
         self.start_nodes()
 
     def dumb_sync_blocks(self):