Skip to content

Commit

Permalink
mempool, CValidationInterface: modify `MempoolTransactionsRemovedForC…
Browse files Browse the repository at this point in the history
…onnectedBlock` parameter

Add `parents`, `nSizeWithAncestors`, and `nModFeesWithAncestors` entries to
`NewMempoolTransactionInfo`.

This commit then changes `MempoolTransactionsRemovedForConnectedBlock` parameter
`txs_removed_for_block` type to `NewMempoolTransactionInfo`.

This added info will enable 25380 even after this, vice versa.
assumed as being mined thanks to solely its own feerate when it's not".

see bitcoin#25380 (comment)
  • Loading branch information
ismaelsadeeq committed Sep 13, 2023
1 parent 1bdac51 commit caae68b
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 27 deletions.
12 changes: 12 additions & 0 deletions src/kernel/mempool_entry.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,15 @@ class CTxMemPoolEntry
const Parents& GetMemPoolParentsConst() const { return m_parents; }
const Children& GetMemPoolChildrenConst() const { return m_children; }
Parents& GetMemPoolParents() const { return m_parents; }
std::vector<CTransactionRef> GetMemPoolParentsCopy() const
{
std::vector<CTransactionRef> parents;
parents.reserve(m_parents.size());
for (const auto& parent : m_parents) {
parents.push_back(parent.get().GetSharedTx());
}
return parents;
}
Children& GetMemPoolChildren() const { return m_children; }

mutable size_t vTxHashesIdx; //!< Index in mempool's vTxHashes
Expand All @@ -178,6 +187,7 @@ class CTxMemPoolEntry

struct NewMempoolTransactionInfo {
CTransactionRef m_tx;
std::vector<CTransactionRef> m_parents;
//! The fee the added transaction paid
CAmount m_fee;
/**
Expand All @@ -191,6 +201,8 @@ struct NewMempoolTransactionInfo {
*/
int64_t m_virtual_transaction_size;
unsigned int txHeight;
int64_t nSizeWithAncestors;
CAmount nModFeesWithAncestors;
bool m_from_disconnected_block;
bool m_submitted_in_package;
bool m_chainstate_is_current;
Expand Down
8 changes: 4 additions & 4 deletions src/policy/fees.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTra
}

void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
const std::vector<CTransactionRef>& txs_removed_for_block)
const std::vector<NewMempoolTransactionInfo>& txs_removed_for_block)
{
LOCK(m_cs_fee_estimator);
if (nBlockHeight <= nBestSeenHeight) {
Expand Down Expand Up @@ -667,8 +667,8 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,

unsigned int countedTxs = 0;
// Update averages with data points from current block
for (const auto& tx : txs_removed_for_block) {
if (processBlockTx(nBlockHeight, tx))
for (const auto& tx_info : txs_removed_for_block) {
if (processBlockTx(nBlockHeight, tx_info.m_tx))
countedTxs++;
}

Expand Down Expand Up @@ -1050,7 +1050,7 @@ void CBlockPolicyEstimator::TransactionRemovedFromMempool(const CTransactionRef&
removeTx(tx->GetHash(), false);
}

void CBlockPolicyEstimator::MempoolTransactionsRemovedForConnectedBlock(const std::vector<CTransactionRef>& txs_removed_for_block, unsigned int nBlockHeight)
void CBlockPolicyEstimator::MempoolTransactionsRemovedForConnectedBlock(const std::vector<NewMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
{
processBlock(nBlockHeight, txs_removed_for_block);
}
Expand Down
4 changes: 2 additions & 2 deletions src/policy/fees.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ class CBlockPolicyEstimator : public CValidationInterface

/** Process all the transactions that have been included in a block */
void processBlock(unsigned int nBlockHeight,
const std::vector<CTransactionRef>& txs_removed_for_block)
const std::vector<NewMempoolTransactionInfo>& txs_removed_for_block)
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);

/** Process a transaction accepted to the mempool*/
Expand Down Expand Up @@ -269,7 +269,7 @@ class CBlockPolicyEstimator : public CValidationInterface
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) override
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);
void MempoolTransactionsRemovedForConnectedBlock(const std::vector<CTransactionRef>& txs_removed_for_block, unsigned int nBlockHeight) override
void MempoolTransactionsRemovedForConnectedBlock(const std::vector<NewMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) override
EXCLUSIVE_LOCKS_REQUIRED(!m_cs_fee_estimator);

private:
Expand Down
26 changes: 22 additions & 4 deletions src/test/fuzz/policy_estimator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,18 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
}
const CTransaction tx{*mtx};
const CTxMemPoolEntry entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx);
const NewMempoolTransactionInfo tx_info = {entry.GetSharedTx(), entry.GetFee(), entry.GetTxSize(), entry.GetHeight(),
fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool(), fuzzed_data_provider.ConsumeBool()};
NewMempoolTransactionInfo tx_info = {
entry.GetSharedTx(),
entry.GetMemPoolParentsCopy(),
entry.GetFee(),
entry.GetTxSize(),
entry.GetHeight(),
entry.GetSizeWithAncestors(),
entry.GetModFeesWithAncestors(),
fuzzed_data_provider.ConsumeBool(),
fuzzed_data_provider.ConsumeBool(),
fuzzed_data_provider.ConsumeBool(),
fuzzed_data_provider.ConsumeBool()};
block_policy_estimator.processTransaction(tx_info);
if (fuzzed_data_provider.ConsumeBool()) {
(void)block_policy_estimator.removeTx(tx.GetHash(), /*inBlock=*/fuzzed_data_provider.ConsumeBool());
Expand All @@ -60,10 +70,18 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
const CTransaction tx{*mtx};
mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx));
}
std::vector<CTransactionRef> txs;
std::vector<NewMempoolTransactionInfo> txs;
txs.reserve(mempool_entries.size());
for (const CTxMemPoolEntry& mempool_entry : mempool_entries) {
txs.push_back(mempool_entry.GetSharedTx());
NewMempoolTransactionInfo tx_info;
tx_info.m_tx = mempool_entry.GetSharedTx();
tx_info.m_parents = mempool_entry.GetMemPoolParentsCopy();
tx_info.m_fee = mempool_entry.GetFee();
tx_info.m_virtual_transaction_size = mempool_entry.GetTxSize();
tx_info.txHeight = mempool_entry.GetHeight();
tx_info.nSizeWithAncestors = mempool_entry.GetSizeWithAncestors();
tx_info.nModFeesWithAncestors = mempool_entry.GetModFeesWithAncestors();
txs.push_back(tx_info);
}
block_policy_estimator.processBlock(fuzzed_data_provider.ConsumeIntegral<unsigned int>(), txs);
},
Expand Down
42 changes: 30 additions & 12 deletions src/test/policyestimator_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
// Since TransactionAddedToMempool callbacks are generated in ATMP,
// not addUnchecked, we cheat and create one manually here
int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx));
GetMainSignals().TransactionAddedToMempool(
{MakeTransactionRef(tx),
feeV[j],
virtual_size,
entry.nHeight,
false,
false,
true,
true},
mpool.GetAndIncrementSequence());
NewMempoolTransactionInfo tx_info;
tx_info.m_tx = MakeTransactionRef(tx);
tx_info.m_fee = feeV[j];
tx_info.m_virtual_transaction_size = virtual_size;
tx_info.txHeight = entry.nHeight;
tx_info.m_from_disconnected_block = false;
tx_info.m_submitted_in_package = false;
tx_info.m_chainstate_is_current = true;
tx_info.m_has_no_mempool_parents = true;
GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
}
txHashes[j].push_back(hash);
}
Expand Down Expand Up @@ -165,7 +165,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
// Since TransactionAddedToMempool callbacks are generated in ATMP,
// not addUnchecked, we cheat and create one manually here
int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx));
GetMainSignals().TransactionAddedToMempool({MakeTransactionRef(tx), feeV[j], virtual_size, entry.nHeight, false, false, true, true}, mpool.GetAndIncrementSequence());
NewMempoolTransactionInfo tx_info;
tx_info.m_tx = MakeTransactionRef(tx);
tx_info.m_fee = feeV[j];
tx_info.m_virtual_transaction_size = virtual_size;
tx_info.txHeight = entry.nHeight;
tx_info.m_from_disconnected_block = false;
tx_info.m_submitted_in_package = false;
tx_info.m_chainstate_is_current = true;
tx_info.m_has_no_mempool_parents = true;
GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
}
txHashes[j].push_back(hash);
}
Expand Down Expand Up @@ -221,7 +230,16 @@ BOOST_AUTO_TEST_CASE(BlockPolicyEstimates)
// Since TransactionAddedToMempool callbacks are generated in ATMP,
// not addUnchecked, we cheat and create one manually here
int64_t virtual_size = GetVirtualTransactionSize(*MakeTransactionRef(tx));
GetMainSignals().TransactionAddedToMempool({MakeTransactionRef(tx), feeV[j], virtual_size, entry.nHeight, false, false, true, true}, mpool.GetAndIncrementSequence());
NewMempoolTransactionInfo tx_info;
tx_info.m_tx = MakeTransactionRef(tx);
tx_info.m_fee = feeV[j];
tx_info.m_virtual_transaction_size = virtual_size;
tx_info.txHeight = entry.nHeight;
tx_info.m_from_disconnected_block = false;
tx_info.m_submitted_in_package = false;
tx_info.m_chainstate_is_current = true;
tx_info.m_has_no_mempool_parents = true;
GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
}
CTransactionRef ptx = mpool.get(hash);
if (ptx)
Expand Down
17 changes: 15 additions & 2 deletions src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -615,15 +615,28 @@ void CTxMemPool::removeConflicts(const CTransaction &tx)
void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigned int nBlockHeight)
{
AssertLockHeld(cs);
std::vector<CTransactionRef> txs_removed_for_block;
std::vector<NewMempoolTransactionInfo> txs_removed_for_block;
txs_removed_for_block.reserve(vtx.size());
for (const auto& tx : vtx)
{
txiter it = mapTx.find(tx->GetHash());
if (it != mapTx.end()) {
NewMempoolTransactionInfo tx_info;
tx_info.m_tx = it->GetSharedTx();
tx_info.m_parents = it->GetMemPoolParentsCopy();
tx_info.m_fee = it->GetFee();
tx_info.m_virtual_transaction_size = it->GetTxSize();
tx_info.txHeight = it->GetHeight();
tx_info.nSizeWithAncestors = it->GetSizeWithAncestors();
tx_info.nModFeesWithAncestors = it->GetModFeesWithAncestors();
txs_removed_for_block.push_back(tx_info);
}
}
for (const auto& tx : vtx) {
txiter it = mapTx.find(tx->GetHash());
if (it != mapTx.end()) {
setEntries stage;
stage.insert(it);
txs_removed_for_block.push_back(tx);
RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
}
removeConflicts(*tx);
Expand Down
6 changes: 6 additions & 0 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1193,9 +1193,12 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
const CTransaction& tx = *ws.m_ptx;
NewMempoolTransactionInfo tx_info = {
ws.m_ptx,
ws.m_entry->GetMemPoolParentsCopy(),
ws.m_base_fees,
ws.m_vsize,
ws.m_entry->GetHeight(),
ws.m_entry->GetSizeWithAncestors(),
ws.m_entry->GetModFeesWithAncestors(),
args.m_bypass_limits,
args.m_package_submission,
IsCurrentForFeeEstimation(m_active_chainstate),
Expand Down Expand Up @@ -1241,9 +1244,12 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
const CTransaction& tx = *ws.m_ptx;
NewMempoolTransactionInfo tx_info = {
ws.m_ptx,
ws.m_entry->GetMemPoolParentsCopy(),
ws.m_base_fees,
ws.m_vsize,
ws.m_entry->GetHeight(),
ws.m_entry->GetSizeWithAncestors(),
ws.m_entry->GetModFeesWithAncestors(),
args.m_bypass_limits,
args.m_package_submission,
IsCurrentForFeeEstimation(m_active_chainstate),
Expand Down
2 changes: 1 addition & 1 deletion src/validationinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ void CMainSignals::TransactionAddedToMempool(const NewMempoolTransactionInfo& tx
tx_info.m_tx->GetWitnessHash().ToString());
}

void CMainSignals::MempoolTransactionsRemovedForConnectedBlock(const std::vector<CTransactionRef>& txs_removed_for_block, unsigned int nBlockHeight)
void CMainSignals::MempoolTransactionsRemovedForConnectedBlock(const std::vector<NewMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight)
{
auto event = [txs_removed_for_block, nBlockHeight, this] {
m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.MempoolTransactionsRemovedForConnectedBlock(txs_removed_for_block, nBlockHeight); });
Expand Down
4 changes: 2 additions & 2 deletions src/validationinterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class CValidationInterface {
*
* Called on a background thread.
*/
virtual void MempoolTransactionsRemovedForConnectedBlock(const std::vector<CTransactionRef>& txs_removed_for_block, unsigned int nBlockHeight) {}
virtual void MempoolTransactionsRemovedForConnectedBlock(const std::vector<NewMempoolTransactionInfo>& txs_removed_for_block, unsigned int nBlockHeight) {}
/**
* Notifies listeners of a block being connected.
* Provides the block that was connected.
Expand Down Expand Up @@ -209,7 +209,7 @@ class CMainSignals {
void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload);
void TransactionAddedToMempool(const NewMempoolTransactionInfo&, uint64_t mempool_sequence);
void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason, uint64_t mempool_sequence);
void MempoolTransactionsRemovedForConnectedBlock(const std::vector<CTransactionRef>&, unsigned int nBlockHeight);
void MempoolTransactionsRemovedForConnectedBlock(const std::vector<NewMempoolTransactionInfo>&, unsigned int nBlockHeight);
void BlockConnected(const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);
void ChainStateFlushed(const CBlockLocator &);
Expand Down

0 comments on commit caae68b

Please sign in to comment.