Merge bitcoin/bitcoin#25445: test: Return new_utxo from create_self_transfer in MiniWallet

fa83c0c test: Remove unused call to generate in rpc_mempool_info (MacroFake)
fa13375 test: Sync MiniWallet utxo state after each generate call (MacroFake)
dddd7c4 test: Drop spent utxos in MiniWallet scan_tx (MacroFake)
fa04ff6 test: Return new_utxos from create_self_transfer_multi in MiniWallet (MacroFake)
fa34e44 test: Return new_utxo from create_self_transfer in MiniWallet (MacroFake)

Pull request description:

  I need this for some stuff, but it also makes sense on its own to:

  * unify the flow with a private `_create_utxo` helper
  * simplify the flow by giving the caller ownership of the utxo right away

ACKs for top commit:
  kouloumos:
    re-ACK fa83c0c 🚀

Tree-SHA512: 381f0e814864ba207363a56859a6c0519e4a86d0562927f16a610a5b706c9fc942c1b5e93388cda0fa0b3cacd58f55bc2ffcc60355858a580263e5bef33c2f4b

Author: MacroFake

test/functional/feature_dbcrash.py

@@ -192,14 +192,12 @@ def generate_small_transactions(self, node, count, utxo_list):
                 # Sanity check -- if we chose inputs that are too small, skip
                 continue
 
-            tx = self.wallet.create_self_transfer_multi(
+            self.wallet.send_self_transfer_multi(
                 from_node=node,
                 utxos_to_spend=utxos_to_spend,
                 num_outputs=3,
-                fee_per_output=FEE // 3)
-
-            # Send the transaction to get into the mempool (skip fee-checks to run faster)
-            node.sendrawtransaction(hexstring=tx.serialize().hex(), maxfeerate=0)
+                fee_per_output=FEE // 3,
+            )
             num_transactions += 1
 
     def run_test(self):

test/functional/feature_fee_estimation.py

@@ -52,7 +52,8 @@ def small_txpuzzle_randfee(
         raise RuntimeError(f"Insufficient funds: need {amount + fee}, have {total_in}")
     tx = wallet.create_self_transfer_multi(
         utxos_to_spend=utxos_to_spend,
-        fee_per_output=0)
+        fee_per_output=0,
+    )["tx"]
     tx.vout[0].nValue = int((total_in - amount - fee) * COIN)
     tx.vout.append(deepcopy(tx.vout[0]))
     tx.vout[1].nValue = int(amount * COIN)

test/functional/feature_rbf.py

@@ -472,11 +472,10 @@ def test_too_many_replacements_with_default_mempool_params(self):
 
             # Now attempt to submit a tx that double-spends all the root tx inputs, which
             # would invalidate `num_txs_invalidated` transactions.
-            double_tx = wallet.create_self_transfer_multi(
+            tx_hex = wallet.create_self_transfer_multi(
                 utxos_to_spend=root_utxos,
                 fee_per_output=10_000_000,  # absurdly high feerate
-            )
-            tx_hex = double_tx.serialize().hex()
+            )["hex"]
 
             if failure_expected:
                 assert_raises_rpc_error(

test/functional/mempool_reorg.py

@@ -68,10 +68,8 @@ def run_test(self):
         assert_raises_rpc_error(-26, 'non-final', self.nodes[0].sendrawtransaction, timelock_tx)
 
         self.log.info("Create spend_2_1 and spend_3_1")
-        spend_2_utxo = wallet.get_utxo(txid=spend_2['txid'])
-        spend_2_1 = wallet.create_self_transfer(utxo_to_spend=spend_2_utxo)
-        spend_3_utxo = wallet.get_utxo(txid=spend_3['txid'])
-        spend_3_1 = wallet.create_self_transfer(utxo_to_spend=spend_3_utxo)
+        spend_2_1 = wallet.create_self_transfer(utxo_to_spend=spend_2["new_utxo"])
+        spend_3_1 = wallet.create_self_transfer(utxo_to_spend=spend_3["new_utxo"])
 
         self.log.info("Broadcast and mine spend_3_1")
         spend_3_1_id = self.nodes[0].sendrawtransaction(spend_3_1['hex'])

test/functional/rpc_mempool_info.py

@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test RPCs that retrieve information from the mempool."""
 
-from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -19,7 +18,6 @@ def set_test_params(self):
 
     def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
-        self.generate(self.wallet, COINBASE_MATURITY + 1)
         self.wallet.rescan_utxos()
         confirmed_utxo = self.wallet.get_utxo()
 

test/functional/test_framework/wallet.py

@@ -101,6 +101,9 @@ def __init__(self, test_node, *, mode=MiniWalletMode.ADDRESS_OP_TRUE):
             self._address, self._internal_key = create_deterministic_address_bcrt1_p2tr_op_true()
             self._scriptPubKey = bytes.fromhex(self._test_node.validateaddress(self._address)['scriptPubKey'])
 
+    def _create_utxo(self, *, txid, vout, value, height):
+        return {"txid": txid, "vout": vout, "value": value, "height": height}
+
     def get_balance(self):
         return sum(u['value'] for u in self._utxos)
 
@@ -110,13 +113,22 @@ def rescan_utxos(self):
         res = self._test_node.scantxoutset(action="start", scanobjects=[self.get_descriptor()])
         assert_equal(True, res['success'])
         for utxo in res['unspents']:
-            self._utxos.append({'txid': utxo['txid'], 'vout': utxo['vout'], 'value': utxo['amount'], 'height': utxo['height']})
+            self._utxos.append(self._create_utxo(txid=utxo["txid"], vout=utxo["vout"], value=utxo["amount"], height=utxo["height"]))
 
     def scan_tx(self, tx):
-        """Scan the tx for self._scriptPubKey outputs and add them to self._utxos"""
+        """Scan the tx and adjust the internal list of owned utxos"""
+        for spent in tx["vin"]:
+            # Mark spent. This may happen when the caller has ownership of a
+            # utxo that remained in this wallet. For example, by passing
+            # mark_as_spent=False to get_utxo or by using an utxo returned by a
+            # create_self_transfer* call.
+            try:
+                self.get_utxo(txid=spent["txid"], vout=spent["vout"])
+            except StopIteration:
+                pass
         for out in tx['vout']:
             if out['scriptPubKey']['hex'] == self._scriptPubKey.hex():
-                self._utxos.append({'txid': tx['txid'], 'vout': out['n'], 'value': out['value'], 'height': 0})
+                self._utxos.append(self._create_utxo(txid=tx["txid"], vout=out["n"], value=out["value"], height=0))
 
     def sign_tx(self, tx, fixed_length=True):
         """Sign tx that has been created by MiniWallet in P2PK mode"""
@@ -135,12 +147,16 @@ def sign_tx(self, tx, fixed_length=True):
         tx.rehash()
 
     def generate(self, num_blocks, **kwargs):
-        """Generate blocks with coinbase outputs to the internal address, and append the outputs to the internal list"""
+        """Generate blocks with coinbase outputs to the internal address, and call rescan_utxos"""
         blocks = self._test_node.generatetodescriptor(num_blocks, self.get_descriptor(), **kwargs)
-        for b in blocks:
-            block_info = self._test_node.getblock(blockhash=b, verbosity=2)
-            cb_tx = block_info['tx'][0]
-            self._utxos.append({'txid': cb_tx['txid'], 'vout': 0, 'value': cb_tx['vout'][0]['value'], 'height': block_info['height']})
+        # Calling rescan_utxos here makes sure that after a generate the utxo
+        # set is in a clean state. For example, the wallet will update
+        # - if the caller consumed utxos, but never used them
+        # - if the caller sent a transaction that is not mined or got rbf'd
+        # - after block re-orgs
+        # - the utxo height for mined mempool txs
+        # - However, the wallet will not consider remaining mempool txs
+        self.rescan_utxos()
         return blocks
 
     def get_scriptPubKey(self):
@@ -206,20 +222,10 @@ def send_to(self, *, from_node, scriptPubKey, amount, fee=1000):
         return txid, 1
 
     def send_self_transfer_multi(self, *, from_node, **kwargs):
-        """
-        Create and send a transaction that spends the given UTXOs and creates a
-        certain number of outputs with equal amounts.
-
-        Returns a dictionary with
-            - txid
-            - serialized transaction in hex format
-            - transaction as CTransaction instance
-            - list of newly created UTXOs, ordered by vout index
-        """
+        """Call create_self_transfer_multi and send the transaction."""
         tx = self.create_self_transfer_multi(**kwargs)
-        txid = self.sendrawtransaction(from_node=from_node, tx_hex=tx.serialize().hex())
-        return {'new_utxos': [self.get_utxo(txid=txid, vout=vout) for vout in range(len(tx.vout))],
-                'txid': txid, 'hex': tx.serialize().hex(), 'tx': tx}
+        self.sendrawtransaction(from_node=from_node, tx_hex=tx["hex"])
+        return tx
 
     def create_self_transfer_multi(
         self,
@@ -253,7 +259,18 @@ def create_self_transfer_multi(
         outputs_value_total = inputs_value_total - fee_per_output * num_outputs
         for o in tx.vout:
             o.nValue = outputs_value_total // num_outputs
-        return tx
+        txid = tx.rehash()
+        return {
+            "new_utxos": [self._create_utxo(
+                txid=txid,
+                vout=i,
+                value=Decimal(tx.vout[i].nValue) / COIN,
+                height=0,
+            ) for i in range(len(tx.vout))],
+            "txid": txid,
+            "hex": tx.serialize().hex(),
+            "tx": tx,
+        }
 
     def create_self_transfer(self, *, fee_rate=Decimal("0.003"), utxo_to_spend=None, locktime=0, sequence=0):
         """Create and return a tx with the specified fee_rate. Fee may be exact or at most one satoshi higher than needed."""
@@ -264,12 +281,12 @@ def create_self_transfer(self, *, fee_rate=Decimal("0.003"), utxo_to_spend=None,
             vsize = Decimal(168)  # P2PK (73 bytes scriptSig + 35 bytes scriptPubKey + 60 bytes other)
         else:
             assert False
-        send_value = int(COIN * (utxo_to_spend['value'] - fee_rate * (vsize / 1000)))
+        send_value = utxo_to_spend["value"] - (fee_rate * vsize / 1000)
         assert send_value > 0
 
         tx = CTransaction()
         tx.vin = [CTxIn(COutPoint(int(utxo_to_spend['txid'], 16), utxo_to_spend['vout']), nSequence=sequence)]
-        tx.vout = [CTxOut(send_value, self._scriptPubKey)]
+        tx.vout = [CTxOut(int(COIN * send_value), self._scriptPubKey)]
         tx.nLockTime = locktime
         if self._mode == MiniWalletMode.RAW_P2PK:
             self.sign_tx(tx)
@@ -283,8 +300,9 @@ def create_self_transfer(self, *, fee_rate=Decimal("0.003"), utxo_to_spend=None,
         tx_hex = tx.serialize().hex()
 
         assert_equal(tx.get_vsize(), vsize)
+        new_utxo = self._create_utxo(txid=tx.rehash(), vout=0, value=send_value, height=0)
 
-        return {'txid': tx.rehash(), 'wtxid': tx.getwtxid(), 'hex': tx_hex, 'tx': tx}
+        return {"txid": new_utxo["txid"], "wtxid": tx.getwtxid(), "hex": tx_hex, "tx": tx, "new_utxo": new_utxo}
 
     def sendrawtransaction(self, *, from_node, tx_hex, maxfeerate=0, **kwargs):
         txid = from_node.sendrawtransaction(hexstring=tx_hex, maxfeerate=maxfeerate, **kwargs)