From e231cc76c66b8c87dd456b9b1c25e6c0573ac5d8 Mon Sep 17 00:00:00 2001 From: kevkevin Date: Fri, 26 Apr 2024 18:28:31 -0500 Subject: [PATCH 1/3] test: use assert_greater_than util and add to where imports are needed In the functional tests there are lots of cases where we assert < which this util will replace, we also are adding the imports and the following commit will add the assertion --- test/functional/feature_coinstatsindex.py | 1 + test/functional/feature_rbf.py | 1 + test/functional/mempool_resurrect.py | 5 ++++- test/functional/p2p_segwit.py | 1 + test/functional/p2p_sendtxrcncl.py | 5 ++++- test/functional/rpc_createmultisig.py | 1 + test/functional/test_framework/blocktools.py | 5 ++++- test/functional/test_framework/netutil.py | 2 ++ test/functional/test_framework/p2p.py | 1 + test/functional/test_framework/script.py | 1 + test/functional/wallet_abandonconflict.py | 1 + test/functional/wallet_conflicts.py | 1 + test/functional/wallet_create_tx.py | 1 + 13 files changed, 23 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index d6c1567e64bca..8fe51b6e0548f 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -28,6 +28,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, ) from test_framework.wallet import ( diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index c5eeaf66e0227..ef37318fd1b5c 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -14,6 +14,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, ) from test_framework.wallet import MiniWallet diff --git a/test/functional/mempool_resurrect.py b/test/functional/mempool_resurrect.py index c10052372d3a2..f6105e7e3cc09 100755 --- a/test/functional/mempool_resurrect.py +++ b/test/functional/mempool_resurrect.py @@ -5,7 +5,10 @@ """Test resurrection of mined transactions when the blockchain is re-organized.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import ( + assert_equal, + assert_greater_than, +) from test_framework.wallet import MiniWallet diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index 213c748edaf35..ba5c143b09567 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -84,6 +84,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, softfork_active, assert_raises_rpc_error, ) diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 8f5e6c0387351..3cc36774a78b8 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -19,7 +19,10 @@ P2P_VERSION, ) from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import ( + assert_equal, + assert_greater_than, +) class PeerNoVerack(P2PInterface): def __init__(self, wtxidrelay=True): diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 65d7b4c4223f4..5fe44e3fe1371 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -17,6 +17,7 @@ from test_framework.util import ( assert_raises_rpc_error, assert_equal, + assert_greater_than, ) from test_framework.wallet_util import generate_keypair from test_framework.wallet import ( diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index cfd923bab3791..b237e07582a91 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -43,7 +43,10 @@ keys_to_multisig_script, script_to_p2wsh_script, ) -from .util import assert_equal +from .util import ( + assert_equal, + assert_greater_than, +) WITNESS_SCALE_FACTOR = 4 MAX_BLOCK_SIGOPS = 20000 diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 08d41fe97fdf5..8f4a0ca79f9ec 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -13,6 +13,8 @@ import array import os +from .util import assert_greater_than + # STATE_ESTABLISHED = '01' # STATE_SYN_SENT = '02' # STATE_SYN_RECV = '03' diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 00bd1e4017aca..efb63251071ce 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -80,6 +80,7 @@ MAX_NODES, p2p_port, wait_until_helper_internal, + assert_greater_than, ) from test_framework.v2_p2p import ( EncryptedP2PState, diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index 7b19d31e1786b..e4f6247beb7a8 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -13,6 +13,7 @@ from .key import TaggedHash, tweak_add_pubkey, compute_xonly_pubkey + from .messages import ( CTransaction, CTxOut, diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index dda48aae1ba70..8d0e9e5bc98f4 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -16,6 +16,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, ) diff --git a/test/functional/wallet_conflicts.py b/test/functional/wallet_conflicts.py index e5739a6a597b8..9d84e2a3843e5 100755 --- a/test/functional/wallet_conflicts.py +++ b/test/functional/wallet_conflicts.py @@ -13,6 +13,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, ) class TxConflicts(BitcoinTestFramework): diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index 4e31b48ec0142..9eeb324251da0 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -6,6 +6,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, ) from test_framework.blocktools import ( From 04d8f07c4317ec4c517ae060a1c2fea2b01416df Mon Sep 17 00:00:00 2001 From: kevkevin Date: Fri, 3 May 2024 10:18:12 -0500 Subject: [PATCH 2/3] scripted-diff: swapped where < was used and used assert_greater_than -BEGIN VERIFY SCRIPT- git grep -l -E "assert .*<{1} [\=\<]{0}" -- ':(exclude)*script.py' ':(exclude)*rpc_createmultisig.py' ':(exclude)*feature_taproot.py' ':(exclude)*wallet_create_tx.py' ':(exclude)*p2p.py' ./test/functional | xargs sed -i.bak -e "/assert .*< / s/assert \(.*\) <\([^#]*\)/assert\2 < \1/" git grep -l -E "assert .*<{1} [\=\<]{0}" -- ':(exclude)*script.py' ':(exclude)*rpc_createmultisig.py' ':(exclude)*feature_taproot.py' ':(exclude)*wallet_create_tx.py' ':(exclude)*p2p.py' ./test/functional | xargs sed -i.bak -e "/assert .*< / s/assert \(.*\) <\([^#]*\)/assert_greater_than(\1,\2)/g" -e "s/assert_greater_than(\(.*\)\(#.*\)/assert_greater_than(\1 \2/g" -e "/assert_greater_than(/ s/\ \ ,/,/g" -END VERIFY SCRIPT- --- test/functional/feature_block.py | 2 +- test/functional/feature_coinstatsindex.py | 2 +- test/functional/feature_rbf.py | 2 +- test/functional/mempool_resurrect.py | 4 ++-- test/functional/p2p_addr_relay.py | 2 +- test/functional/p2p_segwit.py | 4 ++-- test/functional/p2p_sendtxrcncl.py | 2 +- test/functional/rpc_blockchain.py | 6 +++--- test/functional/test_framework/blocktools.py | 2 +- test/functional/test_framework/netutil.py | 2 +- test/functional/wallet_abandonconflict.py | 2 +- test/functional/wallet_bumpfee.py | 2 +- test/functional/wallet_conflicts.py | 2 +- 13 files changed, 17 insertions(+), 17 deletions(-) diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 8a95975184b11..0026d0e2a693c 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -798,7 +798,7 @@ def run_test(self): self.move_tip(57) self.next_block(58, spend=out[17]) tx = CTransaction() - assert len(out[17].vout) < 42 + assert_greater_than(42, len(out[17].vout)) tx.vin.append(CTxIn(COutPoint(out[17].sha256, 42), CScript([OP_TRUE]), SEQUENCE_FINAL)) tx.vout.append(CTxOut(0, b"")) tx.calc_sha256() diff --git a/test/functional/feature_coinstatsindex.py b/test/functional/feature_coinstatsindex.py index 8fe51b6e0548f..2313593d6f919 100755 --- a/test/functional/feature_coinstatsindex.py +++ b/test/functional/feature_coinstatsindex.py @@ -227,7 +227,7 @@ def _test_coin_stats_index(self): self.generate(index_node, 1, sync_fun=self.no_op) res10 = index_node.gettxoutsetinfo('muhash') - assert res8['txouts'] < res10['txouts'] + assert_greater_than(res10['txouts'], res8['txouts']) self.log.info("Test that the index works with -reindex") diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index ef37318fd1b5c..cf48826e6aa6d 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -103,7 +103,7 @@ def make_utxo(self, node, amount, *, confirmed=True, scriptPubKey=None): new_size = len(node.getrawmempool()) # Error out if we have something stuck in the mempool, as this # would likely be a bug. - assert new_size < mempool_size + assert_greater_than(mempool_size, new_size) mempool_size = new_size return self.wallet.get_utxo(txid=tx["txid"], vout=tx["sent_vout"]) diff --git a/test/functional/mempool_resurrect.py b/test/functional/mempool_resurrect.py index f6105e7e3cc09..f5db67b3eae2b 100755 --- a/test/functional/mempool_resurrect.py +++ b/test/functional/mempool_resurrect.py @@ -41,7 +41,7 @@ def run_test(self): assert_equal(set(node.getrawmempool()), set()) confirmed_txns = set(node.getblock(blocks[0])['tx'] + node.getblock(blocks[1])['tx']) # Checks that all spend txns are contained in the mined blocks - assert spends_ids < confirmed_txns + assert_greater_than(confirmed_txns, spends_ids) # Use invalidateblock to re-org back node.invalidateblock(blocks[0]) @@ -54,7 +54,7 @@ def run_test(self): # mempool should be empty, all txns confirmed assert_equal(set(node.getrawmempool()), set()) confirmed_txns = set(node.getblock(blocks[0])['tx']) - assert spends_ids < confirmed_txns + assert_greater_than(confirmed_txns, spends_ids) if __name__ == '__main__': diff --git a/test/functional/p2p_addr_relay.py b/test/functional/p2p_addr_relay.py index b23ec1028b615..bfd099286c47a 100755 --- a/test/functional/p2p_addr_relay.py +++ b/test/functional/p2p_addr_relay.py @@ -113,7 +113,7 @@ def setup_addr_msg(self, num, sequential_ips=True): addr.time = self.mocktime + random.randrange(-100, 100) addr.nServices = P2P_SERVICES if sequential_ips: - assert self.counter < 256 ** 2 # Don't allow the returned ip addresses to wrap. + assert_greater_than(256 ** 2, self.counter) # Don't allow the returned ip addresses to wrap. addr.ip = f"123.123.{self.counter // 256}.{self.counter % 256}" self.counter += 1 else: diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index ba5c143b09567..ff67207c4e7e4 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -847,7 +847,7 @@ def test_block_malleability(self): assert self.nodes[0].getbestblockhash() != block.hash block.vtx[0].wit.vtxinwit[0].scriptWitness.stack.pop() - assert block.get_weight() < MAX_BLOCK_WEIGHT + assert_greater_than(MAX_BLOCK_WEIGHT, block.get_weight()) assert_equal(None, self.nodes[0].submitblock(block.serialize().hex())) assert self.nodes[0].getbestblockhash() == block.hash @@ -1885,7 +1885,7 @@ def test_witness_sigops(self): extra_sigops_available = MAX_SIGOP_COST % sigops_per_script # We chose the number of checkmultisigs/checksigs to make this work: - assert extra_sigops_available < 100 # steer clear of MAX_OPS_PER_SCRIPT + assert_greater_than(100, extra_sigops_available) # steer clear of MAX_OPS_PER_SCRIPT # This script, when spent with the first # N(=MAX_SIGOP_COST//sigops_per_script) outputs of our transaction, diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py index 3cc36774a78b8..a044c2897b9bd 100755 --- a/test/functional/p2p_sendtxrcncl.py +++ b/test/functional/p2p_sendtxrcncl.py @@ -85,7 +85,7 @@ def run_test(self): peer.wait_for_verack() verack_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'verack'][0] sendtxrcncl_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'sendtxrcncl'][0] - assert sendtxrcncl_index < verack_index + assert_greater_than(verack_index, sendtxrcncl_index) self.nodes[0].disconnect_p2ps() self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent') diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 9b7743cafa1be..cde400e982cc6 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -339,7 +339,7 @@ def _test_gettxoutsetinfo(self): assert_equal(res['bestblock'], node.getblockhash(HEIGHT)) size = res['disk_size'] assert size > 6400 - assert size < 64000 + assert_greater_than(64000, size) assert_equal(len(res['bestblock']), 64) assert_equal(len(res['hash_serialized_3']), 64) @@ -433,7 +433,7 @@ def _test_getdifficulty(self): difficulty = self.nodes[0].getdifficulty() # 1 hash in 2 should be valid, so difficulty should be 1/2**31 # binary => decimal => binary math is why we do this check - assert abs(difficulty * 2**31 - 1) < 0.0001 + assert_greater_than(0.0001, abs(difficulty * 2**31 - 1)) def _test_getnetworkhashps(self): self.log.info("Test getnetworkhashps") @@ -475,7 +475,7 @@ def _test_getnetworkhashps(self): # This should be 2 hashes every 10 minutes or 1/300 hashes_per_second = self.nodes[0].getnetworkhashps() - assert abs(hashes_per_second * 300 - 1) < 0.0001 + assert_greater_than(0.0001, abs(hashes_per_second * 300 - 1)) # Test setting the first param of getnetworkhashps to -1 returns the average network # hashes per second from the last difficulty change. diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index b237e07582a91..232e6878ec1e0 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -163,7 +163,7 @@ def create_tx_with_script(prevtx, n, script_sig=b"", *, amount, script_pub_key=C Can optionally pass scriptPubKey and scriptSig, default is anyone-can-spend output. """ tx = CTransaction() - assert n < len(prevtx.vout) + assert_greater_than(len(prevtx.vout), n) tx.vin.append(CTxIn(COutPoint(prevtx.sha256, n), script_sig, SEQUENCE_FINAL)) tx.vout.append(CTxOut(amount, script_pub_key)) tx.calc_sha256() diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 8f4a0ca79f9ec..05ff7bc58765f 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -135,7 +135,7 @@ def addr_to_hex(addr): if i == 0 or i == (len(addr)-1): # skip empty component at beginning or end continue x += 1 # :: skips to suffix - assert x < 2 + assert_greater_than(2, x) else: # two bytes per component val = int(comp, 16) sub[x].append(val >> 8) diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 8d0e9e5bc98f4..fb117402e937f 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -54,7 +54,7 @@ def run_test(self): assert_raises_rpc_error(-5, 'Transaction not eligible for abandonment', lambda: alice.abandontransaction(txid=txA)) newbalance = alice.getbalance() - assert balance - newbalance < Decimal("0.001") #no more than fees lost + assert_greater_than(Decimal("0.001"), balance - newbalance) #no more than fees lost balance = newbalance # Disconnect nodes so node0's transactions don't get into node1's mempool diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py index 5b7db55f4553e..075fbca4d7b21 100755 --- a/test/functional/wallet_bumpfee.py +++ b/test/functional/wallet_bumpfee.py @@ -740,7 +740,7 @@ def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address): def test_bumpfee_metadata(self, rbf_node, dest_address): self.log.info('Test that bumped txn metadata persists to new txn record') - assert rbf_node.getbalance() < 49 + assert_greater_than(49, rbf_node.getbalance()) self.generatetoaddress(rbf_node, 101, rbf_node.getnewaddress()) rbfid = rbf_node.sendtoaddress(dest_address, 49, "comment value", "to value") bumped_tx = rbf_node.bumpfee(rbfid) diff --git a/test/functional/wallet_conflicts.py b/test/functional/wallet_conflicts.py index 9d84e2a3843e5..ef3cd0677711e 100755 --- a/test/functional/wallet_conflicts.py +++ b/test/functional/wallet_conflicts.py @@ -105,7 +105,7 @@ def test_block_conflicts(self): self.log.info("Verify, after the reorg, that Tx_A was accepted, and tx_AB and its Child_Tx are conflicting now") # Tx A was accepted, Tx AB was not. - assert conflicted_AB_tx["confirmations"] < 0 + assert_greater_than(0, conflicted_AB_tx["confirmations"]) assert conflicted_A_tx["confirmations"] > 0 # Conflicted tx should have confirmations set to the confirmations of the most conflicting tx From fe075160f04c3e57fab9571891b17a4ce9f8bec3 Mon Sep 17 00:00:00 2001 From: kevkevin Date: Wed, 1 May 2024 18:13:49 -0500 Subject: [PATCH 3/3] test: using assert_greater_than for edge cases For the cases which the scripted diff did not make sense these were manually updated, like if we had multiple operators on one line or if it was wrapped in () already --- test/functional/rpc_createmultisig.py | 3 ++- test/functional/test_framework/p2p.py | 3 ++- test/functional/test_framework/script.py | 4 ++-- test/functional/wallet_create_tx.py | 3 ++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py index 5fe44e3fe1371..2f905a55be6b8 100755 --- a/test/functional/rpc_createmultisig.py +++ b/test/functional/rpc_createmultisig.py @@ -144,7 +144,8 @@ def checkbalances(self): balw = self.wallet.get_balance() height = node0.getblockchaininfo()["blocks"] - assert 150 < height < 350 + assert_greater_than(350, height) + assert_greater_than(height, 150) total = 149 * 50 + (height - 149 - 100) * 25 assert bal1 == 0 assert bal2 == self.moved diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index efb63251071ce..fee3f3f9dd7c2 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -745,7 +745,8 @@ def listen(cls, p2p, callback, port=None, addr=None, idx=1): for connections, call `callback`.""" if port is None: - assert 0 < idx <= MAX_NODES + assert_greater_than(idx, 0) + assert idx <= MAX_NODES port = p2p_port(MAX_NODES - idx) if addr is None: addr = '127.0.0.1' diff --git a/test/functional/test_framework/script.py b/test/functional/test_framework/script.py index e4f6247beb7a8..2929be06a5977 100644 --- a/test/functional/test_framework/script.py +++ b/test/functional/test_framework/script.py @@ -12,7 +12,7 @@ import unittest from .key import TaggedHash, tweak_add_pubkey, compute_xonly_pubkey - +from .util import assert_greater_than from .messages import ( CTransaction, @@ -814,7 +814,7 @@ def BIP341_sha_outputs(txTo): def TaprootSignatureMsg(txTo, spent_utxos, hash_type, input_index = 0, scriptpath = False, script = CScript(), codeseparator_pos = -1, annex = None, leaf_ver = LEAF_VERSION_TAPSCRIPT): assert (len(txTo.vin) == len(spent_utxos)) - assert (input_index < len(txTo.vin)) + assert_greater_than(len(txTo.vin), input_index) out_type = SIGHASH_ALL if hash_type == 0 else hash_type & 3 in_type = hash_type & SIGHASH_ANYONECANPAY spk = spent_utxos[input_index].scriptPubKey diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index 9eeb324251da0..34e1cf8f00591 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -46,7 +46,8 @@ def test_anti_fee_sniping(self): self.generate(self.nodes[0], 1) txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded'] - assert 0 < tx['locktime'] <= 201 + assert_greater_than(tx['locktime'], 0) + assert tx['locktime'] <= 201 def test_tx_size_too_large(self): # More than 10kB of outputs, so that we hit -maxtxfee with a high feerate