From 9547b884d201901dd9608dc8aeaffbe242fa522b Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Mon, 26 Aug 2024 19:35:00 +0200 Subject: [PATCH] test: switch MiniWallet padding unit from weight to vsize The weight unit is merely a consensus rule detail and largely irrelevant for fee-rate calculations and mempool policy rules (e.g. for package relay and TRUC limits), so there doesn't seem to be any value of using a granularity that we can't even guarantee to reach exactly anyway. Switch to the more natural unit of vsize instead, which simplifies both the padding implementation and the current tests that take use of this padding. The rather annoying multiplications by `WITNESS_SCALE_FACTOR` can then be removed and weird-looking magic numbers like `4004` can be replaced by numbers that are more connected to actual policy limit constants from the codebase, e.g. `1001` for exceeding `TRUC_CHILD_MAX_VSIZE` by one. --- test/functional/feature_blocksxor.py | 2 +- .../feature_framework_miniwallet.py | 16 ++++----- test/functional/mempool_limit.py | 31 ++++++++-------- test/functional/mempool_package_limits.py | 17 ++++----- test/functional/mempool_truc.py | 35 +++++++++---------- test/functional/test_framework/wallet.py | 30 +++++++--------- 6 files changed, 61 insertions(+), 70 deletions(-) diff --git a/test/functional/feature_blocksxor.py b/test/functional/feature_blocksxor.py index 88e0244cd420f6..1dea95769f8f9a 100755 --- a/test/functional/feature_blocksxor.py +++ b/test/functional/feature_blocksxor.py @@ -30,7 +30,7 @@ def run_test(self): node = self.nodes[0] wallet = MiniWallet(node) for _ in range(5): - wallet.send_self_transfer(from_node=node, target_weight=80000) + wallet.send_self_transfer(from_node=node, target_vsize=20000) self.generate(wallet, 1) block_files = list(node.blocks_path.glob('blk[0-9][0-9][0-9][0-9][0-9].dat')) diff --git a/test/functional/feature_framework_miniwallet.py b/test/functional/feature_framework_miniwallet.py index d1aa24e7cd7fea..f723f7f31ee095 100755 --- a/test/functional/feature_framework_miniwallet.py +++ b/test/functional/feature_framework_miniwallet.py @@ -9,7 +9,7 @@ from test_framework.blocktools import COINBASE_MATURITY from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( - assert_greater_than_or_equal, + assert_equal, ) from test_framework.wallet import ( MiniWallet, @@ -22,17 +22,15 @@ def set_test_params(self): self.num_nodes = 1 def test_tx_padding(self): - """Verify that MiniWallet's transaction padding (`target_weight` parameter) - works accurately enough (i.e. at most 3 WUs higher) with all modes.""" + """Verify that MiniWallet's transaction padding (`target_vsize` parameter) + works accurately with all modes.""" for mode_name, wallet in self.wallets: self.log.info(f"Test tx padding with MiniWallet mode {mode_name}...") utxo = wallet.get_utxo(mark_as_spent=False) - for target_weight in [1000, 2000, 5000, 10000, 20000, 50000, 100000, 200000, 4000000, - 989, 2001, 4337, 13371, 23219, 49153, 102035, 223419, 3999989]: - tx = wallet.create_self_transfer(utxo_to_spend=utxo, target_weight=target_weight)["tx"] - self.log.debug(f"-> target weight: {target_weight}, actual weight: {tx.get_weight()}") - assert_greater_than_or_equal(tx.get_weight(), target_weight) - assert_greater_than_or_equal(target_weight + 3, tx.get_weight()) + for target_vsize in [250, 500, 1250, 2500, 5000, 12500, 25000, 50000, 1000000, + 248, 501, 1085, 3343, 5805, 12289, 25509, 55855, 999998]: + tx = wallet.create_self_transfer(utxo_to_spend=utxo, target_vsize=target_vsize)["tx"] + assert_equal(tx.get_vsize(), target_vsize) def test_wallet_tagging(self): """Verify that tagged wallet instances are able to send funds.""" diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py index 626928a49a90c5..239c13bca14860 100755 --- a/test/functional/mempool_limit.py +++ b/test/functional/mempool_limit.py @@ -55,12 +55,12 @@ def test_rbf_carveout_disallowed(self): self.generate(node, 1) # tx_A needs to be RBF'd, set minfee at set size - A_weight = 1000 + A_vsize = 250 mempoolmin_feerate = node.getmempoolinfo()["mempoolminfee"] tx_A = self.wallet.send_self_transfer( from_node=node, fee_rate=mempoolmin_feerate, - target_weight=A_weight, + target_vsize=A_vsize, utxo_to_spend=rbf_utxo, confirmed_only=True ) @@ -68,15 +68,15 @@ def test_rbf_carveout_disallowed(self): # RBF's tx_A, is not yet submitted tx_B = self.wallet.create_self_transfer( fee=tx_A["fee"] * 4, - target_weight=A_weight, + target_vsize=A_vsize, utxo_to_spend=rbf_utxo, confirmed_only=True ) # Spends tx_B's output, too big for cpfp carveout (because that would also increase the descendant limit by 1) - non_cpfp_carveout_weight = 40001 # EXTRA_DESCENDANT_TX_SIZE_LIMIT + 1 + non_cpfp_carveout_vsize = 10001 # EXTRA_DESCENDANT_TX_SIZE_LIMIT + 1 tx_C = self.wallet.create_self_transfer( - target_weight=non_cpfp_carveout_weight, + target_vsize=non_cpfp_carveout_vsize, fee_rate=mempoolmin_feerate, utxo_to_spend=tx_B["new_utxo"], confirmed_only=True @@ -103,14 +103,14 @@ def test_mid_package_eviction(self): # UTXOs to be spent by the ultimate child transaction parent_utxos = [] - evicted_weight = 8000 + evicted_vsize = 2000 # Mempool transaction which is evicted due to being at the "bottom" of the mempool when the # mempool overflows and evicts by descendant score. It's important that the eviction doesn't # happen in the middle of package evaluation, as it can invalidate the coins cache. mempool_evicted_tx = self.wallet.send_self_transfer( from_node=node, fee_rate=mempoolmin_feerate, - target_weight=evicted_weight, + target_vsize=evicted_vsize, confirmed_only=True ) # Already in mempool when package is submitted. @@ -132,14 +132,16 @@ def test_mid_package_eviction(self): # Series of parents that don't need CPFP and are submitted individually. Each one is large and # high feerate, which means they should trigger eviction but not be evicted. - parent_weight = 100000 + parent_vsize = 25000 num_big_parents = 3 - assert_greater_than(parent_weight * num_big_parents, current_info["maxmempool"] - current_info["bytes"]) + # Need to be large enough to trigger eviction + # (note that the mempool usage of a tx is about three times its vsize) + assert_greater_than(parent_vsize * num_big_parents * 3, current_info["maxmempool"] - current_info["bytes"]) parent_feerate = 100 * mempoolmin_feerate big_parent_txids = [] for i in range(num_big_parents): - parent = self.wallet.create_self_transfer(fee_rate=parent_feerate, target_weight=parent_weight, confirmed_only=True) + parent = self.wallet.create_self_transfer(fee_rate=parent_feerate, target_vsize=parent_vsize, confirmed_only=True) parent_utxos.append(parent["new_utxo"]) package_hex.append(parent["hex"]) big_parent_txids.append(parent["txid"]) @@ -311,8 +313,9 @@ def run_test(self): entry = node.getmempoolentry(txid) worst_feerate_btcvb = min(worst_feerate_btcvb, entry["fees"]["descendant"] / entry["descendantsize"]) # Needs to be large enough to trigger eviction - target_weight_each = 200000 - assert_greater_than(target_weight_each * 2, node.getmempoolinfo()["maxmempool"] - node.getmempoolinfo()["bytes"]) + # (note that the mempool usage of a tx is about three times its vsize) + target_vsize_each = 50000 + assert_greater_than(target_vsize_each * 3, node.getmempoolinfo()["maxmempool"] - node.getmempoolinfo()["bytes"]) # Should be a true CPFP: parent's feerate is just below mempool min feerate parent_feerate = mempoolmin_feerate - Decimal("0.000001") # 0.1 sats/vbyte below min feerate # Parent + child is above mempool minimum feerate @@ -320,8 +323,8 @@ def run_test(self): # However, when eviction is triggered, these transactions should be at the bottom. # This assertion assumes parent and child are the same size. miniwallet.rescan_utxos() - tx_parent_just_below = miniwallet.create_self_transfer(fee_rate=parent_feerate, target_weight=target_weight_each) - tx_child_just_above = miniwallet.create_self_transfer(utxo_to_spend=tx_parent_just_below["new_utxo"], fee_rate=child_feerate, target_weight=target_weight_each) + tx_parent_just_below = miniwallet.create_self_transfer(fee_rate=parent_feerate, target_vsize=target_vsize_each) + tx_child_just_above = miniwallet.create_self_transfer(utxo_to_spend=tx_parent_just_below["new_utxo"], fee_rate=child_feerate, target_vsize=target_vsize_each) # This package ranks below the lowest descendant package in the mempool package_fee = tx_parent_just_below["fee"] + tx_child_just_above["fee"] package_vsize = tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize() diff --git a/test/functional/mempool_package_limits.py b/test/functional/mempool_package_limits.py index 6e26a684e22eca..3290ff43c4924f 100755 --- a/test/functional/mempool_package_limits.py +++ b/test/functional/mempool_package_limits.py @@ -4,9 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test logic for limiting mempool and package ancestors/descendants.""" from test_framework.blocktools import COINBASE_MATURITY -from test_framework.messages import ( - WITNESS_SCALE_FACTOR, -) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, @@ -290,19 +287,18 @@ def test_anc_size_limits(self): parent_utxos = [] target_vsize = 30_000 high_fee = 10 * target_vsize # 10 sats/vB - target_weight = target_vsize * WITNESS_SCALE_FACTOR self.log.info("Check that in-mempool and in-package ancestor size limits are calculated properly in packages") # Mempool transactions A and B for _ in range(2): - bulked_tx = self.wallet.create_self_transfer(target_weight=target_weight) + bulked_tx = self.wallet.create_self_transfer(target_vsize=target_vsize) self.wallet.sendrawtransaction(from_node=node, tx_hex=bulked_tx["hex"]) parent_utxos.append(bulked_tx["new_utxo"]) # Package transaction C - pc_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_utxos, fee_per_output=high_fee, target_weight=target_weight) + pc_tx = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_utxos, fee_per_output=high_fee, target_vsize=target_vsize) # Package transaction D - pd_tx = self.wallet.create_self_transfer(utxo_to_spend=pc_tx["new_utxos"][0], target_weight=target_weight) + pd_tx = self.wallet.create_self_transfer(utxo_to_spend=pc_tx["new_utxos"][0], target_vsize=target_vsize) assert_equal(2, node.getmempoolinfo()["size"]) return [pc_tx["hex"], pd_tx["hex"]] @@ -321,20 +317,19 @@ def test_desc_size_limits(self): node = self.nodes[0] target_vsize = 21_000 high_fee = 10 * target_vsize # 10 sats/vB - target_weight = target_vsize * WITNESS_SCALE_FACTOR self.log.info("Check that in-mempool and in-package descendant sizes are calculated properly in packages") # Top parent in mempool, Ma - ma_tx = self.wallet.create_self_transfer_multi(num_outputs=2, fee_per_output=high_fee // 2, target_weight=target_weight) + ma_tx = self.wallet.create_self_transfer_multi(num_outputs=2, fee_per_output=high_fee // 2, target_vsize=target_vsize) self.wallet.sendrawtransaction(from_node=node, tx_hex=ma_tx["hex"]) package_hex = [] for j in range(2): # Two legs (left and right) # Mempool transaction (Mb and Mc) - mempool_tx = self.wallet.create_self_transfer(utxo_to_spend=ma_tx["new_utxos"][j], target_weight=target_weight) + mempool_tx = self.wallet.create_self_transfer(utxo_to_spend=ma_tx["new_utxos"][j], target_vsize=target_vsize) self.wallet.sendrawtransaction(from_node=node, tx_hex=mempool_tx["hex"]) # Package transaction (Pd and Pe) - package_tx = self.wallet.create_self_transfer(utxo_to_spend=mempool_tx["new_utxo"], target_weight=target_weight) + package_tx = self.wallet.create_self_transfer(utxo_to_spend=mempool_tx["new_utxo"], target_vsize=target_vsize) package_hex.append(package_tx["hex"]) assert_equal(3, node.getmempoolinfo()["size"]) diff --git a/test/functional/mempool_truc.py b/test/functional/mempool_truc.py index 28f3256ef1b756..a7b5e83e899218 100755 --- a/test/functional/mempool_truc.py +++ b/test/functional/mempool_truc.py @@ -6,7 +6,6 @@ from test_framework.messages import ( MAX_BIP125_RBF_SEQUENCE, - WITNESS_SCALE_FACTOR, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( @@ -55,14 +54,14 @@ def check_mempool(self, txids): def test_truc_max_vsize(self): node = self.nodes[0] self.log.info("Test TRUC-specific maximum transaction vsize") - tx_v3_heavy = self.wallet.create_self_transfer(target_weight=(TRUC_MAX_VSIZE + 1) * WITNESS_SCALE_FACTOR, version=3) + tx_v3_heavy = self.wallet.create_self_transfer(target_vsize=TRUC_MAX_VSIZE + 1, version=3) assert_greater_than_or_equal(tx_v3_heavy["tx"].get_vsize(), TRUC_MAX_VSIZE) expected_error_heavy = f"TRUC-violation, version=3 tx {tx_v3_heavy['txid']} (wtxid={tx_v3_heavy['wtxid']}) is too big" assert_raises_rpc_error(-26, expected_error_heavy, node.sendrawtransaction, tx_v3_heavy["hex"]) self.check_mempool([]) # Ensure we are hitting the TRUC-specific limit and not something else - tx_v2_heavy = self.wallet.send_self_transfer(from_node=node, target_weight=(TRUC_MAX_VSIZE + 1) * WITNESS_SCALE_FACTOR, version=2) + tx_v2_heavy = self.wallet.send_self_transfer(from_node=node, target_vsize=TRUC_MAX_VSIZE + 1, version=2) self.check_mempool([tx_v2_heavy["txid"]]) @cleanup(extra_args=["-datacarriersize=1000"]) @@ -73,7 +72,7 @@ def test_truc_acceptance(self): self.check_mempool([tx_v3_parent_normal["txid"]]) tx_v3_child_heavy = self.wallet.create_self_transfer( utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_weight=4004, + target_vsize=1001, version=3 ) assert_greater_than_or_equal(tx_v3_child_heavy["tx"].get_vsize(), 1000) @@ -88,7 +87,7 @@ def test_truc_acceptance(self): from_node=node, fee_rate=DEFAULT_FEE, utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_weight=3987, + target_vsize=997, version=3 ) assert_greater_than_or_equal(1000, tx_v3_child_almost_heavy["tx"].get_vsize()) @@ -98,7 +97,7 @@ def test_truc_acceptance(self): from_node=node, fee_rate=DEFAULT_FEE * 2, utxo_to_spend=tx_v3_parent_normal["new_utxo"], - target_weight=3500, + target_vsize=875, version=3 ) assert_greater_than_or_equal(tx_v3_child_almost_heavy["tx"].get_vsize() + tx_v3_child_almost_heavy_rbf["tx"].get_vsize(), 1000) @@ -199,7 +198,7 @@ def test_truc_reorg(self): self.check_mempool([]) tx_v2_from_v3 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block["new_utxo"], version=2) tx_v3_from_v2 = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v2_block["new_utxo"], version=3) - tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_weight=5000, version=3) + tx_v3_child_large = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=tx_v3_block2["new_utxo"], target_vsize=1250, version=3) assert_greater_than(node.getmempoolentry(tx_v3_child_large["txid"])["vsize"], 1000) self.check_mempool([tx_v2_from_v3["txid"], tx_v3_from_v2["txid"], tx_v3_child_large["txid"]]) node.invalidateblock(block[0]) @@ -217,16 +216,16 @@ def test_nondefault_package_limits(self): """ node = self.nodes[0] self.log.info("Test that a decreased limitdescendantsize also applies to TRUC child") - parent_target_weight = 9990 * WITNESS_SCALE_FACTOR - child_target_weight = 500 * WITNESS_SCALE_FACTOR + parent_target_vsize = 9990 + child_target_vsize = 500 tx_v3_parent_large1 = self.wallet.send_self_transfer( from_node=node, - target_weight=parent_target_weight, + target_vsize=parent_target_vsize, version=3 ) tx_v3_child_large1 = self.wallet.create_self_transfer( utxo_to_spend=tx_v3_parent_large1["new_utxo"], - target_weight=child_target_weight, + target_vsize=child_target_vsize, version=3 ) @@ -244,12 +243,12 @@ def test_nondefault_package_limits(self): self.restart_node(0, extra_args=["-limitancestorsize=10", "-datacarriersize=40000"]) tx_v3_parent_large2 = self.wallet.send_self_transfer( from_node=node, - target_weight=parent_target_weight, + target_vsize=parent_target_vsize, version=3 ) tx_v3_child_large2 = self.wallet.create_self_transfer( utxo_to_spend=tx_v3_parent_large2["new_utxo"], - target_weight=child_target_weight, + target_vsize=child_target_vsize, version=3 ) @@ -267,12 +266,12 @@ def test_truc_ancestors_package(self): node = self.nodes[0] tx_v3_parent_normal = self.wallet.create_self_transfer( fee_rate=0, - target_weight=4004, + target_vsize=1001, version=3 ) tx_v3_parent_2_normal = self.wallet.create_self_transfer( fee_rate=0, - target_weight=4004, + target_vsize=1001, version=3 ) tx_v3_child_multiparent = self.wallet.create_self_transfer_multi( @@ -282,7 +281,7 @@ def test_truc_ancestors_package(self): ) tx_v3_child_heavy = self.wallet.create_self_transfer_multi( utxos_to_spend=[tx_v3_parent_normal["new_utxo"]], - target_weight=4004, + target_vsize=1001, fee_per_output=10000, version=3 ) @@ -294,7 +293,7 @@ def test_truc_ancestors_package(self): self.check_mempool([]) result = node.submitpackage([tx_v3_parent_normal["hex"], tx_v3_child_heavy["hex"]]) - # tx_v3_child_heavy is heavy based on weight, not sigops. + # tx_v3_child_heavy is heavy based on vsize, not sigops. assert_equal(result['package_msg'], f"TRUC-violation, version=3 child tx {tx_v3_child_heavy['txid']} (wtxid={tx_v3_child_heavy['wtxid']}) is too big: {tx_v3_child_heavy['tx'].get_vsize()} > 1000 virtual bytes") self.check_mempool([]) @@ -416,7 +415,7 @@ def test_truc_package_inheritance(self): node = self.nodes[0] tx_v3_parent = self.wallet.create_self_transfer( fee_rate=0, - target_weight=4004, + target_vsize=1001, version=3 ) tx_v2_child = self.wallet.create_self_transfer_multi( diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py index f3713f297e6235..9f50f24674481e 100644 --- a/test/functional/test_framework/wallet.py +++ b/test/functional/test_framework/wallet.py @@ -119,20 +119,18 @@ def __init__(self, test_node, *, mode=MiniWalletMode.ADDRESS_OP_TRUE, tag_name=N def _create_utxo(self, *, txid, vout, value, height, coinbase, confirmations): return {"txid": txid, "vout": vout, "value": value, "height": height, "coinbase": coinbase, "confirmations": confirmations} - def _bulk_tx(self, tx, target_weight): - """Pad a transaction with extra outputs until it reaches a target weight (or higher). + def _bulk_tx(self, tx, target_vsize): + """Pad a transaction with extra outputs until it reaches a target vsize. returns the tx """ tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN]))) - # determine number of needed padding bytes by converting weight difference to vbytes - dummy_vbytes = (target_weight - tx.get_weight() + 3) // 4 + # determine number of needed padding bytes + dummy_vbytes = target_vsize - tx.get_vsize() # compensate for the increase of the compact-size encoded script length # (note that the length encoding of the unpadded output script needs one byte) dummy_vbytes -= len(ser_compact_size(dummy_vbytes)) - 1 tx.vout[-1].scriptPubKey = CScript([OP_RETURN] + [OP_1] * dummy_vbytes) - # Actual weight should be at most 3 higher than target weight - assert_greater_than_or_equal(tx.get_weight(), target_weight) - assert_greater_than_or_equal(target_weight + 3, tx.get_weight()) + assert_equal(tx.get_vsize(), target_vsize) def get_balance(self): return sum(u['value'] for u in self._utxos) @@ -309,7 +307,7 @@ def create_self_transfer_multi( locktime=0, sequence=0, fee_per_output=1000, - target_weight=0, + target_vsize=0, confirmed_only=False, ): """ @@ -338,8 +336,8 @@ def create_self_transfer_multi( self.sign_tx(tx) - if target_weight: - self._bulk_tx(tx, target_weight) + if target_vsize: + self._bulk_tx(tx, target_vsize) txid = tx.rehash() return { @@ -364,7 +362,7 @@ def create_self_transfer( fee_rate=Decimal("0.003"), fee=Decimal("0"), utxo_to_spend=None, - target_weight=0, + target_vsize=0, confirmed_only=False, **kwargs, ): @@ -379,20 +377,18 @@ def create_self_transfer( vsize = Decimal(168) # P2PK (73 bytes scriptSig + 35 bytes scriptPubKey + 60 bytes other) else: assert False - if target_weight and not fee: # respect fee_rate if target weight is passed - # the actual weight might be off by 3 WUs, so calculate based on that (see self._bulk_tx) - max_actual_weight = target_weight + 3 - fee = get_fee(math.ceil(max_actual_weight / WITNESS_SCALE_FACTOR), fee_rate) + if target_vsize and not fee: # respect fee_rate if target vsize is passed + fee = get_fee(target_vsize, fee_rate) send_value = utxo_to_spend["value"] - (fee or (fee_rate * vsize / 1000)) # create tx tx = self.create_self_transfer_multi( utxos_to_spend=[utxo_to_spend], amount_per_output=int(COIN * send_value), - target_weight=target_weight, + target_vsize=target_vsize, **kwargs, ) - if not target_weight: + if not target_vsize: assert_equal(tx["tx"].get_vsize(), vsize) tx["new_utxo"] = tx.pop("new_utxos")[0]