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 fix merge conflict with 25380.
  • Loading branch information
ismaelsadeeq committed Nov 2, 2023
1 parent 7ec4f96 commit 53bbc79
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 30 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 @@ -637,7 +637,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 @@ -666,8 +666,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 @@ -1049,7 +1049,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
18 changes: 14 additions & 4 deletions src/test/fuzz/policy_estimator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,16 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator)
const CTxMemPoolEntry entry = ConsumeTxMemPoolEntry(fuzzed_data_provider, tx);
const NewMempoolTransactionInfo tx_info = {
.m_tx = entry.GetSharedTx(),
.m_parents = entry.GetMemPoolParentsCopy(),
.m_fee = entry.GetFee(),
.m_virtual_transaction_size = entry.GetTxSize(),
.txHeight = entry.GetHeight(),
.nSizeWithAncestors = entry.GetSizeWithAncestors(),
.nModFeesWithAncestors = entry.GetModFeesWithAncestors(),
.m_from_disconnected_block = fuzzed_data_provider.ConsumeBool(),
.m_submitted_in_package = fuzzed_data_provider.ConsumeBool(),
.m_chainstate_is_current = fuzzed_data_provider.ConsumeBool(),
.m_has_no_mempool_parents = fuzzed_data_provider.ConsumeBool()
};
.m_has_no_mempool_parents = 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 @@ -68,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 = {
.m_tx = mempool_entry.GetSharedTx(),
.m_parents = mempool_entry.GetMemPoolParentsCopy(),
.m_fee = mempool_entry.GetFee(),
.m_virtual_transaction_size = mempool_entry.GetTxSize(),
.txHeight = mempool_entry.GetHeight(),
.nSizeWithAncestors = mempool_entry.GetSizeWithAncestors(),
.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());
const NewMempoolTransactionInfo tx_info = {
.m_tx = MakeTransactionRef(tx),
.m_fee = feeV[j],
.m_virtual_transaction_size = virtual_size,
.txHeight = entry.nHeight,
.m_from_disconnected_block = false,
.m_submitted_in_package = false,
.m_chainstate_is_current = true,
.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());
const NewMempoolTransactionInfo tx_info = {
.m_tx = MakeTransactionRef(tx),
.m_fee = feeV[j],
.m_virtual_transaction_size = virtual_size,
.txHeight = entry.nHeight,
.m_from_disconnected_block = false,
.m_submitted_in_package = false,
.m_chainstate_is_current = true,
.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());
const NewMempoolTransactionInfo tx_info = {
.m_tx = MakeTransactionRef(tx),
.m_fee = feeV[j],
.m_virtual_transaction_size = virtual_size,
.txHeight = entry.nHeight,
.m_from_disconnected_block = false,
.m_submitted_in_package = false,
.m_chainstate_is_current = true,
.m_has_no_mempool_parents = true};
GetMainSignals().TransactionAddedToMempool(tx_info, mpool.GetAndIncrementSequence());
}
CTransactionRef ptx = mpool.get(hash);
if (ptx)
Expand Down
21 changes: 16 additions & 5 deletions src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -630,21 +630,32 @@ 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()) {
setEntries stage;
stage.insert(it);
txs_removed_for_block.push_back(tx);
RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
NewMempoolTransactionInfo tx_info = {
.m_tx = it->GetSharedTx(),
.m_parents = it->GetMemPoolParentsCopy(),
.m_fee = it->GetFee(),
.m_virtual_transaction_size = it->GetTxSize(),
.txHeight = it->GetHeight(),
.nSizeWithAncestors = it->GetSizeWithAncestors(),
.nModFeesWithAncestors = it->GetModFeesWithAncestors()};
txs_removed_for_block.push_back(tx_info);
}
removeConflicts(*tx);
ClearPrioritisation(tx->GetHash());
}
GetMainSignals().MempoolTransactionsRemovedForConnectedBlock(txs_removed_for_block, nBlockHeight);
for (const auto& tx_info : txs_removed_for_block) {
txiter it = *CHECK_NONFATAL(GetIter(tx_info.m_tx->GetHash()));
setEntries stage;
stage.insert(it);
RemoveStaged(stage, true, MemPoolRemovalReason::BLOCK);
}
lastRollingFeeUpdate = GetTime();
blockSinceLastRollingFeeBump = true;
}
Expand Down
6 changes: 6 additions & 0 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1209,9 +1209,12 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
const CTransaction& tx = *ws.m_ptx;
const NewMempoolTransactionInfo tx_info = {
.m_tx = ws.m_ptx,
.m_parents = ws.m_entry->GetMemPoolParentsCopy(),
.m_fee = ws.m_base_fees,
.m_virtual_transaction_size = ws.m_vsize,
.txHeight = ws.m_entry->GetHeight(),
.nSizeWithAncestors = ws.m_entry->GetSizeWithAncestors(),
.nModFeesWithAncestors = ws.m_entry->GetModFeesWithAncestors(),
.m_from_disconnected_block = args.m_bypass_limits,
.m_submitted_in_package = args.m_package_submission,
.m_chainstate_is_current = IsCurrentForFeeEstimation(m_active_chainstate),
Expand Down Expand Up @@ -1251,9 +1254,12 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
const CTransaction& tx = *ws.m_ptx;
const NewMempoolTransactionInfo tx_info = {
.m_tx = ws.m_ptx,
.m_parents = ws.m_entry->GetMemPoolParentsCopy(),
.m_fee = ws.m_base_fees,
.m_virtual_transaction_size = ws.m_vsize,
.txHeight = ws.m_entry->GetHeight(),
.nSizeWithAncestors = ws.m_entry->GetSizeWithAncestors(),
.nModFeesWithAncestors = ws.m_entry->GetModFeesWithAncestors(),
.m_from_disconnected_block = args.m_bypass_limits,
.m_submitted_in_package = args.m_package_submission,
.m_chainstate_is_current = 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 @@ -216,7 +216,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 @@ -138,7 +138,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 @@ -212,7 +212,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(ChainstateRole, const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);
void ChainStateFlushed(ChainstateRole, const CBlockLocator &);
Expand Down

0 comments on commit 53bbc79

Please sign in to comment.