Skip to content

Commit d9007f5

Browse files
committed
Merge bitcoin#28762: MiniMiner changes for package linearization
d9cc99d [test] MiniMiner::Linearize and manual construction (glozow) dfd6a37 [refactor] unify fee amounts in miniminer_tests (glozow) f4b1b24 [MiniMiner] track inclusion order and add Linearize() function (glozow) 0040759 [test] add case for MiniMiner working with negative fee txns (glozow) fe6332c [MiniMiner] make target_feerate optional (glozow) 5a83f55 [MiniMiner] allow manual construction with non-mempool txns (glozow) e3b2e63 [refactor] change MiniMinerMempoolEntry ctor to take values, update includes (glozow) 4aa98b7 [lint] update expected boost includes (glozow) Pull request description: This is part of bitcoin#27463. It splits off the `MiniMiner`-specific changes from bitcoin#26711 for ease of review, as suggested in bitcoin#26711 (comment). - Allow using `MiniMiner` on transactions that aren't in the mempool. - Make `target_feerate` param of `BuildMockTemplate` optional, meaning "don't stop building the template until all the transactions have been selected." - Add clarification for how this is different from `target_feerate=0` (bitcoin#26711 (comment)) - Track the order in which transactions are included in the template to get the "linearization order" of the transactions. - Tests Reviewers can take a look at bitcoin#26711 to see how these functions are used to linearize the `AncestorPackage` there. ACKs for top commit: TheCharlatan: ACK d9cc99d kevkevinpal: reACK [d9cc99d](bitcoin@d9cc99d) achow101: re-ACK d9cc99d Tree-SHA512: 32b80064b6679536ac573d674825c5ca0cd6245e49c2fd5eaf260dc535335a57683c74ddd7ce1f249b5b12b2683de4362a7b0f1fc0814c3b3b9f14c682665583
2 parents 0fd7ca4 + d9cc99d commit d9007f5

File tree

4 files changed

+345
-29
lines changed

4 files changed

+345
-29
lines changed

src/node/mini_miner.cpp

+77-4
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,14 @@
44

55
#include <node/mini_miner.h>
66

7+
#include <boost/multi_index/detail/hash_index_iterator.hpp>
8+
#include <boost/operators.hpp>
79
#include <consensus/amount.h>
810
#include <policy/feerate.h>
911
#include <primitives/transaction.h>
12+
#include <sync.h>
13+
#include <txmempool.h>
14+
#include <uint256.h>
1015
#include <util/check.h>
1116

1217
#include <algorithm>
@@ -72,7 +77,12 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
7277
// Add every entry to m_entries_by_txid and m_entries, except the ones that will be replaced.
7378
for (const auto& txiter : cluster) {
7479
if (!m_to_be_replaced.count(txiter->GetTx().GetHash())) {
75-
auto [mapiter, success] = m_entries_by_txid.emplace(txiter->GetTx().GetHash(), MiniMinerMempoolEntry(txiter));
80+
auto [mapiter, success] = m_entries_by_txid.emplace(txiter->GetTx().GetHash(),
81+
MiniMinerMempoolEntry{/*fee_self=*/txiter->GetModifiedFee(),
82+
/*fee_ancestor=*/txiter->GetModFeesWithAncestors(),
83+
/*vsize_self=*/txiter->GetTxSize(),
84+
/*vsize_ancestor=*/txiter->GetSizeWithAncestors(),
85+
/*tx_in=*/txiter->GetSharedTx()});
7686
m_entries.push_back(mapiter);
7787
} else {
7888
auto outpoints_it = m_requested_outpoints_by_txid.find(txiter->GetTx().GetHash());
@@ -122,6 +132,48 @@ MiniMiner::MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& ou
122132
SanityCheck();
123133
}
124134

135+
MiniMiner::MiniMiner(const std::vector<MiniMinerMempoolEntry>& manual_entries,
136+
const std::map<Txid, std::set<Txid>>& descendant_caches)
137+
{
138+
for (const auto& entry : manual_entries) {
139+
const auto& txid = entry.GetTx().GetHash();
140+
// We need to know the descendant set of every transaction.
141+
if (!Assume(descendant_caches.count(txid) > 0)) {
142+
m_ready_to_calculate = false;
143+
return;
144+
}
145+
// Just forward these args onto MiniMinerMempoolEntry
146+
auto [mapiter, success] = m_entries_by_txid.emplace(txid, entry);
147+
// Txids must be unique; this txid shouldn't already be an entry in m_entries_by_txid
148+
if (Assume(success)) m_entries.push_back(mapiter);
149+
}
150+
// Descendant cache is already built, but we need to translate them to m_entries_by_txid iters.
151+
for (const auto& [txid, desc_txids] : descendant_caches) {
152+
// Descendant cache should include at least the tx itself.
153+
if (!Assume(!desc_txids.empty())) {
154+
m_ready_to_calculate = false;
155+
return;
156+
}
157+
std::vector<MockEntryMap::iterator> cached_descendants;
158+
for (const auto& desc_txid : desc_txids) {
159+
auto desc_it{m_entries_by_txid.find(desc_txid)};
160+
// Descendants should only include transactions with corresponding entries.
161+
if (!Assume(desc_it != m_entries_by_txid.end())) {
162+
m_ready_to_calculate = false;
163+
return;
164+
} else {
165+
cached_descendants.emplace_back(desc_it);
166+
}
167+
}
168+
m_descendant_set_by_txid.emplace(txid, cached_descendants);
169+
}
170+
Assume(m_to_be_replaced.empty());
171+
Assume(m_requested_outpoints_by_txid.empty());
172+
Assume(m_bump_fees.empty());
173+
Assume(m_inclusion_order.empty());
174+
SanityCheck();
175+
}
176+
125177
// Compare by min(ancestor feerate, individual feerate), then iterator
126178
//
127179
// Under the ancestor-based mining approach, high-feerate children can pay for parents, but high-feerate
@@ -201,8 +253,10 @@ void MiniMiner::SanityCheck() const
201253
[&](const auto& txid){return m_entries_by_txid.find(txid) == m_entries_by_txid.end();}));
202254
}
203255

204-
void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate)
256+
void MiniMiner::BuildMockTemplate(std::optional<CFeeRate> target_feerate)
205257
{
258+
const auto num_txns{m_entries_by_txid.size()};
259+
uint32_t sequence_num{0};
206260
while (!m_entries_by_txid.empty()) {
207261
// Sort again, since transaction removal may change some m_entries' ancestor feerates.
208262
std::sort(m_entries.begin(), m_entries.end(), AncestorFeerateComparator());
@@ -213,7 +267,8 @@ void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate)
213267
const auto ancestor_package_size = (*best_iter)->second.GetSizeWithAncestors();
214268
const auto ancestor_package_fee = (*best_iter)->second.GetModFeesWithAncestors();
215269
// Stop here. Everything that didn't "make it into the block" has bumpfee.
216-
if (ancestor_package_fee < target_feerate.GetFee(ancestor_package_size)) {
270+
if (target_feerate.has_value() &&
271+
ancestor_package_fee < target_feerate->GetFee(ancestor_package_size)) {
217272
break;
218273
}
219274

@@ -237,14 +292,32 @@ void MiniMiner::BuildMockTemplate(const CFeeRate& target_feerate)
237292
to_process.erase(iter);
238293
}
239294
}
295+
// Track the order in which transactions were selected.
296+
for (const auto& ancestor : ancestors) {
297+
m_inclusion_order.emplace(Txid::FromUint256(ancestor->first), sequence_num);
298+
}
240299
DeleteAncestorPackage(ancestors);
241300
SanityCheck();
301+
++sequence_num;
302+
}
303+
if (!target_feerate.has_value()) {
304+
Assume(m_in_block.size() == num_txns);
305+
} else {
306+
Assume(m_in_block.empty() || m_total_fees >= target_feerate->GetFee(m_total_vsize));
242307
}
243-
Assume(m_in_block.empty() || m_total_fees >= target_feerate.GetFee(m_total_vsize));
308+
Assume(m_in_block.empty() || sequence_num > 0);
309+
Assume(m_in_block.size() == m_inclusion_order.size());
244310
// Do not try to continue building the block template with a different feerate.
245311
m_ready_to_calculate = false;
246312
}
247313

314+
315+
std::map<Txid, uint32_t> MiniMiner::Linearize()
316+
{
317+
BuildMockTemplate(std::nullopt);
318+
return m_inclusion_order;
319+
}
320+
248321
std::map<COutPoint, CAmount> MiniMiner::CalculateBumpFees(const CFeeRate& target_feerate)
249322
{
250323
if (!m_ready_to_calculate) return {};

src/node/mini_miner.h

+60-14
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,45 @@
55
#ifndef BITCOIN_NODE_MINI_MINER_H
66
#define BITCOIN_NODE_MINI_MINER_H
77

8-
#include <txmempool.h>
8+
#include <consensus/amount.h>
9+
#include <primitives/transaction.h>
10+
#include <uint256.h>
911

12+
#include <map>
1013
#include <memory>
1114
#include <optional>
15+
#include <set>
1216
#include <stdint.h>
17+
#include <vector>
18+
19+
class CFeeRate;
20+
class CTxMemPool;
1321

1422
namespace node {
1523

1624
// Container for tracking updates to ancestor feerate as we include ancestors in the "block"
1725
class MiniMinerMempoolEntry
1826
{
19-
const CAmount fee_individual;
2027
const CTransactionRef tx;
2128
const int64_t vsize_individual;
22-
CAmount fee_with_ancestors;
2329
int64_t vsize_with_ancestors;
30+
const CAmount fee_individual;
31+
CAmount fee_with_ancestors;
2432

2533
// This class must be constructed while holding mempool.cs. After construction, the object's
2634
// methods can be called without holding that lock.
2735

2836
public:
29-
explicit MiniMinerMempoolEntry(CTxMemPool::txiter entry) :
30-
fee_individual{entry->GetModifiedFee()},
31-
tx{entry->GetSharedTx()},
32-
vsize_individual(entry->GetTxSize()),
33-
fee_with_ancestors{entry->GetModFeesWithAncestors()},
34-
vsize_with_ancestors(entry->GetSizeWithAncestors())
37+
explicit MiniMinerMempoolEntry(CAmount fee_self,
38+
CAmount fee_ancestor,
39+
int64_t vsize_self,
40+
int64_t vsize_ancestor,
41+
const CTransactionRef& tx_in):
42+
tx{tx_in},
43+
vsize_individual{vsize_self},
44+
vsize_with_ancestors{vsize_ancestor},
45+
fee_individual{fee_self},
46+
fee_with_ancestors{fee_ancestor}
3547
{ }
3648

3749
CAmount GetModifiedFee() const { return fee_individual; }
@@ -55,8 +67,14 @@ struct IteratorComparator
5567
}
5668
};
5769

58-
/** A minimal version of BlockAssembler. Allows us to run the mining algorithm on a subset of
59-
* mempool transactions, ignoring consensus rules, to calculate mining scores. */
70+
/** A minimal version of BlockAssembler, using the same ancestor set scoring algorithm. Allows us to
71+
* run this algorithm on a limited set of transactions (e.g. subset of mempool or transactions that
72+
* are not yet in mempool) instead of the entire mempool, ignoring consensus rules.
73+
* Callers may use this to:
74+
* - Calculate the "bump fee" needed to spend an unconfirmed UTXO at a given feerate
75+
* - "Linearize" a list of transactions to see the order in which they would be selected for
76+
* inclusion in a block
77+
*/
6078
class MiniMiner
6179
{
6280
// When true, a caller may use CalculateBumpFees(). Becomes false if we failed to retrieve
@@ -72,7 +90,11 @@ class MiniMiner
7290
// the same tx will have the same bumpfee. Excludes non-mempool transactions.
7391
std::map<uint256, std::vector<COutPoint>> m_requested_outpoints_by_txid;
7492

75-
// What we're trying to calculate.
93+
// Txid to a number representing the order in which this transaction was included (smaller
94+
// number = included earlier). Transactions included in an ancestor set together have the same
95+
// sequence number.
96+
std::map<Txid, uint32_t> m_inclusion_order;
97+
// What we're trying to calculate. Outpoint to the fee needed to bring the transaction to the target feerate.
7698
std::map<COutPoint, CAmount> m_bump_fees;
7799

78100
// The constructed block template
@@ -102,14 +124,32 @@ class MiniMiner
102124
/** Returns true if CalculateBumpFees may be called, false if not. */
103125
bool IsReadyToCalculate() const { return m_ready_to_calculate; }
104126

105-
/** Build a block template until the target feerate is hit. */
106-
void BuildMockTemplate(const CFeeRate& target_feerate);
127+
/** Build a block template until the target feerate is hit. If target_feerate is not given,
128+
* builds a block template until all transactions have been selected. */
129+
void BuildMockTemplate(std::optional<CFeeRate> target_feerate);
107130

108131
/** Returns set of txids in the block template if one has been constructed. */
109132
std::set<uint256> GetMockTemplateTxids() const { return m_in_block; }
110133

134+
/** Constructor that takes a list of outpoints that may or may not belong to transactions in the
135+
* mempool. Copies out information about the relevant transactions in the mempool into
136+
* MiniMinerMempoolEntrys.
137+
*/
111138
MiniMiner(const CTxMemPool& mempool, const std::vector<COutPoint>& outpoints);
112139

140+
/** Constructor in which the MiniMinerMempoolEntry entries have been constructed manually,
141+
* presumably because these transactions are not in the mempool (yet). It is assumed that all
142+
* entries are unique and their values are correct, otherwise results computed by MiniMiner may
143+
* be incorrect. Callers should check IsReadyToCalculate() after construction.
144+
* @param[in] descendant_caches A map from each transaction to the set of txids of this
145+
* transaction's descendant set, including itself. Each tx in
146+
* manual_entries must have a corresponding entry in this map, and
147+
* all of the txids in a descendant set must correspond to a tx in
148+
* manual_entries.
149+
*/
150+
MiniMiner(const std::vector<MiniMinerMempoolEntry>& manual_entries,
151+
const std::map<Txid, std::set<Txid>>& descendant_caches);
152+
113153
/** Construct a new block template and, for each outpoint corresponding to a transaction that
114154
* did not make it into the block, calculate the cost of bumping those transactions (and their
115155
* ancestors) to the minimum feerate. Returns a map from outpoint to bump fee, or an empty map
@@ -120,6 +160,12 @@ class MiniMiner
120160
* not make it into the block to the target feerate. Returns the total bump fee, or std::nullopt
121161
* if it cannot be calculated. */
122162
std::optional<CAmount> CalculateTotalBumpFees(const CFeeRate& target_feerate);
163+
164+
/** Construct a new block template with all of the transactions and calculate the order in which
165+
* they are selected. Returns the sequence number (lower = selected earlier) with which each
166+
* transaction was selected, indexed by txid, or an empty map if it cannot be calculated.
167+
*/
168+
std::map<Txid, uint32_t> Linearize();
123169
};
124170
} // namespace node
125171

0 commit comments

Comments
 (0)