From 5b216dbc5b04ae88254f91120796797f123f15f9 Mon Sep 17 00:00:00 2001
From: glozow <gloriajzhao@gmail.com>
Date: Tue, 17 Jan 2023 13:52:44 +0000
Subject: [PATCH] [test] package rbf

---
 src/test/txpackage_tests.cpp           | 135 +++++++++++++++++
 src/validation.cpp                     |   1 +
 test/functional/mempool_package_rbf.py | 197 +++++++++++++++++++++++++
 test/functional/test_runner.py         |   1 +
 4 files changed, 334 insertions(+)
 create mode 100755 test/functional/mempool_package_rbf.py

diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp
index 637f92bd0fcdf7..9f9ab7434b3b02 100644
--- a/src/test/txpackage_tests.cpp
+++ b/src/test/txpackage_tests.cpp
@@ -6,6 +6,7 @@
 #include <key_io.h>
 #include <policy/packages.h>
 #include <policy/policy.h>
+#include <policy/rbf.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
 #include <test/util/random.h>
@@ -854,4 +855,138 @@ BOOST_FIXTURE_TEST_CASE(package_cpfp_tests, TestChain100Setup)
         BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
     }
 }
+
+BOOST_FIXTURE_TEST_CASE(package_rbf_tests, TestChain100Setup)
+{
+    mineBlocks(5);
+    LOCK(::cs_main);
+    size_t expected_pool_size = m_node.mempool->size();
+    CKey child_key;
+    child_key.MakeNewKey(true);
+    CScript parent_spk = GetScriptForDestination(WitnessV0KeyHash(child_key.GetPubKey()));
+    CKey grandchild_key;
+    grandchild_key.MakeNewKey(true);
+    CScript child_spk = GetScriptForDestination(WitnessV0KeyHash(grandchild_key.GetPubKey()));
+
+    const CAmount coinbase_value{50 * COIN};
+    // Test that de-duplication works. This is not actually package rbf.
+    {
+        // 1 parent paying 200sat, 1 child paying 300sat
+        Package package1;
+        // 1 parent paying 200sat, 1 child paying 500sat
+        Package package2;
+        // Package1 and package2 have the same parent. The children conflict.
+        auto mtx_parent = CreateValidMempoolTransaction(/*input_transaction=*/m_coinbase_txns[0], /*input_vout=*/0,
+                                                        /*input_height=*/0, /*input_signing_key=*/coinbaseKey,
+                                                        /*output_destination=*/parent_spk,
+                                                        /*output_amount=*/coinbase_value - 200, /*submit=*/false, /*version=*/2);
+        CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
+        package1.push_back(tx_parent);
+        package2.push_back(tx_parent);
+
+        CTransactionRef tx_child_1 = MakeTransactionRef(CreateValidMempoolTransaction(tx_parent, 0, 101, child_key, child_spk, coinbase_value - 500, false, 2));
+        package1.push_back(tx_child_1);
+        CTransactionRef tx_child_2 = MakeTransactionRef(CreateValidMempoolTransaction(tx_parent, 0, 101, child_key, child_spk, coinbase_value - 700, false, 2));
+        package2.push_back(tx_child_2);
+
+        LOCK(m_node.mempool->cs);
+        const auto submit1 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package1, /*test_accept=*/false);
+        BOOST_CHECK_MESSAGE(submit1.m_state.IsValid(), "Package validation unexpectedly failed: " << submit1.m_state.GetRejectReason());
+        auto it_parent_1 = submit1.m_tx_results.find(tx_parent->GetWitnessHash());
+        auto it_child_1 = submit1.m_tx_results.find(tx_child_1->GetWitnessHash());
+        BOOST_CHECK(it_parent_1 != submit1.m_tx_results.end());
+        BOOST_CHECK(it_child_1 != submit1.m_tx_results.end());
+        BOOST_CHECK_EQUAL(it_parent_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK_EQUAL(it_child_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        expected_pool_size += 2;
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent->GetHash())));
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child_1->GetHash())));
+
+        const auto submit2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package2, /*test_accept=*/false);
+        auto it_parent_2 = submit2.m_tx_results.find(tx_parent->GetWitnessHash());
+        auto it_child_2 = submit2.m_tx_results.find(tx_child_2->GetWitnessHash());
+        BOOST_CHECK(it_parent_2 != submit2.m_tx_results.end());
+        BOOST_CHECK(it_child_2 != submit2.m_tx_results.end());
+        BOOST_CHECK_MESSAGE(submit2.m_state.IsValid(), "Package validation unexpectedly failed" << submit2.m_state.GetRejectReason());
+        BOOST_CHECK_EQUAL(it_parent_2->second.m_result_type, MempoolAcceptResult::ResultType::MEMPOOL_ENTRY);
+        BOOST_CHECK_EQUAL(it_child_2->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child_2->GetHash())));
+        BOOST_CHECK(!m_node.mempool->exists(GenTxid::Txid(tx_child_1->GetHash())));
+    }
+
+    // Test package rbf.
+    {
+        CTransactionRef tx_parent_1 = MakeTransactionRef(CreateValidMempoolTransaction(
+            m_coinbase_txns[1], /*input_vout=*/0, /*input_height=*/0,
+            coinbaseKey, parent_spk, coinbase_value - 200, /*submit=*/false, /*version=*/2));
+        CTransactionRef tx_child_1 = MakeTransactionRef(CreateValidMempoolTransaction(
+            tx_parent_1, /*input_vout=*/0, /*input_height=*/101,
+            child_key, child_spk, coinbase_value - 400, /*submit=*/false, /*version=*/2));
+
+        CTransactionRef tx_parent_2 = MakeTransactionRef(CreateValidMempoolTransaction(
+            m_coinbase_txns[1], /*input_vout=*/0, /*input_height=*/0,
+            coinbaseKey, parent_spk, coinbase_value - 800, /*submit=*/false, /*version=*/2));
+        CTransactionRef tx_child_2 = MakeTransactionRef(CreateValidMempoolTransaction(
+            tx_parent_2, /*input_vout=*/0, /*input_height=*/101,
+            child_key, child_spk, coinbase_value - 800 - 200, /*submit=*/false, /*version=*/2));
+
+        CTransactionRef tx_parent_3 = MakeTransactionRef(CreateValidMempoolTransaction(
+            m_coinbase_txns[1], /*input_vout=*/0, /*input_height=*/0,
+            coinbaseKey, parent_spk, coinbase_value - 200, /*submit=*/false, /*version=*/2));
+        CTransactionRef tx_child_3 = MakeTransactionRef(CreateValidMempoolTransaction(
+            tx_parent_3, /*input_vout=*/0, /*input_height=*/101,
+            child_key, child_spk, coinbase_value - 200 - 1300, /*submit=*/false, /*version=*/2));
+
+        // 1 parent paying 200sat, 1 child paying 200sat. Both v2.
+        Package package1{tx_parent_1, tx_child_1};
+        // 1 parent paying 800sat, 1 child paying 200sat. Both v2.
+        Package package2{tx_parent_2, tx_child_2};
+        // 1 parent paying 200sat, 1 child paying 1300. Both v2.
+        Package package3{tx_parent_3, tx_child_3};
+        // In all packages, the parents conflict with each other
+
+        const auto submit1 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package1, false);
+        BOOST_CHECK_MESSAGE(submit1.m_state.IsValid(), "Package validation unexpectedly failed" << submit1.m_state.GetRejectReason());
+        auto it_parent_1 = submit1.m_tx_results.find(tx_parent_1->GetWitnessHash());
+        auto it_child_1 = submit1.m_tx_results.find(tx_child_1->GetWitnessHash());
+        BOOST_CHECK(it_parent_1 != submit1.m_tx_results.end());
+        BOOST_CHECK(it_child_1 != submit1.m_tx_results.end());
+        BOOST_CHECK_EQUAL(it_parent_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK_EQUAL(it_child_1->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        expected_pool_size += 2;
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_1->GetHash())));
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child_1->GetHash())));
+
+        // This replacement is actually not package rbf; the parent carries enough fees
+        // to replace the entire package on its own.
+        const auto submit2 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package2, false);
+        BOOST_CHECK_MESSAGE(submit2.m_state.IsValid(), "Package validation unexpectedly failed" << submit2.m_state.GetRejectReason());
+        auto it_parent_2 = submit2.m_tx_results.find(tx_parent_2->GetWitnessHash());
+        auto it_child_2 = submit2.m_tx_results.find(tx_child_2->GetWitnessHash());
+        BOOST_CHECK(it_parent_2 != submit2.m_tx_results.end());
+        BOOST_CHECK(it_child_2 != submit2.m_tx_results.end());
+        BOOST_CHECK_EQUAL(it_parent_2->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK_EQUAL(it_child_2->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_2->GetHash())));
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child_2->GetHash())));
+
+        // Package RBF, in which the replacement transaction's child sponsors the fees to meet RBF feerate rules
+        const auto submit3 = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package3, false);
+        BOOST_CHECK_MESSAGE(submit3.m_state.IsValid(), "Package validation unexpectedly failed" << submit3.m_state.GetRejectReason());
+        auto it_parent_3 = submit3.m_tx_results.find(tx_parent_3->GetWitnessHash());
+        auto it_child_3 = submit3.m_tx_results.find(tx_child_3->GetWitnessHash());
+        BOOST_CHECK(it_parent_3 != submit3.m_tx_results.end());
+        BOOST_CHECK(it_child_3 != submit3.m_tx_results.end());
+        BOOST_CHECK_EQUAL(it_parent_3->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK_EQUAL(it_child_3->second.m_result_type, MempoolAcceptResult::ResultType::VALID);
+        BOOST_CHECK_EQUAL(m_node.mempool->size(), expected_pool_size);
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_parent_3->GetHash())));
+        BOOST_CHECK(m_node.mempool->exists(GenTxid::Txid(tx_child_3->GetHash())));
+    }
+
+}
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/validation.cpp b/src/validation.cpp
index fc2ff465fa375b..146140eb639c0e 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1111,6 +1111,7 @@ bool MemPoolAccept::PackageMempoolChecks(const ATMPArgs& args,
         }
     }
 
+    // TODO check if feerate of child is higher than feerate of parent
     // Check if it's economically rational to mine this package rather than the ones it replaces.
     if (const auto err_string{CheckMinerScores(m_total_modified_fees, m_total_vsize, direct_conflict_iters,
                                                m_all_conflicts)}) {
diff --git a/test/functional/mempool_package_rbf.py b/test/functional/mempool_package_rbf.py
new file mode 100755
index 00000000000000..f13100ca8940f3
--- /dev/null
+++ b/test/functional/mempool_package_rbf.py
@@ -0,0 +1,197 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+from decimal import Decimal
+
+from test_framework.messages import (
+    COIN,
+    MAX_BIP125_RBF_SEQUENCE,
+)
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import (
+    assert_greater_than_or_equal,
+    assert_raises_rpc_error,
+    assert_equal,
+)
+from test_framework.wallet import (
+    DEFAULT_FEE,
+    MiniWallet,
+)
+
+class PackageRBFTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.setup_clean_chain = True
+
+    def assert_mempool_contents(self, expected=None, unexpected=None):
+        """Assert that all transactions in expected are in the mempool,
+        and all transactions in unexpected are not in the mempool.
+        """
+        if not expected:
+            expected = []
+        if not unexpected:
+            unexpected = []
+        assert set(unexpected).isdisjoint(expected)
+        mempool = self.nodes[0].getrawmempool(verbose=False)
+        for tx in expected:
+            assert tx.rehash() in mempool
+        for tx in unexpected:
+            assert tx.rehash() not in mempool
+
+    def create_simple_package(self, parent_coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_FEE, heavy_child=False):
+        """Create a 1 parent 1 child package using the coin passed in as the parent's input. The
+        parent has 1 output, used to fund 1 child transaction.
+        All transactions signal BIP125 replaceability, but nSequence changes based on self.ctr. This
+        prevents identical txids between packages when the parents spend the same coin and have the
+        same fee (i.e. 0sat).
+
+        returns tuple (hex serialized txns, CTransaction objects)
+        """
+        self.ctr += 1
+        # Use fee_rate=0 because create_self_transfer will use the default fee_rate value otherwise.
+        # Passing in fee>0 overrides fee_rate, so this still works for non-zero parent_fee.
+        parent_result = self.wallet.create_self_transfer(
+            fee_rate=0,
+            fee=parent_fee,
+            utxo_to_spend=parent_coin,
+            sequence=MAX_BIP125_RBF_SEQUENCE - self.ctr,
+        )
+
+        num_child_outputs = 10 if heavy_child else 1
+        child_result = self.wallet.create_self_transfer_multi(
+            utxos_to_spend=[parent_result["new_utxo"]],
+            num_outputs=num_child_outputs,
+            fee_per_output=int(child_fee * COIN // num_child_outputs),
+            sequence=MAX_BIP125_RBF_SEQUENCE - self.ctr,
+        )
+        package_hex = [parent_result["hex"], child_result["hex"]]
+        package_txns = [parent_result["tx"], child_result["tx"]]
+        return package_hex, package_txns
+
+    def run_test(self):
+        # Counter used to count the number of times we constructed packages. Since we're constructing parent transactions with the same
+        # coins (to create conflicts), and giving them the same fee (i.e. 0, since their respective children are paying), we might
+        # accidentally just create the exact same transaction again. To prevent this, set nSequences to MAX_BIP125_RBF_SEQUENCE - self.ctr.
+        self.ctr = 0
+
+        self.log.info("Generate blocks to create UTXOs")
+        node = self.nodes[0]
+        self.wallet = MiniWallet(node)
+        self.generate(self.wallet, 160)
+        self.coins = self.wallet.get_utxos(mark_as_spent=False)
+        # Mature coinbase transactions
+        self.generate(self.wallet, 100)
+        self.address = self.wallet.get_address()
+
+        self.test_package_rbf_basic()
+        self.test_package_rbf_additional_fees()
+        self.test_package_rbf_max_conflicts()
+        self.test_package_rbf_conflicting_conflicts()
+        self.test_package_rbf_partial()
+
+    def test_package_rbf_basic(self):
+        self.log.info("Test that a child can pay to replace its parents' conflicts")
+        node = self.nodes[0]
+        # Reuse the same coins so that the transactions conflict with one another.
+        parent_coin = self.coins.pop()
+        package_hex1, package_txns1 = self.create_simple_package(parent_coin, DEFAULT_FEE, DEFAULT_FEE)
+        package_hex2, package_txns2 = self.create_simple_package(parent_coin, DEFAULT_FEE, DEFAULT_FEE * 5)
+        node.submitpackage(package_hex1)
+        self.assert_mempool_contents(expected=package_txns1, unexpected=package_txns2)
+
+        submitres = node.submitpackage(package_hex2)
+        submitres["replaced-transactions"] == [tx.rehash() for tx in package_txns1]
+        self.assert_mempool_contents(expected=package_txns2, unexpected=package_txns1)
+        self.generate(node, 1)
+
+    def test_package_rbf_additional_fees(self):
+        self.log.info("Check Package RBF must increase the absolute fee")
+        node = self.nodes[0]
+        coin = self.coins.pop()
+        package_hex1, package_txns1 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_FEE, heavy_child=True)
+        assert_greater_than_or_equal(1000, package_txns1[-1].get_vsize())
+        node.submitpackage(package_hex1)
+        self.assert_mempool_contents(expected=package_txns1)
+        # Package 2 has a higher feerate but lower absolute fee
+        package_hex2, package_txns2 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_FEE - Decimal("0.000000001"))
+        pkg_results2 = node.submitpackage(package_hex2)
+        assert_equal(pkg_results2["package_msg"], 'package RBF failed: insufficient fees')
+        self.assert_mempool_contents(expected=package_txns1, unexpected=package_txns2)
+        # Package 3 has a higher feerate and absolute fee
+        package_hex3, package_txns3 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_FEE * 3)
+        node.submitpackage(package_hex3)
+        self.assert_mempool_contents(expected=package_txns3, unexpected=package_txns1 + package_txns2)
+        self.generate(node, 1)
+
+        self.log.info("Check Package RBF must pay for the entire package's bandwidth")
+        coin = self.coins.pop()
+        package_hex4, package_txns4 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_FEE)
+        node.submitpackage(package_hex4)
+        self.assert_mempool_contents(expected=package_txns4, unexpected=[])
+        package_hex5, package_txns5 = self.create_simple_package(coin, parent_fee=DEFAULT_FEE, child_fee=DEFAULT_FEE + Decimal("0.000000001"))
+        pkg_results5 = node.submitpackage(package_hex5)
+        assert_equal(pkg_results5["package_msg"], 'package RBF failed: insufficient fees')
+
+        self.assert_mempool_contents(expected=package_txns4, unexpected=package_txns5)
+        self.generate(node, 1)
+
+    def test_package_rbf_max_conflicts(self):
+        node = self.nodes[0]
+        self.log.info("Check Package RBF cannot replace more than 100 transactions")
+        num_coins = 5
+        parent_coins = self.coins[:num_coins]
+        del self.coins[:num_coins]
+        # Original transactions: 5 transactions with 24 descendants each.
+        for coin in parent_coins:
+            self.wallet.send_self_transfer_chain(from_node=node, chain_length=25, utxo_to_spend=coin)
+
+        # Replacement package: 1 parent which conflicts with 5 * (1 + 24) = 125 mempool transactions.
+        package_parent = self.wallet.create_self_transfer_multi(utxos_to_spend=parent_coins)
+        package_child = self.wallet.create_self_transfer(fee_rate=50*DEFAULT_FEE, utxo_to_spend=package_parent["new_utxos"][0])
+
+        pkg_results = node.submitpackage([package_parent["hex"], package_child["hex"]])
+        assert_equal(pkg_results["package_msg"], "package RBF failed: too many potential replacements")
+        self.generate(node, 1)
+
+    def test_package_rbf_conflicting_conflicts(self):
+        node = self.nodes[0]
+        self.log.info("Check that different package transactions cannot share the same conflicts")
+        coin = self.coins.pop()
+        package_hex1, package_txns1 = self.create_simple_package(coin, DEFAULT_FEE, DEFAULT_FEE)
+        package_hex2, package_txns2 = self.create_simple_package(coin, Decimal("0.00009"), DEFAULT_FEE * 2)
+        package_hex3, package_txns3 = self.create_simple_package(coin, DEFAULT_FEE, DEFAULT_FEE * 5)
+        node.submitpackage(package_hex1)
+        self.assert_mempool_contents(expected=package_txns1)
+        # The first two transactions have the same conflicts
+        package_duplicate_conflicts_hex = [package_hex2[0]] + package_hex3
+        # Note that this won't actually go into the RBF logic, because static package checks will
+        # detect that two package transactions conflict with each other. Either way, this must fail.
+        assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, package_duplicate_conflicts_hex)
+        self.assert_mempool_contents(expected=package_txns1, unexpected=package_txns2 + package_txns3)
+        # The RBFs should otherwise work.
+        submitres2 = node.submitpackage(package_hex2)
+        assert_equal(sorted(submitres2["replaced-transactions"]),sorted( [tx.rehash() for tx in package_txns1]))
+        self.assert_mempool_contents(expected=package_txns2, unexpected=package_txns1)
+        submitres3 = node.submitpackage(package_hex3)
+        assert_equal(sorted(submitres3["replaced-transactions"]), sorted([tx.rehash() for tx in package_txns2]))
+        self.assert_mempool_contents(expected=package_txns3, unexpected=package_txns2)
+
+    def test_package_rbf_partial(self):
+        self.log.info("Test that package RBF works when a transaction was already submitted")
+        node = self.nodes[0]
+        coin = self.coins.pop()
+        package_hex1, package_txns1 = self.create_simple_package(coin, DEFAULT_FEE, DEFAULT_FEE)
+        package_hex2, package_txns2 = self.create_simple_package(coin, DEFAULT_FEE * 3, DEFAULT_FEE * 3)
+        node.submitpackage(package_hex1)
+        self.assert_mempool_contents(expected=package_txns1, unexpected=package_txns2)
+        # Submit parent on its own. It should have no trouble replacing the previous
+        # transaction(s) because the fee is tripled.
+        node.sendrawtransaction(package_hex2[0])
+        node.submitpackage(package_hex2)
+        self.assert_mempool_contents(expected=package_txns2, unexpected=package_txns1)
+        self.generate(node, 1)
+
+if __name__ == "__main__":
+    PackageRBFTest().main()
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
index 6016a482f8d1f3..542cc002d0ed07 100755
--- a/test/functional/test_runner.py
+++ b/test/functional/test_runner.py
@@ -275,6 +275,7 @@
     'mempool_packages.py',
     'mempool_package_onemore.py',
     'mempool_package_limits.py',
+    'mempool_package_rbf.py',
     'feature_versionbits_warning.py',
     'rpc_preciousblock.py',
     'wallet_importprunedfunds.py --legacy-wallet',