Skip to content

Commit

Permalink
[fees]: ignore txs that are CPFP'd
Browse files Browse the repository at this point in the history
- The fee estimator would assume that a miner was only
  incentivized by the fees of a transaction in isolation.

- This is inaccurate as some transactions are incentived by
  descendant(s). As such ignore transactions whose mining score
  is not the same as their individual fee rate.
  • Loading branch information
ismaelsadeeq committed May 9, 2024
1 parent cd610d1 commit 09e9898
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 1 deletion.
40 changes: 39 additions & 1 deletion src/policy/fees.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,37 @@ TxAncestorsAndDescendants GetTxAncestorsAndDescendants(const std::vector<Removed
return visited_txs;
}

node::LinearizationResult LinearizeTransactions(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block)
{
// Cache all the transactions for efficient lookup
std::map<Txid, TransactionInfo> tx_caches;
for(const auto& tx: txs_removed_for_block) {
tx_caches.insert({tx.info.m_tx->GetHash(), TransactionInfo(tx.info.m_tx, tx.info.m_fee, tx.info.m_virtual_transaction_size, tx.info.txHeight)});
}

const auto& txAncestorsAndDescendants = GetTxAncestorsAndDescendants(txs_removed_for_block);
std::vector<node::MiniMinerMempoolEntry> transactions;
std::map<Txid, std::set<Txid>> descendant_caches;
transactions.reserve(txAncestorsAndDescendants.size());

for (const auto& transaction : txAncestorsAndDescendants) {
const auto& txid = transaction.first;
const auto& [ancestors, descendants] = transaction.second;
int64_t vsize_ancestor = 0;
CAmount fee_with_ancestors = 0;
for (auto& ancestor_id : ancestors) {
const auto& ancestor = tx_caches.find(ancestor_id)->second;
vsize_ancestor += ancestor.m_virtual_transaction_size;
fee_with_ancestors += ancestor.m_fee;
}

descendant_caches.emplace(txid, descendants);
auto tx_info = tx_caches.find(txid)->second;
transactions.emplace_back(tx_info.m_tx, tx_info.m_virtual_transaction_size, vsize_ancestor, tx_info.m_fee, fee_with_ancestors);
}
return node::MiniMiner(std::move(transactions), std::move(descendant_caches)).Linearize();
}

namespace {

struct EncodedDoubleFormatter
Expand Down Expand Up @@ -719,9 +750,16 @@ void CBlockPolicyEstimator::processBlock(const std::vector<RemovedMempoolTransac

unsigned int countedTxs = 0;
// Update averages with data points from current block
const auto linearizedTransactions = LinearizeTransactions(txs_removed_for_block);
for (const auto& tx : txs_removed_for_block) {
if (processBlockTx(nBlockHeight, tx))
const auto tx_inclusion_order = linearizedTransactions.inclusion_order.find(tx.info.m_tx->GetHash())->second;
const auto mining_fee_rate = std::get<0>(linearizedTransactions.size_per_feerate[tx_inclusion_order]);
if (mining_fee_rate != CFeeRate(tx.info.m_fee, tx.info.m_virtual_transaction_size)) {
// Ignore all transactions whose mining score is not the same with it's fee rate
_removeTx(tx.info.m_tx->GetHash(), /* inBlock = */ true);
} else if (processBlockTx(nBlockHeight, tx)) {
countedTxs++;
}
}

if (firstRecordedHeight == 0 && countedTxs > 0) {
Expand Down
3 changes: 3 additions & 0 deletions src/policy/fees.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define BITCOIN_POLICY_FEES_H

#include <consensus/amount.h>
#include <node/mini_miner.h>
#include <policy/feerate.h>
#include <random.h>
#include <sync.h>
Expand Down Expand Up @@ -67,6 +68,8 @@ using TxAncestorsAndDescendants = std::map<Txid, std::tuple<std::set<Txid>, std:
*/
TxAncestorsAndDescendants GetTxAncestorsAndDescendants(const std::vector<RemovedMempoolTransactionInfo>& transactions);

node::LinearizationResult LinearizeTransactions(const std::vector<RemovedMempoolTransactionInfo>& txs_removed_for_block);

/* Enumeration of reason for returned fee estimate */
enum class FeeReason {
NONE,
Expand Down

0 comments on commit 09e9898

Please sign in to comment.