Skip to content

Commit

Permalink
test: switch MiniWallet padding unit from weight to vsize
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
theStack committed Aug 26, 2024
1 parent 6d54633 commit 9547b88
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 70 deletions.
2 changes: 1 addition & 1 deletion test/functional/feature_blocksxor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down
16 changes: 7 additions & 9 deletions test/functional/feature_framework_miniwallet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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."""
Expand Down
31 changes: 17 additions & 14 deletions test/functional/mempool_limit.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,28 +55,28 @@ 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
)

# 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
Expand All @@ -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.
Expand All @@ -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"])
Expand Down Expand Up @@ -311,17 +313,18 @@ 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
child_feerate = (worst_feerate_btcvb * 1000) - Decimal("0.000001") # 0.1 sats/vbyte below worst feerate
# 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()
Expand Down
17 changes: 6 additions & 11 deletions test/functional/mempool_package_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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"]]
Expand All @@ -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"])
Expand Down
35 changes: 17 additions & 18 deletions test/functional/mempool_truc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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"])
Expand All @@ -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)
Expand All @@ -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())
Expand All @@ -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)
Expand Down Expand Up @@ -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])
Expand All @@ -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
)

Expand All @@ -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
)

Expand All @@ -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(
Expand All @@ -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
)
Expand All @@ -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([])

Expand Down Expand Up @@ -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(
Expand Down
Loading

0 comments on commit 9547b88

Please sign in to comment.