Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: use assert_greater_than util #30019

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/functional/feature_block.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -798,7 +798,7 @@ def run_test(self):
self.move_tip(57) self.move_tip(57)
self.next_block(58, spend=out[17]) self.next_block(58, spend=out[17])
tx = CTransaction() 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.vin.append(CTxIn(COutPoint(out[17].sha256, 42), CScript([OP_TRUE]), SEQUENCE_FINAL))
tx.vout.append(CTxOut(0, b"")) tx.vout.append(CTxOut(0, b""))
tx.calc_sha256() tx.calc_sha256()
Expand Down
3 changes: 2 additions & 1 deletion test/functional/feature_coinstatsindex.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
assert_greater_than,
assert_raises_rpc_error, assert_raises_rpc_error,
) )
from test_framework.wallet import ( from test_framework.wallet import (
Expand Down Expand Up @@ -226,7 +227,7 @@ def _test_coin_stats_index(self):


self.generate(index_node, 1, sync_fun=self.no_op) self.generate(index_node, 1, sync_fun=self.no_op)
res10 = index_node.gettxoutsetinfo('muhash') 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") self.log.info("Test that the index works with -reindex")


Expand Down
3 changes: 2 additions & 1 deletion test/functional/feature_rbf.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py
index cf48826e6a..415e4f1b95 100755
--- a/test/functional/feature_rbf.py
+++ b/test/functional/feature_rbf.py
@@ -15,6 +15,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_greater_than,
+    assert_greater_than_or_equal,
     assert_raises_rpc_error,
 )
 from test_framework.wallet import MiniWallet
@@ -446,9 +447,9 @@ class ReplaceByFeeTest(BitcoinTestFramework):
             num_txs_invalidated = len(root_utxos) + (num_tx_graphs * txs_per_graph)

             if failure_expected:
-                assert num_txs_invalidated > MAX_REPLACEMENT_LIMIT
+                assert_greater_than(num_txs_invalidated, MAX_REPLACEMENT_LIMIT)
             else:
-                assert num_txs_invalidated <= MAX_REPLACEMENT_LIMIT
+                assert_greater_than_or_equal(MAX_REPLACEMENT_LIMIT, num_txs_invalidated)

             # Now attempt to submit a tx that double-spends all the root tx inputs, which
             # would invalidate `num_txs_invalidated` transactions.

Original file line number Original file line Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
assert_greater_than,
assert_raises_rpc_error, assert_raises_rpc_error,
) )
from test_framework.wallet import MiniWallet from test_framework.wallet import MiniWallet
Expand Down Expand Up @@ -102,7 +103,7 @@ def make_utxo(self, node, amount, *, confirmed=True, scriptPubKey=None):
new_size = len(node.getrawmempool()) new_size = len(node.getrawmempool())
# Error out if we have something stuck in the mempool, as this # Error out if we have something stuck in the mempool, as this
# would likely be a bug. # would likely be a bug.
assert new_size < mempool_size assert_greater_than(mempool_size, new_size)
mempool_size = new_size mempool_size = new_size


return self.wallet.get_utxo(txid=tx["txid"], vout=tx["sent_vout"]) return self.wallet.get_utxo(txid=tx["txid"], vout=tx["sent_vout"])
Expand Down
9 changes: 6 additions & 3 deletions test/functional/mempool_resurrect.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
"""Test resurrection of mined transactions when the blockchain is re-organized.""" """Test resurrection of mined transactions when the blockchain is re-organized."""


from test_framework.test_framework import BitcoinTestFramework 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 from test_framework.wallet import MiniWallet




Expand Down Expand Up @@ -38,7 +41,7 @@ def run_test(self):
assert_equal(set(node.getrawmempool()), set()) assert_equal(set(node.getrawmempool()), set())
confirmed_txns = set(node.getblock(blocks[0])['tx'] + node.getblock(blocks[1])['tx']) confirmed_txns = set(node.getblock(blocks[0])['tx'] + node.getblock(blocks[1])['tx'])
# Checks that all spend txns are contained in the mined blocks # 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 # Use invalidateblock to re-org back
node.invalidateblock(blocks[0]) node.invalidateblock(blocks[0])
Expand All @@ -51,7 +54,7 @@ def run_test(self):
# mempool should be empty, all txns confirmed # mempool should be empty, all txns confirmed
assert_equal(set(node.getrawmempool()), set()) assert_equal(set(node.getrawmempool()), set())
confirmed_txns = set(node.getblock(blocks[0])['tx']) confirmed_txns = set(node.getblock(blocks[0])['tx'])
assert spends_ids < confirmed_txns assert_greater_than(confirmed_txns, spends_ids)




if __name__ == '__main__': if __name__ == '__main__':
Expand Down
2 changes: 1 addition & 1 deletion test/functional/p2p_addr_relay.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def setup_addr_msg(self, num, sequential_ips=True):
addr.time = self.mocktime + random.randrange(-100, 100) addr.time = self.mocktime + random.randrange(-100, 100)
addr.nServices = P2P_SERVICES addr.nServices = P2P_SERVICES
if sequential_ips: 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}" addr.ip = f"123.123.{self.counter // 256}.{self.counter % 256}"
self.counter += 1 self.counter += 1
else: else:
Expand Down
5 changes: 3 additions & 2 deletions test/functional/p2p_segwit.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index ff67207c4e..2a4cc9dfba 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -85,6 +85,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     assert_greater_than,
+    assert_greater_than_or_equal,
     softfork_active,
     assert_raises_rpc_error,
 )
@@ -376,14 +377,14 @@ class SegWitTest(BitcoinTestFramework):
         self.test_node.send_message(msg_headers())

         self.test_node.announce_block_and_wait_for_getdata(block1, use_header=False)
-        assert self.test_node.last_message["getdata"].inv[0].type == blocktype
+        assert_equal(self.test_node.last_message["getdata"].inv[0].type, blocktype)
         test_witness_block(self.nodes[0], self.test_node, block1, True)

         block2 = self.build_next_block()
         block2.solve()

         self.test_node.announce_block_and_wait_for_getdata(block2, use_header=True)
-        assert self.test_node.last_message["getdata"].inv[0].type == blocktype
+        assert_equal(self.test_node.last_message["getdata"].inv[0].type, blocktype)
         test_witness_block(self.nodes[0], self.test_node, block2, True)

         # Check that we can getdata for witness blocks or regular blocks,
@@ -412,8 +413,8 @@ class SegWitTest(BitcoinTestFramework):
             block = self.build_next_block()
             self.update_witness_block_with_transactions(block, [])
             # This gives us a witness commitment.
-            assert len(block.vtx[0].wit.vtxinwit) == 1
-            assert len(block.vtx[0].wit.vtxinwit[0].scriptWitness.stack) == 1
+            assert_equal(len(block.vtx[0].wit.vtxinwit), 1)
+            assert_equal(len(block.vtx[0].wit.vtxinwit[0].scriptWitness.stack), 1)
             test_witness_block(self.nodes[0], self.test_node, block, accepted=True)
             # Now try to retrieve it...
             rpc_block = self.nodes[0].getblock(block.hash, False)
@@ -539,7 +540,7 @@ class SegWitTest(BitcoinTestFramework):
         # Verify that if a peer doesn't set nServices to include NODE_WITNESS,
         # the getdata is just for the non-witness portion.
         self.old_node.announce_tx_and_wait_for_getdata(tx)
-        assert self.old_node.last_message["getdata"].inv[0].type == MSG_TX
+        assert_equal(self.old_node.last_message["getdata"].inv[0].type, MSG_TX)

         # Since we haven't delivered the tx yet, inv'ing the same tx from
         # a witness transaction ought not result in a getdata.
@@ -807,7 +808,7 @@ class SegWitTest(BitcoinTestFramework):
         block_3.vtx[0].vout[-1].nValue += 1
         block_3.vtx[0].rehash()
         block_3.hashMerkleRoot = block_3.calc_merkle_root()
-        assert len(block_3.vtx[0].vout) == 4  # 3 OP_returns
+        assert_equal(len(block_3.vtx[0].vout), 4)  # 3 OP_returns
         block_3.solve()
         test_witness_block(self.nodes[0], self.test_node, block_3, accepted=True)

@@ -838,7 +839,7 @@ class SegWitTest(BitcoinTestFramework):
         block.solve()

         block.vtx[0].wit.vtxinwit[0].scriptWitness.stack.append(b'a' * 5000000)
-        assert block.get_weight() > MAX_BLOCK_WEIGHT
+        assert_greater_than(block.get_weight(), MAX_BLOCK_WEIGHT)

         # We can't send over the p2p network, because this is too big to relay
         # TODO: repeat this test with a block that can be relayed
@@ -850,7 +851,7 @@ class SegWitTest(BitcoinTestFramework):
         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
+        assert_equal(self.nodes[0].getbestblockhash(), block.hash)

         # Now make sure that malleating the witness reserved value doesn't
         # result in a block permanently marked bad.
@@ -875,7 +876,7 @@ class SegWitTest(BitcoinTestFramework):
         # Test that witness-bearing blocks are limited at ceil(base + wit/4) <= 1MB.
         block = self.build_next_block()

-        assert len(self.utxo) > 0
+        assert_greater_than(len(self.utxo), 0)

         # Create a P2WSH transaction.
         # The witness script will be a bunch of OP_2DROP's, followed by OP_TRUE.
@@ -896,7 +897,7 @@ class SegWitTest(BitcoinTestFramework):
         for _ in range(NUM_OUTPUTS):
             parent_tx.vout.append(CTxOut(child_value, script_pubkey))
         parent_tx.vout[0].nValue -= 50000
-        assert parent_tx.vout[0].nValue > 0
+        assert_greater_than(parent_tx.vout[0].nValue, 0)
         parent_tx.rehash()

         child_tx = CTransaction()
@@ -924,7 +925,7 @@ class SegWitTest(BitcoinTestFramework):
         assert_equal(block.get_weight(), MAX_BLOCK_WEIGHT + 1)
         # Make sure that our test case would exceed the old max-network-message
         # limit
-        assert len(block.serialize()) > 2 * 1024 * 1024
+        assert_greater_than(len(block.serialize()), 2 * 1024 * 1024)

         test_witness_block(self.nodes[0], self.test_node, block, accepted=False, reason='bad-blk-we
ight')

@@ -934,7 +935,7 @@ class SegWitTest(BitcoinTestFramework):
         block.vtx[0].vout.pop()
         add_witness_commitment(block)
         block.solve()
-        assert block.get_weight() == MAX_BLOCK_WEIGHT
+        assert_equal(block.get_weight(), MAX_BLOCK_WEIGHT)

         test_witness_block(self.nodes[0], self.test_node, block, accepted=True)

@@ -1098,7 +1099,7 @@ class SegWitTest(BitcoinTestFramework):

         # This script is 19 max pushes (9937 bytes), then 64 more opcode-bytes.
         long_witness_script = CScript([b'a' * MAX_SCRIPT_ELEMENT_SIZE] * 19 + [OP_DROP] * 63 + [OP_
TRUE])
-        assert len(long_witness_script) == MAX_WITNESS_SCRIPT_LENGTH + 1
+        assert_equal(len(long_witness_script), MAX_WITNESS_SCRIPT_LENGTH + 1)
         long_script_pubkey = script_to_p2wsh_script(long_witness_script)

         block = self.build_next_block()
@@ -1122,7 +1123,7 @@ class SegWitTest(BitcoinTestFramework):

         # Try again with one less byte in the witness script
         witness_script = CScript([b'a' * MAX_SCRIPT_ELEMENT_SIZE] * 19 + [OP_DROP] * 62 + [OP_TRUE]
)
-        assert len(witness_script) == MAX_WITNESS_SCRIPT_LENGTH
+        assert_equal(len(witness_script), MAX_WITNESS_SCRIPT_LENGTH)
         script_pubkey = script_to_p2wsh_script(witness_script)

         tx.vout[0] = CTxOut(tx.vout[0].nValue, script_pubkey)
@@ -1151,7 +1152,7 @@ class SegWitTest(BitcoinTestFramework):
         for _ in range(10):
             tx.vout.append(CTxOut(int(value / 10), script_pubkey))
         tx.vout[0].nValue -= 1000
-        assert tx.vout[0].nValue >= 0
+        assert_greater_than_or_equal(tx.vout[0].nValue, 0)

         block = self.build_next_block()
         self.update_witness_block_with_transactions(block, [tx])
@@ -1358,7 +1359,7 @@ class SegWitTest(BitcoinTestFramework):
             temp_utxo.append(UTXO(tx.sha256, 0, tx.vout[0].nValue))

         self.generate(self.nodes[0], 1)  # Mine all the transactions
-        assert len(self.nodes[0].getrawmempool()) == 0
+        assert_equal(len(self.nodes[0].getrawmempool()), 0)

         # Finally, verify that version 0 -> version 2 transactions
         # are standard
@@ -1622,7 +1623,7 @@ class SegWitTest(BitcoinTestFramework):
             # Create a slight bias for producing more utxos
             num_outputs = random.randint(1, 11)
             random.shuffle(temp_utxos)
-            assert len(temp_utxos) > num_inputs
+            assert_greater_than(len(temp_utxos), num_inputs)
             tx = CTransaction()
             total_value = 0
             for i in range(num_inputs):

Original file line number Original file line Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
assert_greater_than,
softfork_active, softfork_active,
assert_raises_rpc_error, assert_raises_rpc_error,
) )
Expand Down Expand Up @@ -846,7 +847,7 @@ def test_block_malleability(self):
assert self.nodes[0].getbestblockhash() != block.hash assert self.nodes[0].getbestblockhash() != block.hash


block.vtx[0].wit.vtxinwit[0].scriptWitness.stack.pop() 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_equal(None, self.nodes[0].submitblock(block.serialize().hex()))


assert self.nodes[0].getbestblockhash() == block.hash assert self.nodes[0].getbestblockhash() == block.hash
Expand Down Expand Up @@ -1884,7 +1885,7 @@ def test_witness_sigops(self):
extra_sigops_available = MAX_SIGOP_COST % sigops_per_script extra_sigops_available = MAX_SIGOP_COST % sigops_per_script


# We chose the number of checkmultisigs/checksigs to make this work: # 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 # This script, when spent with the first
# N(=MAX_SIGOP_COST//sigops_per_script) outputs of our transaction, # N(=MAX_SIGOP_COST//sigops_per_script) outputs of our transaction,
Expand Down
7 changes: 5 additions & 2 deletions test/functional/p2p_sendtxrcncl.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@
P2P_VERSION, P2P_VERSION,
) )
from test_framework.test_framework import BitcoinTestFramework 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): class PeerNoVerack(P2PInterface):
def __init__(self, wtxidrelay=True): def __init__(self, wtxidrelay=True):
Expand Down Expand Up @@ -82,7 +85,7 @@ def run_test(self):
peer.wait_for_verack() peer.wait_for_verack()
verack_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'verack'][0] 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] 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.nodes[0].disconnect_p2ps()


self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent') self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent')
Expand Down
6 changes: 3 additions & 3 deletions test/functional/rpc_blockchain.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ def _test_gettxoutsetinfo(self):
assert_equal(res['bestblock'], node.getblockhash(HEIGHT)) assert_equal(res['bestblock'], node.getblockhash(HEIGHT))
size = res['disk_size'] size = res['disk_size']
assert size > 6400 assert size > 6400
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert size > 6400
assert_greater_than(size, 6400)

assert size < 64000 assert_greater_than(64000, size)
assert_equal(len(res['bestblock']), 64) assert_equal(len(res['bestblock']), 64)
assert_equal(len(res['hash_serialized_3']), 64) assert_equal(len(res['hash_serialized_3']), 64)


Expand Down Expand Up @@ -433,7 +433,7 @@ def _test_getdifficulty(self):
difficulty = self.nodes[0].getdifficulty() difficulty = self.nodes[0].getdifficulty()
# 1 hash in 2 should be valid, so difficulty should be 1/2**31 # 1 hash in 2 should be valid, so difficulty should be 1/2**31
# binary => decimal => binary math is why we do this check # 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): def _test_getnetworkhashps(self):
self.log.info("Test getnetworkhashps") self.log.info("Test getnetworkhashps")
Expand Down Expand Up @@ -475,7 +475,7 @@ def _test_getnetworkhashps(self):


# This should be 2 hashes every 10 minutes or 1/300 # This should be 2 hashes every 10 minutes or 1/300
hashes_per_second = self.nodes[0].getnetworkhashps() 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 # Test setting the first param of getnetworkhashps to -1 returns the average network
# hashes per second from the last difficulty change. # hashes per second from the last difficulty change.
Expand Down
4 changes: 3 additions & 1 deletion test/functional/rpc_createmultisig.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from test_framework.util import ( from test_framework.util import (
assert_raises_rpc_error, assert_raises_rpc_error,
assert_equal, assert_equal,
assert_greater_than,
) )
from test_framework.wallet_util import generate_keypair from test_framework.wallet_util import generate_keypair
from test_framework.wallet import ( from test_framework.wallet import (
Expand Down Expand Up @@ -143,7 +144,8 @@ def checkbalances(self):
balw = self.wallet.get_balance() balw = self.wallet.get_balance()


height = node0.getblockchaininfo()["blocks"] 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 total = 149 * 50 + (height - 149 - 100) * 25
assert bal1 == 0 assert bal1 == 0
assert bal2 == self.moved assert bal2 == self.moved
Comment on lines 150 to 151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change these to assert_equal is an easy gain here.

Expand Down
7 changes: 5 additions & 2 deletions test/functional/test_framework/blocktools.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@
keys_to_multisig_script, keys_to_multisig_script,
script_to_p2wsh_script, script_to_p2wsh_script,
) )
from .util import assert_equal from .util import (
assert_equal,
assert_greater_than,
)


WITNESS_SCALE_FACTOR = 4 WITNESS_SCALE_FACTOR = 4
MAX_BLOCK_SIGOPS = 20000 MAX_BLOCK_SIGOPS = 20000
Expand Down Expand Up @@ -160,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. Can optionally pass scriptPubKey and scriptSig, default is anyone-can-spend output.
""" """
tx = CTransaction() 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.vin.append(CTxIn(COutPoint(prevtx.sha256, n), script_sig, SEQUENCE_FINAL))
tx.vout.append(CTxOut(amount, script_pub_key)) tx.vout.append(CTxOut(amount, script_pub_key))
tx.calc_sha256() tx.calc_sha256()
Expand Down
4 changes: 3 additions & 1 deletion test/functional/test_framework/netutil.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
import array import array
import os import os


from .util import assert_greater_than

# STATE_ESTABLISHED = '01' # STATE_ESTABLISHED = '01'
# STATE_SYN_SENT = '02' # STATE_SYN_SENT = '02'
# STATE_SYN_RECV = '03' # STATE_SYN_RECV = '03'
Expand Down Expand Up @@ -133,7 +135,7 @@ def addr_to_hex(addr):
if i == 0 or i == (len(addr)-1): # skip empty component at beginning or end if i == 0 or i == (len(addr)-1): # skip empty component at beginning or end
continue continue
x += 1 # :: skips to suffix x += 1 # :: skips to suffix
assert x < 2 assert_greater_than(2, x)
else: # two bytes per component else: # two bytes per component
val = int(comp, 16) val = int(comp, 16)
sub[x].append(val >> 8) sub[x].append(val >> 8)
Expand Down
4 changes: 3 additions & 1 deletion test/functional/test_framework/p2p.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
MAX_NODES, MAX_NODES,
p2p_port, p2p_port,
wait_until_helper_internal, wait_until_helper_internal,
assert_greater_than,
) )
from test_framework.v2_p2p import ( from test_framework.v2_p2p import (
EncryptedP2PState, EncryptedP2PState,
Expand Down Expand Up @@ -744,7 +745,8 @@ def listen(cls, p2p, callback, port=None, addr=None, idx=1):
for connections, call `callback`.""" for connections, call `callback`."""


if port is None: if port is None:
assert 0 < idx <= MAX_NODES assert_greater_than(idx, 0)
assert idx <= MAX_NODES
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert idx <= MAX_NODES
assert_greater_than(MAX_NODES + 1, idx)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why MAX_NODES + 1?

Copy link
Contributor

@paplorinc paplorinc May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idx can equal MAX_NODES according to the code - alternatively could use assert_greater_than_or_equal, if that's more readable

port = p2p_port(MAX_NODES - idx) port = p2p_port(MAX_NODES - idx)
if addr is None: if addr is None:
addr = '127.0.0.1' addr = '127.0.0.1'
Expand Down
3 changes: 2 additions & 1 deletion test/functional/test_framework/script.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import unittest import unittest


from .key import TaggedHash, tweak_add_pubkey, compute_xonly_pubkey from .key import TaggedHash, tweak_add_pubkey, compute_xonly_pubkey
from .util import assert_greater_than


from .messages import ( from .messages import (
CTransaction, CTransaction,
Expand Down Expand Up @@ -813,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): 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 (len(txTo.vin) == len(spent_utxos))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert (len(txTo.vin) == len(spent_utxos))
assert_equal(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 out_type = SIGHASH_ALL if hash_type == 0 else hash_type & 3
in_type = hash_type & SIGHASH_ANYONECANPAY in_type = hash_type & SIGHASH_ANYONECANPAY
spk = spent_utxos[input_index].scriptPubKey spk = spent_utxos[input_index].scriptPubKey
Expand Down
3 changes: 2 additions & 1 deletion test/functional/wallet_abandonconflict.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
assert_greater_than,
assert_raises_rpc_error, assert_raises_rpc_error,
) )


Expand Down Expand Up @@ -53,7 +54,7 @@ def run_test(self):
assert_raises_rpc_error(-5, 'Transaction not eligible for abandonment', lambda: alice.abandontransaction(txid=txA)) assert_raises_rpc_error(-5, 'Transaction not eligible for abandonment', lambda: alice.abandontransaction(txid=txA))


newbalance = alice.getbalance() 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed as this specific test is full of Decimal objects. This is what you get if you take it out.

❯ test/functional/wallet_abandonconflict.py
2024-05-07T21:36:52.495000Z TestFramework (INFO): PRNG seed is: 7791852828985173276
2024-05-07T21:36:52.495000Z TestFramework (INFO): Initializing test directory /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_8oozog30
2024-05-07T21:36:55.004000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
  File "/Users/jose.edil/2-development/bitcoin/bitcoin-core/test/functional/test_framework/test_framework.py", line 132, in main
    self.run_test()
  File "/Users/jose.edil/2-development/bitcoin/bitcoin-core/test/functional/wallet_abandonconflict.py", line 57, in run_test
    assert_greater_than("0.001", balance - newbalance) #no more than fees lost
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jose.edil/2-development/bitcoin/bitcoin-core/test/functional/test_framework/util.py", line 78, in assert_greater_than
    if thing1 <= thing2:
       ^^^^^^^^^^^^^^^^
TypeError: '<=' not supported between instances of 'str' and 'decimal.Decimal'
2024-05-07T21:36:55.057000Z TestFramework (INFO): Stopping nodes
2024-05-07T21:36:55.165000Z TestFramework (WARNING): Not cleaning up dir /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_8oozog30
2024-05-07T21:36:55.165000Z TestFramework (ERROR): Test failed. Test logging available at /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_8oozog30/test_framework.log
2024-05-07T21:36:55.165000Z TestFramework (ERROR):
2024-05-07T21:36:55.165000Z TestFramework (ERROR): Hint: Call /Users/jose.edil/2-development/bitcoin/bitcoin-core/test/functional/combine_logs.py '/var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_8oozog30' to consolidate all logs
2024-05-07T21:36:55.165000Z TestFramework (ERROR):
2024-05-07T21:36:55.165000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-05-07T21:36:55.165000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2024-05-07T21:36:55.166000Z TestFramework (ERROR):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you are a newcomer too. When reviewing code, better to open the files in the text editor and check for a broader context than what the github interface will present.

In my case, I use emacs with ediff, but all major IDEs will have a nice diff interface to inspect the PRs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you are a newcomer too.

this isn't necessary...

assert_greater_than("0.001", balance - newbalance)

I wasn't suggesting comparing it to a string, of course, but to a decimal, as in the example I've provided, i.e.:

assert_greater_than(0.001, balance - newbalance)

This seems to be working for me, is it failing for you?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see.

Still, better to leave to preserve potential bugs since this is refactoring.

balance = newbalance balance = newbalance


# Disconnect nodes so node0's transactions don't get into node1's mempool # Disconnect nodes so node0's transactions don't get into node1's mempool
Expand Down
2 changes: 1 addition & 1 deletion test/functional/wallet_bumpfee.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address):


def test_bumpfee_metadata(self, rbf_node, dest_address): def test_bumpfee_metadata(self, rbf_node, dest_address):
self.log.info('Test that bumped txn metadata persists to new txn record') 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()) self.generatetoaddress(rbf_node, 101, rbf_node.getnewaddress())
rbfid = rbf_node.sendtoaddress(dest_address, 49, "comment value", "to value") rbfid = rbf_node.sendtoaddress(dest_address, 49, "comment value", "to value")
bumped_tx = rbf_node.bumpfee(rbfid) bumped_tx = rbf_node.bumpfee(rbfid)
Expand Down
3 changes: 2 additions & 1 deletion test/functional/wallet_conflicts.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
assert_greater_than,
) )


class TxConflicts(BitcoinTestFramework): class TxConflicts(BitcoinTestFramework):
Expand Down Expand Up @@ -104,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") 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. # 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 assert conflicted_A_tx["confirmations"] > 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like an actual assert_greater_than

Suggested change
assert conflicted_A_tx["confirmations"] > 0
assert_greater_than(conflicted_A_tx["confirmations"], 0)



# Conflicted tx should have confirmations set to the confirmations of the most conflicting tx # Conflicted tx should have confirmations set to the confirmations of the most conflicting tx
Expand Down
4 changes: 3 additions & 1 deletion test/functional/wallet_create_tx.py
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from test_framework.test_framework import BitcoinTestFramework from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import ( from test_framework.util import (
assert_equal, assert_equal,
assert_greater_than,
assert_raises_rpc_error, assert_raises_rpc_error,
) )
from test_framework.blocktools import ( from test_framework.blocktools import (
Expand Down Expand Up @@ -45,7 +46,8 @@ def test_anti_fee_sniping(self):
self.generate(self.nodes[0], 1) self.generate(self.nodes[0], 1)
txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)
tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded'] 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert tx['locktime'] <= 201
assert_greater_than(202, tx['locktime'])



def test_tx_size_too_large(self): def test_tx_size_too_large(self):
# More than 10kB of outputs, so that we hit -maxtxfee with a high feerate # More than 10kB of outputs, so that we hit -maxtxfee with a high feerate
Expand Down