Skip to content

Commit

Permalink
Clean up FinalizeSubpackage to avoid workspace-specific information
Browse files Browse the repository at this point in the history 
Also, use the "package hash" for logging replacements in the package rbf
setting.
  • Loading branch information
@sdaftuar
sdaftuar committed Nov 13, 2024
1 parent 57983b8 commit 34b6c58
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 24 deletions.
12 changes: 12 additions & 0 deletions src/txmempool.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -844,6 +844,18 @@ class CTxMemPool
return ret;
}

std::vector<CTransactionRef> GetAddedTxns() const {
std::vector<CTransactionRef> ret;
ret.reserve(m_entry_vec.size());
for (const auto& entry : m_entry_vec) {
ret.emplace_back(entry->GetSharedTx());
}
return ret;
}

size_t GetTxCount() const { return m_entry_vec.size(); }
const CTransaction& GetAddedTxn(size_t index) const { return m_entry_vec.at(index)->GetTx(); }

void Apply() EXCLUSIVE_LOCKS_REQUIRED(cs_main);

private:
Expand Down
61 changes: 37 additions & 24 deletions src/validation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ class MemPoolAccept
bool ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);

// Try to add the transaction to the mempool, removing any conflicts first.
void FinalizeSubpackage(const ATMPArgs& args, std::vector<Workspace>& ws) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);
void FinalizeSubpackage(const ATMPArgs& args) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_pool.cs);

// Submit all transactions to the mempool and call ConsensusScriptChecks to add to the script
// cache - should only be called after successful validation of all transactions in the package.
Expand Down Expand Up @@ -1283,34 +1283,48 @@ bool MemPoolAccept::ConsensusScriptChecks(const ATMPArgs& args, Workspace& ws)
return true;
}

void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args, std::vector<Workspace>& workspaces)
void MemPoolAccept::FinalizeSubpackage(const ATMPArgs& args)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_pool.cs);
const CTransaction& tx = *workspaces.front().m_ptx;
const uint256& hash = workspaces.front().m_hash;

if (!m_subpackage.m_all_conflicts.empty()) Assume(args.m_allow_replacement);
// Remove conflicting transactions from the mempool
for (CTxMemPool::txiter it : m_subpackage.m_all_conflicts)
{
LogDebug(BCLog::MEMPOOL, "replacing mempool tx %s (wtxid=%s, fees=%s, vsize=%s). New tx %s (wtxid=%s, fees=%s, vsize=%s)\n",
it->GetTx().GetHash().ToString(),
it->GetTx().GetWitnessHash().ToString(),
it->GetFee(),
it->GetTxSize(),
hash.ToString(),
tx.GetWitnessHash().ToString(),
workspaces[0].m_tx_handle->GetFee(),
workspaces[0].m_tx_handle->GetTxSize());
std::string log_string = strprintf("replacing mempool tx %s (wtxid=%s, fees=%s, vsize=%s). ",
it->GetTx().GetHash().ToString(),
it->GetTx().GetWitnessHash().ToString(),
it->GetFee(),
it->GetTxSize());
FeeFrac feerate{m_subpackage.m_total_modified_fees, int32_t(m_subpackage.m_total_vsize)};
uint256 tx_or_package_hash{};
if (m_subpackage.m_changeset->GetTxCount() == 1) {
const CTransaction& tx = m_subpackage.m_changeset->GetAddedTxn(0);
tx_or_package_hash = tx.GetHash();
log_string += strprintf("New tx %s (wtxid=%s, fees=%s, vsize=%s)",
tx.GetHash().ToString(),
tx.GetWitnessHash().ToString(),
feerate.fee,
feerate.size);
} else {
tx_or_package_hash = GetPackageHash(m_subpackage.m_changeset->GetAddedTxns());
log_string += strprintf("New package %s with %lu txs, fees=%s, vsize=%s",
tx_or_package_hash.ToString(),
m_subpackage.m_changeset->GetTxCount(),
feerate.fee,
feerate.size);

}
LogDebug(BCLog::MEMPOOL, "%s\n", log_string);
TRACEPOINT(mempool, replaced,
it->GetTx().GetHash().data(),
it->GetTxSize(),
it->GetFee(),
std::chrono::duration_cast<std::chrono::duration<std::uint64_t>>(it->GetTime()).count(),
hash.data(),
workspaces[0].m_tx_handle->GetTxSize(),
workspaces[0].m_tx_handle->GetFee()
tx_or_package_hash.data(),
feerate.size,
feerate.fee
);
m_subpackage.m_replaced_transactions.push_back(it->GetSharedTx());
}
Expand All @@ -1333,7 +1347,7 @@ bool MemPoolAccept::SubmitPackage(const ATMPArgs& args, std::vector<Workspace>&
return !m_pool.exists(GenTxid::Txid(ws.m_ptx->GetHash())); }));

bool all_submitted = true;
FinalizeSubpackage(args, workspaces);
FinalizeSubpackage(args);
// ConsensusScriptChecks adds to the script cache and is therefore consensus-critical;
// CheckInputsFromMempoolAndCache asserts that transactions only spend coins available from the
// mempool or UTXO set. Submit each transaction to the mempool immediately after calling
Expand Down Expand Up @@ -1402,8 +1416,7 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
AssertLockHeld(cs_main);
LOCK(m_pool.cs); // mempool "read lock" (held through m_pool.m_opts.signals->TransactionAddedToMempool())

std::vector<Workspace> workspaces{Workspace(ptx)};
Workspace &ws = workspaces.front();
Workspace ws(ptx);
const std::vector<Wtxid> single_wtxid{ws.m_ptx->GetWitnessHash()};

if (!PreChecks(args, ws)) {
Expand All @@ -1414,6 +1427,9 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
return MempoolAcceptResult::Failure(ws.m_state);
}

m_subpackage.m_total_vsize = ws.m_vsize;
m_subpackage.m_total_modified_fees = ws.m_modified_fees;

// Individual modified feerate exceeded caller-defined max; abort
if (args.m_client_maxfeerate && CFeeRate(ws.m_modified_fees, ws.m_vsize) > args.m_client_maxfeerate.value()) {
ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "max feerate exceeded", "");
Expand Down Expand Up @@ -1451,12 +1467,9 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
ws.m_base_fees, effective_feerate, single_wtxid);
}

FinalizeSubpackage(args, workspaces);
FinalizeSubpackage(args);

// trim mempool and check if tx was trimmed
// If we are validating a package, don't trim here because we could evict a previous transaction
// in the package. LimitMempoolSize() should be called at the very end to make sure the mempool
// is still within limits and package submission happens atomically.
// Limit the mempool, if appropriate.
if (!args.m_package_submission && !args.m_bypass_limits) {
LimitMempoolSize(m_pool, m_active_chainstate.CoinsTip());
if (!m_pool.exists(GenTxid::Txid(ws.m_hash))) {
Expand Down

0 comments on commit 34b6c58

Please sign in to comment.