Additional tests

[Zshan0]

Apart from the standard tests that have been written in #413, this PR contains more functional tests with other scenarios.

[Zshan0]

I believe that the problem is primarily here, I am unable to find a way around ensuring that res_vault_txid is not spent but the other unvaults are

[darosior]

So the issue you are having is with regard to getting a set of Spend and Unvault transactions, all unconfirmed? You could just use 4 different vaults, broadcast the unvault for all of them, and only confirm the Unvault + broadcast the Spend for 2 of them.

Here is a solution that de-prioritizes a Spend transaction so we can still use the helpers (which generate blocks and would otherwise confirm the first Spend!). It's not necessary but copying all the logic of the helpers would be less nice:

diff --git a/tests/test_rpc.py b/tests/test_rpc.py
index e6ec7e4..8dd3bd6 100644
--- a/tests/test_rpc.py
+++ b/tests/test_rpc.py
@@ -1645,77 +1645,50 @@ def test_manual_cpfp_lower_feerate(revault_network, bitcoind):
     man.wait_for_log("Nothing to CPFP in the given list.")
 
 
-## Mixed tests
 @pytest.mark.skipif(not POSTGRES_IS_SETUP, reason="Needs Postgres for servers db")
-def test_manual_cpfp_batch_spend_unvault(revault_network, bitcoind):
+def test_manual_cpfp_mixed_unvault_spend(revault_network, bitcoind):
+    """
+    Test to manually CPFP a set of unconfirmed Spend and Unvault transactions at once.
+    """
     CSV = 12
     revault_network.deploy(
         2,
         1,
         csv=CSV,
-        bitcoind_rpc_mocks={"estimatesmartfee": {"feerate": 0.0005}},  # 50 sats/vbyte
+        bitcoind_rpc_mocks={"estimatesmartfee": {"feerate": 0.001}},  # 100 sats/vbyte
     )
     man = revault_network.mans()[0]
-    vaults = revault_network.fundmany([1, 2, 3])
-    res_vault = vaults[2]
-    vaults  = vaults[:2]
+    vaults = revault_network.fundmany([1, 2, 3, 4])
 
-    # Activating all the vaults
+    # Activate all the vaults, prepare those we'll spend and those we'll just unvault.
+    # Note the prioritisetransaction trick to get 2, separate, unconfirmed, Spend txs.
     revault_network.activate_fresh_vaults(vaults)
-    
-    # Broadcast the unvaults and get their txids, not to broadcast the res.
-    spend_psbts = [
-        revault_network.broadcast_unvaults_anyhow([vault], priority=True)
-        for vault in vaults]
-    
-    revault_network.activate_fresh_vaults([res_vault])
-    revault_network.broadcast_unvaults_anyhow([res_vault], priority=True)
-    res_vault_txid = get_unvault_txids(man, [res_vault])[0]
-
-    unvault_txids = get_unvault_txids(man, vaults)
-    spend_txids = [spend_psbt.tx.hash for spend_psbt in spend_psbts]
-
-    for w in revault_network.participants():
-        wait_for(
-            lambda: len(w.rpc.listvaults(["unvaulting"])["vaults"]) == len(vaults + [res_vault]),
-        )
-
-    # The Transaction is present in the mempool.
-    entry = bitcoind.rpc.getmempoolentry(res_vault_txid)
-    # Confirming the unvaults
-    bitcoind.generate_block(1, wait_for_mempool=unvault_txids + [res_vault_txid])
-    for w in revault_network.participants():
-        wait_for(
-            lambda: len(w.rpc.listvaults(["unvaulted"])["vaults"]) == len(vaults + [res_vault]),
-        )
-
-    # The transaction goes away from the mempool.
-    entry = bitcoind.rpc.getmempoolentry(res_vault_txid)
-    bitcoind.generate_blocks_censor(1, [res_vault_txid])
-    man.wait_for_log(f"Succesfully broadcasted Spend tx")
-
-    for w in revault_network.participants():
-        wait_for(
-            lambda: len(w.rpc.listvaults(["spending"])["vaults"]) == len(vaults),
-        )
-
-    for spend_txid in spend_txids:
-        entry = bitcoind.rpc.getmempoolentry(spend_txid)
+    vaults_unvaulting, vaults_spending = vaults[:2], vaults[2:]
+    _, spend_psbt = revault_network.spend_vaults_anyhow_unconfirmed(
+        [vaults_spending[0]]
+    )
+    spend_txid = spend_psbt.tx.hash
+    bitcoind.rpc.prioritisetransaction(spend_txid, None, -1000)
+    revault_network.spend_vaults_anyhow_unconfirmed([vaults_spending[1]])
+    bitcoind.rpc.prioritisetransaction(spend_txid, None, 1000)
+    for v in vaults_unvaulting:
+        revault_network.broadcast_unvaults_anyhow([v])
+
+    # At this point, all 4 txs are in mempool with a feerate lower than the estimate
+    assert len(bitcoind.rpc.getrawmempool()) == 4
+    all_txids = bitcoind.rpc.getrawmempool()
+    assert len(all_txids) == 4  # 2 Unvaults, 2 Spend
+    for txid in all_txids:
+        entry = bitcoind.rpc.getmempoolentry(txid)
         assert entry["descendantcount"] == 1
         package_feerate = entry["fees"]["descendant"] * COIN / entry["descendantsize"]
-        assert package_feerate < 50
+        assert package_feerate < 100
 
-    # Manual CPFP trigger for spend_txid.
-    man.rpc.cpfp(spend_txids, 50)
-    # Not mentioning the ids since the order might not be same.
-    man.wait_for_log(
-        f"CPFPed transactions with ids",
-    )
-
-    wait_for(lambda: len(bitcoind.rpc.getrawmempool()) == 3)
-    for spend_txid in spend_txids:
-        entry = bitcoind.rpc.getmempoolentry(spend_txid)
+    # Manually CPFP them at a feerate higher than the estimate. They should all have
+    # at least the feerate we gave them.
+    man.rpc.cpfp(all_txids, 104)
+    for txid in all_txids:
+        entry = bitcoind.rpc.getmempoolentry(txid)
         assert entry["descendantcount"] == 2
         package_feerate = entry["fees"]["descendant"] * COIN / entry["descendantsize"]
-        assert package_feerate >= 50
-
+        assert package_feerate >= 104

[darosior]

We should test for error cases too, such as not having enough funds available. Currently the RPC call would not error but would only log...