Merge bitcoin/bitcoin#26152: Bump unconfirmed ancestor transactions to target feerate

f18f9ef4d3 Amend bumpfee for inputs with overlapping ancestry (Murch)
2e35e944da Bump unconfirmed parent txs to target feerate (Murch)
3e3e052411 coinselection: Move GetSelectionWaste into SelectionResult (Andrew Chow)
c57889da66 [node] interface to get bump fees (glozow)
c24851be94 Make MiniMinerMempoolEntry fields private (Murch)
ac6030e4d8 Remove unused imports (Murch)
d2f90c31ef Fix calculation of ancestor set feerates in test (Murch)
a1f7d986e0 Match tx names to index in miniminer overlap test (Murch)

Pull request description:

  Includes some commits to address follow-ups from #27021: https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156

  Reduces the effective value of unconfirmed UTXOs by the fees necessary to bump their ancestor transactions to the same feerate.

  While the individual UTXOs always account for their full ancestry before coin-selection, we can correct potential overestimates with a second pass where we establish the ancestry and bump fee for the whole input set collectively.

  Fixes #9645
  Fixes #9864
  Fixes #15553

ACKs for top commit:
  S3RK:
    ACK f18f9ef4d3
  ismaelsadeeq:
    ACK f18f9ef4d3
  achow101:
    ACK f18f9ef4d3
  brunoerg:
    crACK f18f9ef4d3
  t-bast:
    ACK f18f9ef4d3, I reviewed the latest changes and run e2e tests against eclair, everything looks good 👍

Tree-SHA512: b65180c4243b1f9d13c311ada7a1c9f2f055d530d6c533b78c2068b50b8c29ac1321e89e85675b15515760d4f1b653ebd9da77b37c7be52d9bc565a3538f0aa6
This commit is contained in:
Andrew Chow
2023-09-14 16:00:58 -04:00
14 changed files with 1081 additions and 312 deletions

View File

@@ -28,6 +28,7 @@
#include <node/coin.h>
#include <node/context.h>
#include <node/interface_ui.h>
#include <node/mini_miner.h>
#include <node/transaction.h>
#include <policy/feerate.h>
#include <policy/fees.h>
@@ -665,6 +666,26 @@ public:
if (!m_node.mempool) return;
m_node.mempool->GetTransactionAncestry(txid, ancestors, descendants, ancestorsize, ancestorfees);
}
std::map<COutPoint, CAmount> CalculateIndividualBumpFees(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override
{
if (!m_node.mempool) {
std::map<COutPoint, CAmount> bump_fees;
for (const auto& outpoint : outpoints) {
bump_fees.emplace(std::make_pair(outpoint, 0));
}
return bump_fees;
}
return MiniMiner(*m_node.mempool, outpoints).CalculateBumpFees(target_feerate);
}
std::optional<CAmount> CalculateCombinedBumpFee(const std::vector<COutPoint>& outpoints, const CFeeRate& target_feerate) override
{
if (!m_node.mempool) {
return 0;
}
return MiniMiner(*m_node.mempool, outpoints).CalculateTotalBumpFees(target_feerate);
}
void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) override
{
const CTxMemPool::Limits default_limits{};

View File

@@ -7,9 +7,7 @@
#include <consensus/amount.h>
#include <policy/feerate.h>
#include <primitives/transaction.h>
#include <timedata.h>
#include <util/check.h>
#include <util/moneystr.h>
#include <algorithm>
#include <numeric>
@@ -171,9 +169,8 @@ void MiniMiner::DeleteAncestorPackage(const std::set<MockEntryMap::iterator, Ite
for (auto& descendant : it->second) {
// If these fail, we must be double-deducting.
Assume(descendant->second.GetModFeesWithAncestors() >= anc->second.GetModifiedFee());
Assume(descendant->second.vsize_with_ancestors >= anc->second.GetTxSize());
descendant->second.fee_with_ancestors -= anc->second.GetModifiedFee();
descendant->second.vsize_with_ancestors -= anc->second.GetTxSize();
Assume(descendant->second.GetSizeWithAncestors() >= anc->second.GetTxSize());
descendant->second.UpdateAncestorState(-anc->second.GetTxSize(), -anc->second.GetModifiedFee());
}
}
// Delete these entries.

View File

@@ -19,12 +19,13 @@ class MiniMinerMempoolEntry
const CAmount fee_individual;
const CTransactionRef tx;
const int64_t vsize_individual;
CAmount fee_with_ancestors;
int64_t vsize_with_ancestors;
// 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()},
@@ -38,6 +39,10 @@ public:
int64_t GetTxSize() const { return vsize_individual; }
int64_t GetSizeWithAncestors() const { return vsize_with_ancestors; }
const CTransaction& GetTx() const LIFETIMEBOUND { return *tx; }
void UpdateAncestorState(int64_t vsize_change, CAmount fee_change) {
vsize_with_ancestors += vsize_change;
fee_with_ancestors += fee_change;
}
};
// Comparator needed for std::set<MockEntryMap::iterator>