Merge bitcoin/bitcoin#27021: Implement Mini version of BlockAssembler to calculate mining scores

6b605b9 [fuzz] Add MiniMiner target + diff fuzz against BlockAssembler (glozow)
3f3f2d5 [unit test] GatherClusters and MiniMiner unit tests (glozow)
59afcc8 Implement Mini version of BlockAssembler to calculate mining scores (glozow)
56484f0 [mempool] find connected mempool entries with GatherClusters(…) (glozow)

Pull request description:

  Implement Mini version of BlockAssembler to calculate mining scores

  Run the mining algorithm on a subset of the mempool, only disturbing the
  mempool to copy out fee information for relevant entries. Intended to be
  used by wallet to calculate amounts needed for fee-bumping unconfirmed
  transactions.

  From comments of sipa and glozow below:

  > > In what way does the code added here differ from the real block assembly code?
  >
  >    * Only operates on the relevant transactions rather than full mempool
  >    * Has the ability to remove transactions that will be replaced so they don't impact their ancestors
  >    * Does not hold mempool lock outside of the constructor, makes copies of the entries it needs instead (though I'm not sure if this has an effect in practice)
  >    * Doesn't do the sanity checks like keeping weight within max block weight and `IsFinalTx()`
  >    * After the block template is built, additionally calculates fees to bump remaining ancestor packages to target feerate

ACKs for top commit:
  achow101:
    ACK 6b605b9
  Xekyo:
    > ACK [6b605b9](bitcoin/bitcoin@6b605b9) modulo `miniminer_overlap` test.
  furszy:
    ACK 6b605b9 modulo `miniminer_overlap` test.
  theStack:
    Code-review ACK 6b605b9

Tree-SHA512: f86a8b4ae0506858a7b15d90f417ebceea5038b395c05c825e3796123ad3b6cb8a98ebb948521316802a4c6d60ebd7041093356b1e2c2922a06b3b96b3b8acb6

Author: glozow

src/Makefile.am

@@ -217,6 +217,7 @@ BITCOIN_CORE_H = \
   node/mempool_args.h \
   node/mempool_persist_args.h \
   node/miner.h \
+  node/mini_miner.h \
   node/minisketchwrapper.h \
   node/psbt.h \
   node/transaction.h \
@@ -410,6 +411,7 @@ libbitcoin_node_a_SOURCES = \
   node/mempool_args.cpp \
   node/mempool_persist_args.cpp \
   node/miner.cpp \
+  node/mini_miner.cpp \
   node/minisketchwrapper.cpp \
   node/psbt.cpp \
   node/transaction.cpp \

src/Makefile.test.include

@@ -106,6 +106,7 @@ BITCOIN_TESTS =\
   test/merkle_tests.cpp \
   test/merkleblock_tests.cpp \
   test/miner_tests.cpp \
+  test/miniminer_tests.cpp \
   test/miniscript_tests.cpp \
   test/minisketch_tests.cpp \
   test/multisig_tests.cpp \
@@ -287,6 +288,7 @@ test_fuzz_fuzz_SOURCES = \
  test/fuzz/message.cpp \
  test/fuzz/miniscript.cpp \
  test/fuzz/minisketch.cpp \
+ test/fuzz/mini_miner.cpp \
  test/fuzz/muhash.cpp \
  test/fuzz/multiplication_overflow.cpp \
  test/fuzz/net.cpp \

src/node/mini_miner.cpp

@@ -0,0 +1,121 @@
+// Copyright (c) 2022 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_NODE_MINI_MINER_H
+#define BITCOIN_NODE_MINI_MINER_H
+
+#include <txmempool.h>
+
+#include <memory>
+#include <optional>
+#include <stdint.h>
+
+namespace node {
+
+// Container for tracking updates to ancestor feerate as we include ancestors in the "block"
+class MiniMinerMempoolEntry
+{
+    const CAmount fee_individual;
+    const CTransactionRef tx;
+    const int64_t vsize_individual;
+
+// This class must be constructed while holding mempool.cs. After construction, the object's
+// methods can be called without holding that lock.
+public:
+    CAmount fee_with_ancestors;
+    int64_t vsize_with_ancestors;
+    explicit MiniMinerMempoolEntry(CTxMemPool::txiter entry) :
+        fee_individual{entry->GetModifiedFee()},
+        tx{entry->GetSharedTx()},
+        vsize_individual(entry->GetTxSize()),
+        fee_with_ancestors{entry->GetModFeesWithAncestors()},
+        vsize_with_ancestors(entry->GetSizeWithAncestors())
+    { }
+
+    CAmount GetModifiedFee() const { return fee_individual; }
+    CAmount GetModFeesWithAncestors() const { return fee_with_ancestors; }
+    int64_t GetTxSize() const { return vsize_individual; }
+    int64_t GetSizeWithAncestors() const { return vsize_with_ancestors; }
+    const CTransaction& GetTx() const LIFETIMEBOUND { return *tx; }
+};
+
+// Comparator needed for std::set<MockEntryMap::iterator>
+struct IteratorComparator
+{
+    template<typename I>
+    bool operator()(const I& a, const I& b) const
+    {
+        return &(*a) < &(*b);
+    }
+};
+
+/** A minimal version of BlockAssembler. Allows us to run the mining algorithm on a subset of
+ * mempool transactions, ignoring consensus rules, to calculate mining scores. */
+class MiniMiner
+{
+    // When true, a caller may use CalculateBumpFees(). Becomes false if we failed to retrieve
+    // mempool entries (i.e. cluster size too large) or bump fees have already been calculated.
+    bool m_ready_to_calculate{true};
+
+    // Set once per lifetime, fill in during initialization.
+    // txids of to-be-replaced transactions
+    std::set<uint256> m_to_be_replaced;
+
+    // If multiple argument outpoints correspond to the same transaction, cache them together in
+    // a single entry indexed by txid. Then we can just work with txids since all outpoints from
+    // the same tx will have the same bumpfee. Excludes non-mempool transactions.
+    std::map<uint256, std::vector<COutPoint>> m_requested_outpoints_by_txid;
+
+    // What we're trying to calculate.
+    std::map<COutPoint, CAmount> m_bump_fees;
+
+    // The constructed block template
+    std::set<uint256> m_in_block;
+
+    // Information on the current status of the block
+    CAmount m_total_fees{0};
+    int32_t m_total_vsize{0};
+
+    /** Main data structure holding the entries, can be indexed by txid */
+    std::map<uint256, MiniMinerMempoolEntry> m_entries_by_txid;
+    using MockEntryMap = decltype(m_entries_by_txid);
+
+    /** Vector of entries, can be sorted by ancestor feerate. */
+    std::vector<MockEntryMap::iterator> m_entries;
+
+    /** Map of txid to its descendants. Should be inclusive. */
+    std::map<uint256, std::vector<MockEntryMap::iterator>> m_descendant_set_by_txid;
+
+    /** Consider this ancestor package "mined" so remove all these entries from our data structures. */
+    void DeleteAncestorPackage(const std::set<MockEntryMap::iterator, IteratorComparator>& ancestors);
+
+    /** Perform some checks. */
+    void SanityCheck() const;
+
+public:
+    /** Returns true if CalculateBumpFees may be called, false if not. */
+    bool IsReadyToCalculate() const { return m_ready_to_calculate; }
+
+    /** Build a block template until the target feerate is hit. */
+    void BuildMockTemplate(const CFeeRate& target_feerate);
+
+    /** Returns set of txids in the block template if one has been constructed. */
+    std::set<uint256> GetMockTemplateTxids() const { return m_in_block; }
+
+    MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& outpoints);
+
+    /** Construct a new block template and, for each outpoint corresponding to a transaction that
+     * did not make it into the block, calculate the cost of bumping those transactions (and their
+     * ancestors) to the minimum feerate. Returns a map from outpoint to bump fee, or an empty map
+     * if they cannot be calculated. */
+    std::map<COutPoint, CAmount> CalculateBumpFees(const CFeeRate& target_feerate);
+
+    /** Construct a new block template and, calculate the cost of bumping all transactions that did
+     * not make it into the block to the target feerate. Returns the total bump fee, or std::nullopt
+     * if it cannot be calculated. */
+    std::optional<CAmount> CalculateTotalBumpFees(const CFeeRate& target_feerate);
+};
+} // namespace node
+
+#endif // BITCOIN_NODE_MINI_MINER_H

src/node/mini_miner.h

@@ -0,0 +1,192 @@
+#include <test/fuzz/FuzzedDataProvider.h>
+#include <test/fuzz/fuzz.h>
+#include <test/fuzz/util.h>
+#include <test/fuzz/util/mempool.h>
+#include <test/util/script.h>
+#include <test/util/setup_common.h>
+#include <test/util/txmempool.h>
+#include <test/util/mining.h>
+
+#include <node/mini_miner.h>
+#include <node/miner.h>
+#include <primitives/transaction.h>
+#include <random.h>
+#include <txmempool.h>
+
+#include <deque>
+#include <vector>
+
+namespace {
+
+const TestingSetup* g_setup;
+std::deque<COutPoint> g_available_coins;
+void initialize_miner()
+{
+    static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
+    g_setup = testing_setup.get();
+    for (uint32_t i = 0; i < uint32_t{100}; ++i) {
+        g_available_coins.push_back(COutPoint{uint256::ZERO, i});
+    }
+}
+
+// Test that the MiniMiner can run with various outpoints and feerates.
+FUZZ_TARGET_INIT(mini_miner, initialize_miner)
+{
+    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
+    CTxMemPool pool{CTxMemPool::Options{}};
+    std::vector<COutPoint> outpoints;
+    std::deque<COutPoint> available_coins = g_available_coins;
+    LOCK2(::cs_main, pool.cs);
+    // Cluster size cannot exceed 500
+    LIMITED_WHILE(!available_coins.empty(), 500)
+    {
+        CMutableTransaction mtx = CMutableTransaction();
+        const size_t num_inputs = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, available_coins.size());
+        const size_t num_outputs = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 50);
+        for (size_t n{0}; n < num_inputs; ++n) {
+            auto prevout = available_coins.front();
+            mtx.vin.push_back(CTxIn(prevout, CScript()));
+            available_coins.pop_front();
+        }
+        for (uint32_t n{0}; n < num_outputs; ++n) {
+            mtx.vout.push_back(CTxOut(100, P2WSH_OP_TRUE));
+        }
+        CTransactionRef tx = MakeTransactionRef(mtx);
+        TestMemPoolEntryHelper entry;
+        const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)};
+        assert(MoneyRange(fee));
+        pool.addUnchecked(entry.Fee(fee).FromTx(tx));
+
+        // All outputs are available to spend
+        for (uint32_t n{0}; n < num_outputs; ++n) {
+            if (fuzzed_data_provider.ConsumeBool()) {
+                available_coins.push_back(COutPoint{tx->GetHash(), n});
+            }
+        }
+
+        if (fuzzed_data_provider.ConsumeBool() && !tx->vout.empty()) {
+            // Add outpoint from this tx (may or not be spent by a later tx)
+            outpoints.push_back(COutPoint{tx->GetHash(),
+                                          (uint32_t)fuzzed_data_provider.ConsumeIntegralInRange<size_t>(0, tx->vout.size())});
+        } else {
+            // Add some random outpoint (will be interpreted as confirmed or not yet submitted
+            // to mempool).
+            auto outpoint = ConsumeDeserializable<COutPoint>(fuzzed_data_provider);
+            if (outpoint.has_value() && std::find(outpoints.begin(), outpoints.end(), *outpoint) == outpoints.end()) {
+                outpoints.push_back(*outpoint);
+            }
+        }
+
+    }
+
+    const CFeeRate target_feerate{CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/1000)}};
+    std::optional<CAmount> total_bumpfee;
+    CAmount sum_fees = 0;
+    {
+        node::MiniMiner mini_miner{pool, outpoints};
+        assert(mini_miner.IsReadyToCalculate());
+        const auto bump_fees = mini_miner.CalculateBumpFees(target_feerate);
+        for (const auto& outpoint : outpoints) {
+            auto it = bump_fees.find(outpoint);
+            assert(it != bump_fees.end());
+            assert(it->second >= 0);
+            sum_fees += it->second;
+        }
+        assert(!mini_miner.IsReadyToCalculate());
+    }
+    {
+        node::MiniMiner mini_miner{pool, outpoints};
+        assert(mini_miner.IsReadyToCalculate());
+        total_bumpfee = mini_miner.CalculateTotalBumpFees(target_feerate);
+        assert(total_bumpfee.has_value());
+        assert(!mini_miner.IsReadyToCalculate());
+    }
+    // Overlapping ancestry across multiple outpoints can only reduce the total bump fee.
+    assert (sum_fees >= *total_bumpfee);
+}
+
+// Test that MiniMiner and BlockAssembler build the same block given the same transactions and constraints.
+FUZZ_TARGET_INIT(mini_miner_selection, initialize_miner)
+{
+    FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
+    CTxMemPool pool{CTxMemPool::Options{}};
+    // Make a copy to preserve determinism.
+    std::deque<COutPoint> available_coins = g_available_coins;
+    std::vector<CTransactionRef> transactions;
+
+    LOCK2(::cs_main, pool.cs);
+    LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100)
+    {
+        CMutableTransaction mtx = CMutableTransaction();
+        const size_t num_inputs = 2;
+        const size_t num_outputs = fuzzed_data_provider.ConsumeIntegralInRange<size_t>(2, 5);
+        for (size_t n{0}; n < num_inputs; ++n) {
+            auto prevout = available_coins.front();
+            mtx.vin.push_back(CTxIn(prevout, CScript()));
+            available_coins.pop_front();
+        }
+        for (uint32_t n{0}; n < num_outputs; ++n) {
+            mtx.vout.push_back(CTxOut(100, P2WSH_OP_TRUE));
+        }
+        CTransactionRef tx = MakeTransactionRef(mtx);
+
+        // First 2 outputs are available to spend. The rest are added to outpoints to calculate bumpfees.
+        // There is no overlap between spendable coins and outpoints passed to MiniMiner because the
+        // MiniMiner interprets spent coins as to-be-replaced and excludes them.
+        for (uint32_t n{0}; n < num_outputs - 1; ++n) {
+            if (fuzzed_data_provider.ConsumeBool()) {
+                available_coins.push_front(COutPoint{tx->GetHash(), n});
+            } else {
+                available_coins.push_back(COutPoint{tx->GetHash(), n});
+            }
+        }
+
+        // Stop if pool reaches DEFAULT_BLOCK_MAX_WEIGHT because BlockAssembler will stop when the
+        // block template reaches that, but the MiniMiner will keep going.
+        if (pool.GetTotalTxSize() + GetVirtualTransactionSize(*tx) >= DEFAULT_BLOCK_MAX_WEIGHT) break;
+        TestMemPoolEntryHelper entry;
+        const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)};
+        assert(MoneyRange(fee));
+        pool.addUnchecked(entry.Fee(fee).FromTx(tx));
+        transactions.push_back(tx);
+    }
+    std::vector<COutPoint> outpoints;
+    for (const auto& coin : g_available_coins) {
+        if (!pool.GetConflictTx(coin)) outpoints.push_back(coin);
+    }
+    for (const auto& tx : transactions) {
+        assert(pool.exists(GenTxid::Txid(tx->GetHash())));
+        for (uint32_t n{0}; n < tx->vout.size(); ++n) {
+            COutPoint coin{tx->GetHash(), n};
+            if (!pool.GetConflictTx(coin)) outpoints.push_back(coin);
+        }
+    }
+    const CFeeRate target_feerate{ConsumeMoney(fuzzed_data_provider, /*max=*/MAX_MONEY/100000)};
+
+    node::BlockAssembler::Options miner_options;
+    miner_options.blockMinFeeRate = target_feerate;
+    miner_options.nBlockMaxWeight = DEFAULT_BLOCK_MAX_WEIGHT;
+    miner_options.test_block_validity = false;
+
+    node::BlockAssembler miner{g_setup->m_node.chainman->ActiveChainstate(), &pool, miner_options};
+    node::MiniMiner mini_miner{pool, outpoints};
+    assert(mini_miner.IsReadyToCalculate());
+
+    CScript spk_placeholder = CScript() << OP_0;
+    // Use BlockAssembler as oracle. BlockAssembler and MiniMiner should select the same
+    // transactions, stopping once packages do not meet target_feerate.
+    const auto blocktemplate{miner.CreateNewBlock(spk_placeholder)};
+    mini_miner.BuildMockTemplate(target_feerate);
+    assert(!mini_miner.IsReadyToCalculate());
+    auto mock_template_txids = mini_miner.GetMockTemplateTxids();
+    // MiniMiner doesn't add a coinbase tx.
+    assert(mock_template_txids.count(blocktemplate->block.vtx[0]->GetHash()) == 0);
+    mock_template_txids.emplace(blocktemplate->block.vtx[0]->GetHash());
+    assert(mock_template_txids.size() <= blocktemplate->block.vtx.size());
+    assert(mock_template_txids.size() >= blocktemplate->block.vtx.size());
+    assert(mock_template_txids.size() == blocktemplate->block.vtx.size());
+    for (const auto& tx : blocktemplate->block.vtx) {
+        assert(mock_template_txids.count(tx->GetHash()));
+    }
+}
+} // namespace