test: Don't rely on incentive incompatible replacement in mempool_accept_v3.py #29986

        fee_to_beat = max(int(tx_v3_child_2["fee"] * COIN), int(tx_unrelated_replacee["fee"] * COIN))

would you consider this instead? seems the more direct thing we're trying to accomplish

makes sense to me. Use fee_per_output=fee_to_beat_child2*2 ?

I think that works, let me know if what I pushed matches what you're thinking!

Oops -- I miscalculated the ancestor feerates after rewriting it this way, and the new tx still has insufficient fee. Here's what everything looks like with the change here (that is now merged, doh):

tx_unrelated_replacee: fee 31200, vsize 104 parent tx: fee 2000, vsize 147 tx_v3_child_2: fee 10400, vsize 104

tx_v3_child_3: fee 62400, vsize 157

So the total fee of the new transaction is high enough, but the ancestor feerate is still too low: (62400+2000)/304 = 211, which is less than the 300 s/b we need to beat the first tx.

Oof my bad for not checking more closely. How's this? (Edited)

diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py
index 8285b82c19..ab43ca60c3 100755
--- a/test/functional/mempool_accept_v3.py
+++ b/test/functional/mempool_accept_v3.py
@@ -533,10 +533,28 @@ class MempoolAcceptV3(BitcoinTestFramework):
         tx_unrelated_replacee = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_unrelated_conflict)
         assert tx_unrelated_replacee["txid"] in node.getrawmempool()
 
-        fee_to_beat = max(int(tx_v3_child_2["fee"] * COIN), int(tx_unrelated_replacee["fee"]*COIN))
+        fee_to_beat_absolute = int(tx_v3_child_2["fee"] * COIN) + int(tx_unrelated_replacee["fee"]*COIN)
+
+        entry_unrelated_replacee = node.getmempoolentry(tx_unrelated_replacee["txid"])
+        entry_tx_v3_child_2 = node.getmempoolentry(tx_v3_child_2["txid"])
+        entry_tx_parent = node.getmempoolentry(tx_v3_parent["txid"])
+
+        # Need to have a higher feerate than both direct conflicts
+        feerate_to_beat = max(int(entry_unrelated_replacee["fees"]["modified"] / entry_unrelated_replacee["vsize"] * COIN), \
+            int(entry_tx_v3_child_2["fees"]["modified"] / entry_tx_v3_child_2["vsize"] * COIN))
+
+        # Simulate to get tx size
+        test_tx_v3_child_3 = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict], version=3
+        )
+
+        # Include the parent size because the ancestor feerate is what matters.
+        fee_to_beat_feerate = feerate_to_beat * (test_tx_v3_child_3["tx"].get_vsize() + entry_tx_parent["vsize"])
+        fee_v3_child_3 = max(fee_to_beat_feerate, fee_to_beat_absolute) + 5
 
         tx_v3_child_3 = self.wallet.create_self_transfer_multi(
-            utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict], fee_per_output=fee_to_beat*2, version=3
+            utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict],
+            fee_per_output=fee_v3_child_3, version=3
         )
         node.sendrawtransaction(tx_v3_child_3["hex"])
         self.check_mempool(txids_v2_100 + [tx_v3_parent["txid"], tx_v3_child_3["txid"]])

above:

• fee_to_beat_absolute is 41600 (above code uses max(tx_unrelated_replacee, tx_v3_child_2) but Rule 3 would require us to beat the sum of their fees haha)
• feerate_to_beat is 300, i.e. 31200 /104
• fee_to_beat_feerate is 300 * (147 + 154) = 90300
• fee_v3_child_3 is max(41600, 90300) + 5 = 90305
• this means the ancestor feerate is 90305 / (147 + 154) = 300.017

Inspired by your approach, I thought it might be helpful to just add support for setting a target feerate on a transaction in miniwallet, something like this?

diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py
index 8285b82c19..db6aa78167 100755
--- a/test/functional/mempool_accept_v3.py
+++ b/test/functional/mempool_accept_v3.py
@@ -533,13 +533,25 @@ class MempoolAcceptV3(BitcoinTestFramework):
         tx_unrelated_replacee = self.wallet.send_self_transfer(from_node=node, utxo_to_spend=utxo_unrelated_conflict)
         assert tx_unrelated_replacee["txid"] in node.getrawmempool()
 
-        fee_to_beat = max(int(tx_v3_child_2["fee"] * COIN), int(tx_unrelated_replacee["fee"]*COIN))
-
+        def get_ancestor_feerate(txid):
+            entry = node.getmempoolentry(txid)
+            return entry["fees"]["ancestor"]*COIN // entry["ancestorsize"]
+
+        # For RBF to succeed, we should have a greater total fee and a greater
+        # feerate than what we're replacing.
+        # Use 500 vbytes as an upper bound on how big the new transaction might be.
+        fee_to_beat = int(tx_v3_child_2["fee"]*COIN) + int(tx_unrelated_replacee["fee"]*COIN) + 500
+        feerate_to_beat = max(get_ancestor_feerate(tx_v3_child_2["txid"]),
+                    get_ancestor_feerate(tx_unrelated_replacee["txid"]))
+
+        # Since this transaction has a parent, double the target feerate so
+        # that we'll pay for the parent (assuming comparable sizes).
         tx_v3_child_3 = self.wallet.create_self_transfer_multi(
-            utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict], fee_per_output=fee_to_beat*2, version=3
+            utxos_to_spend=[tx_v3_parent["new_utxos"][0], utxo_unrelated_conflict], fee_per_output=fee_to_beat, version=3, target_feerate=2*feerate_to_beat
         )
         node.sendrawtransaction(tx_v3_child_3["hex"])
         self.check_mempool(txids_v2_100 + [tx_v3_parent["txid"], tx_v3_child_3["txid"]])
+        assert get_ancestor_feerate(tx_v3_child_3["txid"]) > feerate_to_beat [@cleanup](/bitcoin-bitcoin/contributor/cleanup/)(extra_args=["-acceptnonstdtxn=1"])
     def test_reorg_sibling_eviction_1p2c(self):
diff --git a/test/functional/test_framework/wallet.py b/test/functional/test_framework/wallet.py
index 470ed08ed4..1e31ad6139 100644
--- a/test/functional/test_framework/wallet.py
+++ b/test/functional/test_framework/wallet.py
@@ -292,6 +292,7 @@ class MiniWallet:
         fee_per_output=1000,
         target_weight=0,
         confirmed_only=False,
+        target_feerate=None
     ):
         """
         Create and return a transaction that spends the given UTXOs and creates a
@@ -322,6 +323,20 @@ class MiniWallet:
         if target_weight:
             self._bulk_tx(tx, target_weight)
 
+        # If a certain feerate is required, use the size we just calculated to
+        # adjust the outputs, so that we achieve the target feerate.
+        if target_feerate:
+            additional_fee = target_feerate * tx.get_vsize() - fee
+            if (additional_fee > 0):
+                reduce_amount = additional_fee // num_outputs
+                outputs_value_total = 0
+                for i in range(num_outputs):
+                    tx.vout[i].nValue -= int(reduce_amount)
+                    assert tx.vout[i].nValue > 0
+                    outputs_value_total += tx.vout[i].nValue
+                self.sign_tx(tx)
+                fee = Decimal(inputs_value_total - outputs_value_total) / COIN
+
         txid = tx.rehash()
         return {