Skip to content

Commit 9e910d8

Browse files
committed
scripted-diff: clean up MemPoolAccept aliases
The aliases are leftover from a previous MOVEONLY refactor - they are unnecessary and removing them reduces the diff for splitting out mempool Checks from PreChecks, making RBF variables MemPoolAccept-wide, etc. -BEGIN VERIFY SCRIPT- unalias() { sed -i "s:\<$1\>:$2:g" src/validation.cpp; sed -i "/$2 = $2/d" src/validation.cpp; } unalias nModifiedFees ws.m_modified_fees unalias nConflictingFees ws.m_conflicting_fees unalias nConflictingSize ws.m_conflicting_size unalias setConflicts ws.m_conflicts unalias allConflicting ws.m_all_conflicting unalias setAncestors ws.m_ancestors -END VERIFY SCRIPT-
1 parent fd92b0c commit 9e910d8

File tree

1 file changed

+25
-36
lines changed

1 file changed

+25
-36
lines changed

src/validation.cpp

Lines changed: 25 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -603,13 +603,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
603603

604604
// Alias what we need out of ws
605605
TxValidationState& state = ws.m_state;
606-
std::set<uint256>& setConflicts = ws.m_conflicts;
607-
CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting;
608-
CTxMemPool::setEntries& setAncestors = ws.m_ancestors;
609606
std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry;
610-
CAmount& nModifiedFees = ws.m_modified_fees;
611-
CAmount& nConflictingFees = ws.m_conflicting_fees;
612-
size_t& nConflictingSize = ws.m_conflicting_size;
613607

614608
if (!CheckTransaction(tx, state)) {
615609
return false; // state filled in by CheckTransaction
@@ -655,7 +649,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
655649
// Transaction conflicts with a mempool tx, but we're not allowing replacements.
656650
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "bip125-replacement-disallowed");
657651
}
658-
if (!setConflicts.count(ptxConflicting->GetHash()))
652+
if (!ws.m_conflicts.count(ptxConflicting->GetHash()))
659653
{
660654
// Transactions that don't explicitly signal replaceability are
661655
// *not* replaceable with the current logic, even if one of their
@@ -668,7 +662,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
668662
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "txn-mempool-conflict");
669663
}
670664

671-
setConflicts.insert(ptxConflicting->GetHash());
665+
ws.m_conflicts.insert(ptxConflicting->GetHash());
672666
}
673667
}
674668
}
@@ -732,9 +726,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
732726

733727
int64_t nSigOpsCost = GetTransactionSigOpCost(tx, m_view, STANDARD_SCRIPT_VERIFY_FLAGS);
734728

735-
// nModifiedFees includes any fee deltas from PrioritiseTransaction
736-
nModifiedFees = ws.m_base_fees;
737-
m_pool.ApplyDelta(hash, nModifiedFees);
729+
// ws.m_modified_fees includes any fee deltas from PrioritiseTransaction
730+
ws.m_modified_fees = ws.m_base_fees;
731+
m_pool.ApplyDelta(hash, ws.m_modified_fees);
738732

739733
// Keep track of transactions that spend a coinbase, which we re-scan
740734
// during reorgs to ensure COINBASE_MATURITY is still met.
@@ -757,11 +751,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
757751

758752
// No transactions are allowed below minRelayTxFee except from disconnected
759753
// blocks
760-
if (!bypass_limits && !CheckFeeRate(ws.m_vsize, nModifiedFees, state)) return false;
754+
if (!bypass_limits && !CheckFeeRate(ws.m_vsize, ws.m_modified_fees, state)) return false;
761755

762-
ws.m_iters_conflicting = m_pool.GetIterSet(setConflicts);
756+
ws.m_iters_conflicting = m_pool.GetIterSet(ws.m_conflicts);
763757
// Calculate in-mempool ancestors, up to a limit.
764-
if (setConflicts.size() == 1) {
758+
if (ws.m_conflicts.size() == 1) {
765759
// In general, when we receive an RBF transaction with mempool conflicts, we want to know whether we
766760
// would meet the chain limits after the conflicts have been removed. However, there isn't a practical
767761
// way to do this short of calculating the ancestor and descendant sets with an overlay cache of
@@ -797,8 +791,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
797791
}
798792

799793
std::string errString;
800-
if (!m_pool.CalculateMemPoolAncestors(*entry, setAncestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) {
801-
setAncestors.clear();
794+
if (!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, errString)) {
795+
ws.m_ancestors.clear();
802796
// If CalculateMemPoolAncestors fails second time, we want the original error string.
803797
std::string dummy_err_string;
804798
// Contracting/payment channels CPFP carve-out:
@@ -813,24 +807,24 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
813807
// outputs - one for each counterparty. For more info on the uses for
814808
// this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html
815809
if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT ||
816-
!m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) {
810+
!m_pool.CalculateMemPoolAncestors(*entry, ws.m_ancestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) {
817811
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString);
818812
}
819813
}
820814

821815
// A transaction that spends outputs that would be replaced by it is invalid. Now
822816
// that we have the set of all ancestors we can detect this
823-
// pathological case by making sure setConflicts and setAncestors don't
817+
// pathological case by making sure ws.m_conflicts and ws.m_ancestors don't
824818
// intersect.
825-
if (const auto err_string{EntriesAndTxidsDisjoint(setAncestors, setConflicts, hash)}) {
819+
if (const auto err_string{EntriesAndTxidsDisjoint(ws.m_ancestors, ws.m_conflicts, hash)}) {
826820
// We classify this as a consensus error because a transaction depending on something it
827821
// conflicts with would be inconsistent.
828822
return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-txns-spends-conflicting-tx", *err_string);
829823
}
830824

831-
m_rbf = !setConflicts.empty();
825+
m_rbf = !ws.m_conflicts.empty();
832826
if (m_rbf) {
833-
CFeeRate newFeeRate(nModifiedFees, ws.m_vsize);
827+
CFeeRate newFeeRate(ws.m_modified_fees, ws.m_vsize);
834828
// It's possible that the replacement pays more fees than its direct conflicts but not more
835829
// than all conflicts (i.e. the direct conflicts have high-fee descendants). However, if the
836830
// replacement doesn't pay more fees than its direct conflicts, then we can be sure it's not
@@ -842,7 +836,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
842836
}
843837

844838
// Calculate all conflicting entries and enforce BIP125 Rule #5.
845-
if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, allConflicting)}) {
839+
if (const auto err_string{GetEntriesForConflicts(tx, m_pool, ws.m_iters_conflicting, ws.m_all_conflicting)}) {
846840
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY,
847841
"too many potential replacements", *err_string);
848842
}
@@ -854,11 +848,11 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
854848

855849
// Check if it's economically rational to mine this transaction rather than the ones it
856850
// replaces and pays for its own relay fees. Enforce BIP125 Rules #3 and #4.
857-
for (CTxMemPool::txiter it : allConflicting) {
858-
nConflictingFees += it->GetModifiedFee();
859-
nConflictingSize += it->GetTxSize();
851+
for (CTxMemPool::txiter it : ws.m_all_conflicting) {
852+
ws.m_conflicting_fees += it->GetModifiedFee();
853+
ws.m_conflicting_size += it->GetTxSize();
860854
}
861-
if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, ws.m_vsize,
855+
if (const auto err_string{PaysForRBF(ws.m_conflicting_fees, ws.m_modified_fees, ws.m_vsize,
862856
::incrementalRelayFee, hash)}) {
863857
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string);
864858
}
@@ -931,24 +925,19 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws)
931925
TxValidationState& state = ws.m_state;
932926
const bool bypass_limits = args.m_bypass_limits;
933927

934-
CTxMemPool::setEntries& allConflicting = ws.m_all_conflicting;
935-
CTxMemPool::setEntries& setAncestors = ws.m_ancestors;
936-
const CAmount& nModifiedFees = ws.m_modified_fees;
937-
const CAmount& nConflictingFees = ws.m_conflicting_fees;
938-
const size_t& nConflictingSize = ws.m_conflicting_size;
939928
std::unique_ptr<CTxMemPoolEntry>& entry = ws.m_entry;
940929

941930
// Remove conflicting transactions from the mempool
942-
for (CTxMemPool::txiter it : allConflicting)
931+
for (CTxMemPool::txiter it : ws.m_all_conflicting)
943932
{
944933
LogPrint(BCLog::MEMPOOL, "replacing tx %s with %s for %s additional fees, %d delta bytes\n",
945934
it->GetTx().GetHash().ToString(),
946935
hash.ToString(),
947-
FormatMoney(nModifiedFees - nConflictingFees),
948-
(int)entry->GetTxSize() - (int)nConflictingSize);
936+
FormatMoney(ws.m_modified_fees - ws.m_conflicting_fees),
937+
(int)entry->GetTxSize() - (int)ws.m_conflicting_size);
949938
ws.m_replaced_transactions.push_back(it->GetSharedTx());
950939
}
951-
m_pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED);
940+
m_pool.RemoveStaged(ws.m_all_conflicting, false, MemPoolRemovalReason::REPLACED);
952941

953942
// This transaction should only count for fee estimation if:
954943
// - it's not being re-added during a reorg which bypasses typical mempool fee limits
@@ -957,7 +946,7 @@ bool MemPoolAccept::Finalize(const ATMPArgs& args, Workspace& ws)
957946
bool validForFeeEstimation = !bypass_limits && IsCurrentForFeeEstimation(m_active_chainstate) && m_pool.HasNoInputsOf(tx);
958947

959948
// Store transaction in memory
960-
m_pool.addUnchecked(*entry, setAncestors, validForFeeEstimation);
949+
m_pool.addUnchecked(*entry, ws.m_ancestors, validForFeeEstimation);
961950

962951
// trim mempool and check if tx was trimmed
963952
if (!bypass_limits) {

0 commit comments

Comments
 (0)