[MiniMiner] make target_feerate optional

Add an option to keep building the template regardless of feerate. We
can't just use target_feerate=0 because it's possible for transactions
to have negative modified feerates.

No behavior change for users that pass in a target_feerate.

Author: glozow

src/node/mini_miner.cpp

@@ -173,7 +173,6 @@ MiniMiner::MiniMiner(const std::vector<MiniMinerMempoolEntry>& manual_entries,
     SanityCheck();
 }
 
-
 // Compare by min(ancestor feerate, individual feerate), then iterator
 //
 // Under the ancestor-based mining approach, high-feerate children can pay for parents, but high-feerate
@@ -253,8 +252,9 @@ void MiniMiner::SanityCheck() const
         [&](const auto& txid){return m_entries_by_txid.find(txid) == m_entries_by_txid.end();}));
 }
 
-void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate)
+void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
 {
+    const auto num_txns{m_entries_by_txid.size()};
     while (!m_entries_by_txid.empty()) {
         // Sort again, since transaction removal may change some m_entries' ancestor feerates.
         std::sort(m_entries.begin(), m_entries.end(), AncestorFeerateComparator());
@@ -265,7 +265,8 @@ void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate)
         const auto ancestor_package_size = (*best_iter)->second.GetSizeWithAncestors();
         const auto ancestor_package_fee = (*best_iter)->second.GetModFeesWithAncestors();
         // Stop here. Everything that didn't "make it into the block" has bumpfee.
-        if (ancestor_package_fee < target_feerate.GetFee(ancestor_package_size)) {
+        if (target_feerate.has_value() &&
+            ancestor_package_fee < target_feerate->GetFee(ancestor_package_size)) {
             break;
         }
 
@@ -292,7 +293,11 @@ void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate)
         DeleteAncestorPackage(ancestors);
         SanityCheck();
     }
-    Assume(m_in_block.empty() || m_total_fees >= target_feerate.GetFee(m_total_vsize));
+    if (!target_feerate.has_value()) {
+        Assume(m_in_block.size() == num_txns);
+    } else {
+        Assume(m_in_block.empty() || m_total_fees >= target_feerate->GetFee(m_total_vsize));
+    }
     // Do not try to continue building the block template with a different feerate.
     m_ready_to_calculate = false;
 }

src/node/mini_miner.h

@@ -84,7 +84,7 @@ class MiniMiner
     // 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.
+    // What we're trying to calculate. Outpoint to the fee needed to bring the transaction to the target feerate.
     std::map<COutPoint, CAmount> m_bump_fees;
 
     // The constructed block template
@@ -114,8 +114,9 @@ class MiniMiner
     /** 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);
+    /** Build a block template until the target feerate is hit. If target_feerate is not given,
+     * builds a block template until all transactions have been selected. */
+    void BuildMockTemplate(std::optional<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; }
@@ -149,6 +150,7 @@ class MiniMiner
      * 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