Merge bitcoin/bitcoin#30162: test: MiniWallet: respect passed feerate for padded txs (using `target_weight`)

39d135e79f3f0c40dfd8fad2c53723d533cd19b4 test: MiniWallet: respect fee_rate for target_weight, use in mempool_limit.py (Sebastian Falbesoner)
b2f0a9f8b0776d49ef1639310311ca50435a2a0a test: add framework functional test for MiniWallet's tx padding (Sebastian Falbesoner)
c17550bc3a68faa1eb82d1bdb767b41d8cd85a6b test: MiniWallet: fix tx padding (`target_weight`) for large sizes, improve accuracy (Sebastian Falbesoner)

Pull request description:

  MiniWallet allows to create padded transactions that are equal or slightly above a certain `target_weight` (first introduced in PR #25379, commit 1d6b438ef0ccd05e1522ac38b44f847c1d93e72f), which can be useful especially for mempool-related tests, e.g. for policy limit checks or scenarios to trigger mempool eviction. Currently the `target_weight` parameter doesn't play together with `fee_rate` though, as the fee calculation is incorrectly based on the tx vsize before the padding output is added, so the fee-rate is consequently far off. This means users are forced to pass an absolute fee, which can be quite inconvenient and leads to lots of duplicated "calculate absolute fee from fee-rate and vsize" code with the pattern `fee = (feerate / 1000) * (weight // 4)` on the call-sites.

  This PR first improves the tx padding itself to be more accurate, adds a functional test for it, and fixes the `fee_rate` treatment for the `{create,send}_self_transfer` methods. (Next step would be to enable this also for the `_self_transfer_multi` methods, but those currently don't even offer a `fee_rate` parameter). Finally, the ability to pass both `target_weight` and `fee_rate` is used in the `mempool_limit.py` functional test. There might be more use-cases in other tests, that could be done in a follow-up.

ACKs for top commit:
  rkrux:
    tACK [39d135e](https://github.com/bitcoin/bitcoin/pull/30162/commits/39d135e79f3f0c40dfd8fad2c53723d533cd19b4)
  ismaelsadeeq:
    Code Review ACK 39d135e79f3f0c40dfd8fad2c53723d533cd19b4  🚀
  glozow:
    light review ACK 39d135e79f3f0c40dfd8fad2c53723d533cd19b4

Tree-SHA512: 6bf8e853a921576d463291d619cdfd6a7e74cf92f61933a563800ac0b3c023a06569b581243166906f56b3c5e8858fec2d8a6910d55899e904221f847eb0953d

4 files changed, 81 insertions, 17 deletions

diff --git a/test/functional/feature_framework_miniwallet.py b/test/functional/feature_framework_miniwallet.py
new file mode 100755
index 0000000000..f108289018
--- /dev/null
+++ b/test/functional/feature_framework_miniwallet.py
@@ -0,0 +1,49 @@
+#!/usr/bin/env python3
+# Copyright (c) 2024 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test MiniWallet."""
+from test_framework.blocktools import COINBASE_MATURITY
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_greater_than_or_equal,
+)
+from test_framework.wallet import (
+    MiniWallet,
+    MiniWalletMode,
+)
+
+
+class FeatureFrameworkMiniWalletTest(BitcoinTestFramework):
+    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."""
+        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())
+
+    def run_test(self):
+        node = self.nodes[0]
+        self.wallets = [
+            ("ADDRESS_OP_TRUE", MiniWallet(node, mode=MiniWalletMode.ADDRESS_OP_TRUE)),
+            ("RAW_OP_TRUE",     MiniWallet(node, mode=MiniWalletMode.RAW_OP_TRUE)),
+            ("RAW_P2PK",        MiniWallet(node, mode=MiniWalletMode.RAW_P2PK)),
+        ]
+        for _, wallet in self.wallets:
+            self.generate(wallet, 10)
+        self.generate(wallet, COINBASE_MATURITY)
+
+        self.test_tx_padding()
+
+
+if __name__ == '__main__':
+    FeatureFrameworkMiniWalletTest().main()
diff --git a/test/functional/mempool_limit.py b/test/functional/mempool_limit.py
index d46924f4ce..49a0a32c45 100755
--- a/test/functional/mempool_limit.py
+++ b/test/functional/mempool_limit.py
@@ -59,7 +59,7 @@ class MempoolLimitTest(BitcoinTestFramework):
         mempoolmin_feerate = node.getmempoolinfo()["mempoolminfee"]
         tx_A = self.wallet.send_self_transfer(
             from_node=node,
-            fee=(mempoolmin_feerate / 1000) * (A_weight // 4) + Decimal('0.000001'),
+            fee_rate=mempoolmin_feerate,
             target_weight=A_weight,
             utxo_to_spend=rbf_utxo,
             confirmed_only=True
@@ -77,7 +77,7 @@ class MempoolLimitTest(BitcoinTestFramework):
         non_cpfp_carveout_weight = 40001 # EXTRA_DESCENDANT_TX_SIZE_LIMIT + 1
         tx_C = self.wallet.create_self_transfer(
             target_weight=non_cpfp_carveout_weight,
-            fee = (mempoolmin_feerate / 1000) * (non_cpfp_carveout_weight // 4) + Decimal('0.000001'),
+            fee_rate=mempoolmin_feerate,
             utxo_to_spend=tx_B["new_utxo"],
             confirmed_only=True
         )
@@ -109,7 +109,7 @@ class MempoolLimitTest(BitcoinTestFramework):
         # 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=(mempoolmin_feerate / 1000) * (evicted_weight // 4) + Decimal('0.000001'),
+            fee_rate=mempoolmin_feerate,
             target_weight=evicted_weight,
             confirmed_only=True
         )
@@ -135,11 +135,11 @@ class MempoolLimitTest(BitcoinTestFramework):
         parent_weight = 100000
         num_big_parents = 3
         assert_greater_than(parent_weight * num_big_parents, current_info["maxmempool"] - current_info["bytes"])
-        parent_fee = (100 * mempoolmin_feerate / 1000) * (parent_weight // 4)
+        parent_feerate = 100 * mempoolmin_feerate
 
         big_parent_txids = []
         for i in range(num_big_parents):
-            parent = self.wallet.create_self_transfer(fee=parent_fee, target_weight=parent_weight, confirmed_only=True)
+            parent = self.wallet.create_self_transfer(fee_rate=parent_feerate, target_weight=parent_weight, confirmed_only=True)
             parent_utxos.append(parent["new_utxo"])
             package_hex.append(parent["hex"])
             big_parent_txids.append(parent["txid"])
@@ -314,18 +314,20 @@ class MempoolLimitTest(BitcoinTestFramework):
         target_weight_each = 200000
         assert_greater_than(target_weight_each * 2, node.getmempoolinfo()["maxmempool"] - node.getmempoolinfo()["bytes"])
         # Should be a true CPFP: parent's feerate is just below mempool min feerate
-        parent_fee = (mempoolmin_feerate / 1000) * (target_weight_each // 4) - Decimal("0.00001")
+        parent_feerate = mempoolmin_feerate - Decimal("0.000001")  # 0.1 sats/vbyte below min feerate
         # Parent + child is above mempool minimum feerate
-        child_fee = (worst_feerate_btcvb) * (target_weight_each // 4) - Decimal("0.00001")
+        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=parent_fee, target_weight=target_weight_each)
-        tx_child_just_above = miniwallet.create_self_transfer(utxo_to_spend=tx_parent_just_below["new_utxo"], fee=child_fee, target_weight=target_weight_each)
+        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)
         # This package ranks below the lowest descendant package in the mempool
-        assert_greater_than(worst_feerate_btcvb, (parent_fee + child_fee) / (tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize()))
-        assert_greater_than(mempoolmin_feerate, (parent_fee) / (tx_parent_just_below["tx"].get_vsize()))
-        assert_greater_than((parent_fee + child_fee) / (tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize()), mempoolmin_feerate / 1000)
+        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()
+        assert_greater_than(worst_feerate_btcvb, package_fee / package_vsize)
+        assert_greater_than(mempoolmin_feerate, tx_parent_just_below["fee"] / (tx_parent_just_below["tx"].get_vsize()))
+        assert_greater_than(package_fee / package_vsize, mempoolmin_feerate / 1000)
         res = node.submitpackage([tx_parent_just_below["hex"], tx_child_just_above["hex"]])
         for wtxid in [tx_parent_just_below["wtxid"], tx_child_just_above["wtxid"]]:
             assert_equal(res["tx-results"][wtxid]["error"], "mempool full")
diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py
index 4433cbcc55..7d4f4a3392 100644
--- a/test/functional/test_framework/wallet.py
+++ b/test/functional/test_framework/wallet.py
@@ -7,6 +7,7 @@
 from copy import deepcopy
 from decimal import Decimal
 from enum import Enum
+import math
 from typing import (
     Any,
     Optional,
@@ -33,10 +34,13 @@ from test_framework.messages import (
     CTxInWitness,
     CTxOut,
     hash256,
+    ser_compact_size,
+    WITNESS_SCALE_FACTOR,
 )
 from test_framework.script import (
     CScript,
     LEAF_VERSION_TAPSCRIPT,
+    OP_1,
     OP_NOP,
     OP_RETURN,
     OP_TRUE,
@@ -52,6 +56,7 @@ from test_framework.script_util import (
 from test_framework.util import (
     assert_equal,
     assert_greater_than_or_equal,
+    get_fee,
 )
 from test_framework.wallet_util import generate_keypair
 
@@ -119,13 +124,16 @@ class MiniWallet:
         """Pad a transaction with extra outputs until it reaches a target weight (or higher).
         returns the tx
         """
-        tx.vout.append(CTxOut(nValue=0, scriptPubKey=CScript([OP_RETURN, b'a'])))
+        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
-        tx.vout[-1].scriptPubKey = CScript([OP_RETURN, b'a' * dummy_vbytes])
-        # Lower bound should always be off by at most 3
+        # 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)
-        # Higher bound should always be off by at most 3 + 12 weight (for encoding the length)
-        assert_greater_than_or_equal(target_weight + 15, tx.get_weight())
+        assert_greater_than_or_equal(target_weight + 3, tx.get_weight())
 
     def get_balance(self):
         return sum(u['value'] for u in self._utxos)
@@ -367,6 +375,10 @@ class MiniWallet:
             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)
         send_value = utxo_to_spend["value"] - (fee or (fee_rate * vsize / 1000))
 
         # create tx
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 725b116281..84e524558f 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -361,6 +361,7 @@ BASE_SCRIPTS = [
     'feature_addrman.py',
     'feature_asmap.py',
     'feature_fastprune.py',
+    'feature_framework_miniwallet.py',
     'mempool_unbroadcast.py',
     'mempool_compatibility.py',
     'mempool_accept_wtxid.py',