Merge #6291: backport: merge bitcoin#23037, bitcoin#23047, bitcoin#23102, bitcoin#23209, bitcoin#23210, bitcoin#23501, bitcoin#23392, bitcoin#23799, bitcoin#24195, bitcoin#24324 (test backports)

51e1170 merge bitcoin#24324: refactor: remove unneeded bytes<->hex conversions in `byte_to_base58` (Kittywhiskers Van Gogh)
473ee8e merge bitcoin#24195: Fix failfast option for functional test runner (Kittywhiskers Van Gogh)
64dd467 merge bitcoin#23799: Let test_runner.py start multiple jobs per timeslot (Kittywhiskers Van Gogh)
1b241a2 merge bitcoin#23392: move check_node_connections to util (Kittywhiskers Van Gogh)
f6fa0c0 merge bitcoin#23501: various feature_nulldummy.py improvements (Kittywhiskers Van Gogh)
851dae7 merge bitcoin#23210: Replace satoshi_round with int() or Decimal() (Kittywhiskers Van Gogh)
739394d merge bitcoin#23209: Avoid RPC roundtrip in MiniWallet get_descriptor() (Kittywhiskers Van Gogh)
c96b9aa merge bitcoin#23102: Add missing re.escape() to feature_addrman test (Kittywhiskers Van Gogh)
4db9108 merge bitcoin#23047: Use MiniWallet in mempool_persist (Kittywhiskers Van Gogh)
234f22a merge bitcoin#23037: fix confusing off-by-one nValue in feature_coinstatsindex.py (Kittywhiskers Van Gogh)

Pull request description:

  ## Breaking Changes

  None expected.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    utACK 51e1170
  UdjinM6:
    utACK 51e1170

Tree-SHA512: 54ffbb2a44ad411d8602a7a752a05527faa65199da6cfa585f953892017c4d9e906c86c0b5b5e5d151d76eb0649aa76a7d17104ab0b3797a111d364ca52d716b

Author: PastaPastaPasta

test/functional/feature_addrman.py

@@ -5,6 +5,7 @@
 """Test addrman functionality"""
 
 import os
+import re
 import struct
 
 from test_framework.messages import ser_uint256, hash256
@@ -56,7 +57,7 @@ def run_test(self):
         init_error = lambda reason: (
             f"Error: Invalid or corrupt peers.dat \\({reason}\\). If you believe this "
             f"is a bug, please report it to {self.config['environment']['PACKAGE_BUGREPORT']}. "
-            f'As a workaround, you can move the file \\("{peers_dat}"\\) out of the way \\(rename, '
+            f'As a workaround, you can move the file \\("{re.escape(peers_dat)}"\\) out of the way \\(rename, '
             "move, or delete\\) to have a new one created on the next start."
         )
 

test/functional/feature_anchors.py

@@ -8,18 +8,12 @@
 
 from test_framework.p2p import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
+from test_framework.util import check_node_connections
 
 INBOUND_CONNECTIONS = 5
 BLOCK_RELAY_CONNECTIONS = 2
 
 
-def check_node_connections(*, node, num_in, num_out):
-    info = node.getnetworkinfo()
-    assert_equal(info["connections_in"], num_in)
-    assert_equal(info["connections_out"], num_out)
-
-
 class AnchorsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1

test/functional/feature_bip68_sequence.py

@@ -21,7 +21,6 @@
     assert_equal,
     assert_greater_than,
     assert_raises_rpc_error,
-    satoshi_round,
     softfork_active,
 )
 from test_framework.script_util import DUMMY_P2SH_SCRIPT
@@ -91,7 +90,7 @@ def test_disable_flag(self):
         utxo = utxos[0]
 
         tx1 = CTransaction()
-        value = int(satoshi_round(utxo["amount"] - self.relayfee)*COIN)
+        value = int((utxo["amount"] - self.relayfee) * COIN)
 
         # Check that the disable flag disables relative locktime.
         # If sequence locks were used, this would require 1 block for the

test/functional/feature_coinstatsindex.py

@@ -178,7 +178,7 @@ def _test_coin_stats_index(self):
         # Generate and send another tx with an OP_RETURN output (which is unspendable)
         tx2 = CTransaction()
         tx2.vin.append(CTxIn(COutPoint(int(tx1_txid, 16), n), b''))
-        tx2.vout.append(CTxOut(int(20.99 * COIN), CScript([OP_RETURN] + [OP_FALSE]*30)))
+        tx2.vout.append(CTxOut(int(Decimal('20.99') * COIN), CScript([OP_RETURN] + [OP_FALSE]*30)))
         tx2_hex = self.nodes[0].signrawtransactionwithwallet(tx2.serialize().hex())['hex']
         self.nodes[0].sendrawtransaction(tx2_hex)
 
@@ -189,16 +189,16 @@ def _test_coin_stats_index(self):
         for hash_option in index_hash_options:
             # Check all amounts were registered correctly
             res6 = index_node.gettxoutsetinfo(hash_option, 108)
-            assert_equal(res6['total_unspendable_amount'], Decimal('70.98999999'))
+            assert_equal(res6['total_unspendable_amount'], Decimal('70.99000000'))
             assert_equal(res6['block_info'], {
-                'unspendable': Decimal('20.98999999'),
+                'unspendable': Decimal('20.99000000'),
                 'prevout_spent': 511,
                 'new_outputs_ex_coinbase': Decimal('489.99999741'),
-                'coinbase': Decimal('500.01000260'),
+                'coinbase': Decimal('500.01000259'),
                 'unspendables': {
                     'genesis_block': 0,
                     'bip30': 0,
-                    'scripts': Decimal('20.98999999'),
+                    'scripts': Decimal('20.99000000'),
                     'unclaimed_rewards': 0
                 }
             })
@@ -220,7 +220,7 @@ def _test_coin_stats_index(self):
 
         for hash_option in index_hash_options:
             res7 = index_node.gettxoutsetinfo(hash_option, 109)
-            assert_equal(res7['total_unspendable_amount'], Decimal('530.98999999'))
+            assert_equal(res7['total_unspendable_amount'], Decimal('530.99000000'))
             assert_equal(res7['block_info'], {
                 'unspendable': 460,
                 'prevout_spent': 0,

test/functional/feature_nulldummy.py

@@ -20,7 +20,10 @@
     create_transaction,
 )
 from test_framework.messages import CTransaction
-from test_framework.script import CScript
+from test_framework.script import (
+    OP_0,
+    OP_TRUE,
+)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -29,16 +32,12 @@
 
 NULLDUMMY_ERROR = "non-mandatory-script-verify-flag (Dummy CHECKMULTISIG argument must be zero)"
 
-def trueDummy(tx):
-    scriptSig = CScript(tx.vin[0].scriptSig)
-    newscript = []
-    for i in scriptSig:
-        if len(newscript) == 0:
-            assert len(i) == 0
-            newscript.append(b'\x51')
-        else:
-            newscript.append(i)
-    tx.vin[0].scriptSig = CScript(newscript)
+
+def invalidate_nulldummy_tx(tx):
+    """Transform a NULLDUMMY compliant tx (i.e. scriptSig starts with OP_0)
+    to be non-NULLDUMMY compliant by replacing the dummy with OP_TRUE"""
+    assert_equal(tx.vin[0].scriptSig[0], OP_0)
+    tx.vin[0].scriptSig = bytes([OP_TRUE]) + tx.vin[0].scriptSig[1:]
     tx.rehash()
 
 class NULLDUMMYTest(BitcoinTestFramework):
@@ -86,7 +85,7 @@ def run_test(self):
 
         self.log.info("Test 2: Non-NULLDUMMY base multisig transaction should not be accepted to mempool before activation")
         test2tx = create_transaction(self.nodes[0], txid2, self.ms_address, amount=47)
-        trueDummy(test2tx)
+        invalidate_nulldummy_tx(test2tx)
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test2tx.serialize().hex(), 0)
 
         self.log.info(f"Test 3: Non-NULLDUMMY base transactions should be accepted in a block before activation [{COINBASE_MATURITY + 4}]")
@@ -95,7 +94,7 @@ def run_test(self):
         self.log.info("Test 4: Non-NULLDUMMY base multisig transaction is invalid after activation")
         test4tx = create_transaction(self.nodes[0], test2tx.hash, self.address, amount=46)
         test6txs=[CTransaction(test4tx)]
-        trueDummy(test4tx)
+        invalidate_nulldummy_tx(test4tx)
         assert_raises_rpc_error(-26, NULLDUMMY_ERROR, self.nodes[0].sendrawtransaction, test4tx.serialize().hex(), 0)
         self.block_submit(self.nodes[0], [test4tx], accept=False)
 
@@ -110,12 +109,7 @@ def block_submit(self, node, txs, *, accept=False):
         tmpl = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
         assert_equal(tmpl['previousblockhash'], self.lastblockhash)
         assert_equal(tmpl['height'], self.lastblockheight + 1)
-        block = create_block(tmpl=tmpl, ntime=self.lastblocktime + 1, dip4_activated=dip4_activated)
-        for tx in txs:
-            tx.rehash()
-            block.vtx.append(tx)
-        block.hashMerkleRoot = block.calc_merkle_root()
-        block.rehash()
+        block = create_block(tmpl=tmpl, ntime=self.lastblocktime + 1, txlist=txs, dip4_activated=dip4_activated)
         block.solve()
         assert_equal(None if accept else NULLDUMMY_ERROR, node.submitblock(block.serialize().hex()))
         if accept:

test/functional/mempool_packages.py

@@ -14,7 +14,6 @@
     assert_equal,
     assert_raises_rpc_error,
     chain_transaction,
-    satoshi_round,
 )
 
 # default limits
@@ -190,10 +189,10 @@ def run_test(self):
             entry = self.nodes[0].getmempoolentry(x)
             descendant_fees += entry['fee']
             if (x == chain[-1]):
-                assert_equal(entry['modifiedfee'], entry['fee']+satoshi_round(0.00002))
-                assert_equal(entry['fees']['modified'], entry['fee']+satoshi_round(0.00002))
+                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']['descendant'], descendant_fees+satoshi_round(0.00002))
+            assert_equal(entry['fees']['descendant'], descendant_fees + Decimal("0.00002"))
 
         # Check that node1's mempool is as expected (-> custom ancestor limit)
         mempool0 = self.nodes[0].getrawmempool(False)
@@ -289,7 +288,7 @@ def run_test(self):
         value = utxo[0]['amount']
         vout = utxo[0]['vout']
 
-        send_value = satoshi_round((value - fee)/2)
+        send_value = (value - fee) / 2
         inputs = [ {'txid' : txid, 'vout' : vout} ]
         outputs = {}
         for _ in range(2):

test/functional/mempool_persist.py

@@ -44,22 +44,34 @@
     assert_equal,
     assert_greater_than_or_equal, assert_raises_rpc_error,
 )
+from test_framework.wallet import MiniWallet
 
 
 class MempoolPersistTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 3
         self.extra_args = [[], ["-persistmempool=0"], []]
 
-    def skip_test_if_missing_module(self):
-        self.skip_if_no_wallet()
-
     def run_test(self):
+        self.mini_wallet = MiniWallet(self.nodes[2])
+        self.mini_wallet.rescan_utxos()
+        if self.is_sqlite_compiled():
+            self.nodes[2].createwallet(
+                wallet_name="watch",
+                descriptors=True,
+                disable_private_keys=True,
+                load_on_startup=False,
+            )
+            wallet_watch = self.nodes[2].get_wallet_rpc("watch")
+            assert_equal([{'success': True}], wallet_watch.importdescriptors([{'desc': self.mini_wallet.get_descriptor(), 'timestamp': 0}]))
+
         self.log.debug("Send 5 transactions from node2 (to its own address)")
         tx_creation_time_lower = self.mocktime
         for _ in range(5):
-            last_txid = self.nodes[2].sendtoaddress(self.nodes[2].getnewaddress(), Decimal("10"))
-        node2_balance = self.nodes[2].getbalance()
+            last_txid = self.mini_wallet.send_self_transfer(from_node=self.nodes[2])["txid"]
+        if self.is_sqlite_compiled():
+            self.nodes[2].syncwithvalidationinterfacequeue()  # Flush mempool to wallet
+            node2_balance = wallet_watch.getbalance()
         self.sync_all()
         tx_creation_time_higher = self.mocktime
 
@@ -80,16 +92,16 @@ def run_test(self):
         assert_equal(total_fee_old, self.nodes[0].getmempoolinfo()['total_fee'])
         assert_equal(total_fee_old, sum(v['fees']['base'] for k, v in self.nodes[0].getrawmempool(verbose=True).items()))
 
-        tx_creation_time = self.nodes[0].getmempoolentry(txid=last_txid)['time']
+        last_entry = self.nodes[0].getmempoolentry(txid=last_txid)
+        tx_creation_time = last_entry['time']
         assert_greater_than_or_equal(tx_creation_time, tx_creation_time_lower)
         assert_greater_than_or_equal(tx_creation_time_higher, tx_creation_time)
 
         # disconnect nodes & make a txn that remains in the unbroadcast set.
         self.disconnect_nodes(0, 1)
-        assert(len(self.nodes[0].getpeerinfo()) == 0)
-        assert(len(self.nodes[0].p2ps) == 0)
-        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), Decimal("12"))
-        self.connect_nodes(0, 2)
+        assert_equal(len(self.nodes[0].getpeerinfo()), 0)
+        assert_equal(len(self.nodes[0].p2ps), 0)
+        self.mini_wallet.send_self_transfer(from_node=self.nodes[0])
 
         self.log.debug("Stop-start the nodes. Verify that node0 has the transactions in its mempool and node1 does not. Verify that node2 calculates its balance correctly after loading wallet transactions.")
         self.stop_nodes()
@@ -109,17 +121,19 @@ def run_test(self):
         fees = self.nodes[0].getmempoolentry(txid=last_txid)['fees']
         assert_equal(fees['base'] + Decimal('0.00001000'), fees['modified'])
 
-        self.log.debug('Verify time is loaded correctly')
-        assert_equal(tx_creation_time, self.nodes[0].getmempoolentry(txid=last_txid)['time'])
+        self.log.debug('Verify all fields are loaded correctly')
+        assert_equal(last_entry, self.nodes[0].getmempoolentry(txid=last_txid))
 
         # Verify accounting of mempool transactions after restart is correct
-        self.nodes[2].syncwithvalidationinterfacequeue()  # Flush mempool to wallet
-        assert_equal(node2_balance, self.nodes[2].getbalance())
+        if self.is_sqlite_compiled():
+            self.nodes[2].loadwallet("watch")
+            wallet_watch = self.nodes[2].get_wallet_rpc("watch")
+            self.nodes[2].syncwithvalidationinterfacequeue()  # Flush mempool to wallet
+            assert_equal(node2_balance, wallet_watch.getbalance())
 
-        # start node0 with wallet disabled so wallet transactions don't get resubmitted
         self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.")
         self.stop_nodes()
-        self.start_node(0, extra_args=["-persistmempool=0", "-disablewallet"])
+        self.start_node(0, extra_args=["-persistmempool=0"])
         assert self.nodes[0].getmempoolinfo()["loaded"]
         assert_equal(len(self.nodes[0].getrawmempool()), 0)
 
@@ -163,18 +177,18 @@ def test_persist_unbroadcast(self):
 
         # ensure node0 doesn't have any connections
         # make a transaction that will remain in the unbroadcast set
-        assert(len(node0.getpeerinfo()) == 0)
-        assert(len(node0.p2ps) == 0)
-        node0.sendtoaddress(self.nodes[1].getnewaddress(), Decimal("12"))
+        assert_equal(len(node0.getpeerinfo()), 0)
+        assert_equal(len(node0.p2ps), 0)
+        self.mini_wallet.send_self_transfer(from_node=node0)
 
         # shutdown, then startup with wallet disabled
-        self.stop_nodes()
-        self.start_node(0, extra_args=["-disablewallet"])
+        self.restart_node(0, extra_args=["-disablewallet"])
 
         # check that txn gets broadcast due to unbroadcast logic
         # conn = node0.add_p2p_connection(P2PTxInvStore())
-        # node0.mockscheduler(16*60) # 15 min + 1 for buffer
+        # node0.mockscheduler(16 * 60)  # 15 min + 1 for buffer
         # self.wait_until(lambda: len(conn.get_invs()) == 1)
 
-if __name__ == '__main__':
+
+if __name__ == "__main__":
     MempoolPersistTest().main()

test/functional/p2p_add_connections.py

@@ -6,13 +6,7 @@
 
 from test_framework.p2p import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
-
-
-def check_node_connections(*, node, num_in, num_out):
-    info = node.getnetworkinfo()
-    assert_equal(info["connections_in"], num_in)
-    assert_equal(info["connections_out"], num_out)
+from test_framework.util import check_node_connections
 
 
 class P2PAddConnections(BitcoinTestFramework):

test/functional/test_framework/address.py

@@ -25,17 +25,15 @@
 
 def byte_to_base58(b, version):
     result = ''
-    str = b.hex()
-    str = chr(version).encode('latin-1').hex() + str
-    checksum = hash256(bytes.fromhex(str)).hex()
-    str += checksum[:8]
-    value = int('0x' + str, 0)
+    b = bytes([version]) + b  # prepend version
+    b += hash256(b)[:4]       # append checksum
+    value = int.from_bytes(b, 'big')
     while value > 0:
         result = chars[value % 58] + result
         value //= 58
-    while (str[:2] == '00'):
+    while b[0] == 0:
         result = chars[0] + result
-        str = str[2:]
+        b = b[1:]
     return result
 
 

test/functional/test_framework/util.py

@@ -264,6 +264,7 @@ def wait_until_helper(predicate, *, attempts=float('inf'), timeout=float('inf'),
     else:
         return False
 
+
 def sha256sum_file(filename):
     h = hashlib.sha256()
     with open(filename, 'rb') as f:
@@ -479,6 +480,11 @@ def set_node_times(nodes, t):
         node.mocktime = t
         node.setmocktime(t)
 
+def check_node_connections(*, node, num_in, num_out):
+    info = node.getnetworkinfo()
+    assert_equal(info["connections_in"], num_in)
+    assert_equal(info["connections_out"], num_out)
+
 
 def force_finish_mnsync(node):
     """