scripted-diff: rename variables in policy/rbf

"Fee Delta" is already a term used for prioritizing transactions:
modified = base fees + delta

Here, delta also means the difference between original and modified replacement fees:
nDeltaFees = (original_base + original_delta) - (replacement_base + replacement_delta)

This is insanely confusing. Also, since mempool is no longer a member of a
class (MemPoolAccept.m_pool), the "m" prefix is unnecessary. The rest are
clarity/style-focused changes to already-touched lines.

-BEGIN VERIFY SCRIPT-

ren() { sed -i "s/\<$1\>/$2/g" src/policy/rbf* ; }

ren nDeltaFees additional_fees
ren m_pool pool

ren nSize replacement_vsize
ren nModifiedFees replacement_fees
ren nConflictingFees original_fees
ren oldFeeRate original_feerate
ren newFeeRate replacement_feerate

ren setAncestors ancestors
ren setIterConflicting iters_conflicting
ren setConflictsParents parents_of_conflicts
ren setConflicts direct_conflicts
ren allConflicting all_conflicts

sed -i "s/ hash\b/ txid/g" src/policy/rbf*
-END VERIFY SCRIPT-

Author: glozow

src/policy/rbf.cpp

@@ -13,7 +13,7 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
 {
     AssertLockHeld(pool.cs);
 
-    CTxMemPool::setEntries setAncestors;
+    CTxMemPool::setEntries ancestors;
 
     // First check the transaction itself.
     if (SignalsOptInRBF(tx)) {
@@ -31,9 +31,9 @@ RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool)
     uint64_t noLimit = std::numeric_limits<uint64_t>::max();
     std::string dummy;
     CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash());
-    pool.CalculateMemPoolAncestors(entry, setAncestors, noLimit, noLimit, noLimit, noLimit, dummy, false);
+    pool.CalculateMemPoolAncestors(entry, ancestors, noLimit, noLimit, noLimit, noLimit, dummy, false);
 
-    for (CTxMemPool::txiter it : setAncestors) {
+    for (CTxMemPool::txiter it : ancestors) {
         if (SignalsOptInRBF(it->GetTx())) {
             return RBFTransactionState::REPLACEABLE_BIP125;
         }
@@ -48,43 +48,43 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx)
 }
 
 std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx,
-                            CTxMemPool& m_pool,
-                            const CTxMemPool::setEntries& setIterConflicting,
-                            CTxMemPool::setEntries& allConflicting)
+                            CTxMemPool& pool,
+                            const CTxMemPool::setEntries& iters_conflicting,
+                            CTxMemPool::setEntries& all_conflicts)
 {
-    AssertLockHeld(m_pool.cs);
-    const uint256 hash = tx.GetHash();
+    AssertLockHeld(pool.cs);
+    const uint256 txid = tx.GetHash();
     uint64_t nConflictingCount = 0;
-    for (const auto& mi : setIterConflicting) {
+    for (const auto& mi : iters_conflicting) {
         nConflictingCount += mi->GetCountWithDescendants();
         // This potentially overestimates the number of actual descendants
         // but we just want to be conservative to avoid doing too much
         // work.
         if (nConflictingCount > MAX_BIP125_REPLACEMENT_CANDIDATES) {
             return strprintf("rejecting replacement %s; too many potential replacements (%d > %d)\n",
-                        hash.ToString(),
+                        txid.ToString(),
                         nConflictingCount,
                         MAX_BIP125_REPLACEMENT_CANDIDATES);
         }
     }
     // If not too many to replace, then calculate the set of
     // transactions that would have to be evicted
-    for (CTxMemPool::txiter it : setIterConflicting) {
-        m_pool.CalculateDescendants(it, allConflicting);
+    for (CTxMemPool::txiter it : iters_conflicting) {
+        pool.CalculateDescendants(it, all_conflicts);
     }
     return std::nullopt;
 }
 
 std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
-                                               const CTxMemPool& m_pool,
-                                               const CTxMemPool::setEntries& setIterConflicting)
+                                               const CTxMemPool& pool,
+                                               const CTxMemPool::setEntries& iters_conflicting)
 {
-    AssertLockHeld(m_pool.cs);
-    std::set<uint256> setConflictsParents;
-    for (const auto& mi : setIterConflicting) {
+    AssertLockHeld(pool.cs);
+    std::set<uint256> parents_of_conflicts;
+    for (const auto& mi : iters_conflicting) {
         for (const CTxIn &txin : mi->GetTx().vin)
         {
-            setConflictsParents.insert(txin.prevout.hash);
+            parents_of_conflicts.insert(txin.prevout.hash);
         }
     }
 
@@ -99,12 +99,12 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
         // this may break the CalculateMempoolAncestors RBF relaxation,
         // above. See the comment above the first CalculateMempoolAncestors
         // call for more info.
-        if (!setConflictsParents.count(tx.vin[j].prevout.hash))
+        if (!parents_of_conflicts.count(tx.vin[j].prevout.hash))
         {
             // Rather than check the UTXO set - potentially expensive -
             // it's cheaper to just check if the new input refers to a
             // tx that's in the mempool.
-            if (m_pool.exists(tx.vin[j].prevout.hash)) {
+            if (pool.exists(tx.vin[j].prevout.hash)) {
                 return strprintf("replacement %s adds unconfirmed input, idx %d",
                                  tx.GetHash().ToString(), j);
             }
@@ -113,14 +113,14 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx,
     return std::nullopt;
 }
 
-std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& setAncestors,
-                                                   const std::set<uint256>& setConflicts,
+std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
+                                                   const std::set<uint256>& direct_conflicts,
                                                    const uint256& txid)
 {
-    for (CTxMemPool::txiter ancestorIt : setAncestors)
+    for (CTxMemPool::txiter ancestorIt : ancestors)
     {
         const uint256 &hashAncestor = ancestorIt->GetTx().GetHash();
-        if (setConflicts.count(hashAncestor))
+        if (direct_conflicts.count(hashAncestor))
         {
             return strprintf("%s spends conflicting transaction %s",
                              txid.ToString(),
@@ -130,11 +130,11 @@ std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries&
     return std::nullopt;
 }
 
-std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& setIterConflicting,
-                                                 CFeeRate newFeeRate,
-                                                 const uint256& hash)
+std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& iters_conflicting,
+                                                 CFeeRate replacement_feerate,
+                                                 const uint256& txid)
 {
-    for (const auto& mi : setIterConflicting) {
+    for (const auto& mi : iters_conflicting) {
         // Don't allow the replacement to reduce the feerate of the
         // mempool.
         //
@@ -149,41 +149,41 @@ std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& s
         // mean high feerate children are ignored when deciding whether
         // or not to replace, we do require the replacement to pay more
         // overall fees too, mitigating most cases.
-        CFeeRate oldFeeRate(mi->GetModifiedFee(), mi->GetTxSize());
-        if (newFeeRate <= oldFeeRate)
+        CFeeRate original_feerate(mi->GetModifiedFee(), mi->GetTxSize());
+        if (replacement_feerate <= original_feerate)
         {
             return strprintf("rejecting replacement %s; new feerate %s <= old feerate %s",
-                             hash.ToString(),
-                             newFeeRate.ToString(),
-                             oldFeeRate.ToString());
+                             txid.ToString(),
+                             replacement_feerate.ToString(),
+                             original_feerate.ToString());
         }
     }
     return std::nullopt;
 }
 
-std::optional<std::string> PaysForRBF(CAmount nConflictingFees,
-                                      CAmount nModifiedFees,
-                                      size_t nSize,
-                                      const uint256& hash)
+std::optional<std::string> PaysForRBF(CAmount original_fees,
+                                      CAmount replacement_fees,
+                                      size_t replacement_vsize,
+                                      const uint256& txid)
 {
     // The replacement must pay greater fees than the transactions it
     // replaces - if we did the bandwidth used by those conflicting
     // transactions would not be paid for.
-    if (nModifiedFees < nConflictingFees)
+    if (replacement_fees < original_fees)
     {
         return strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s",
-                         hash.ToString(), FormatMoney(nModifiedFees), FormatMoney(nConflictingFees));
+                         txid.ToString(), FormatMoney(replacement_fees), FormatMoney(original_fees));
     }
 
     // Finally in addition to paying more fees than the conflicts the
     // new transaction must pay for its own bandwidth.
-    CAmount nDeltaFees = nModifiedFees - nConflictingFees;
-    if (nDeltaFees < ::incrementalRelayFee.GetFee(nSize))
+    CAmount additional_fees = replacement_fees - original_fees;
+    if (additional_fees < ::incrementalRelayFee.GetFee(replacement_vsize))
     {
         return strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s",
-                         hash.ToString(),
-                         FormatMoney(nDeltaFees),
-                         FormatMoney(::incrementalRelayFee.GetFee(nSize)));
+                         txid.ToString(),
+                         FormatMoney(additional_fees),
+                         FormatMoney(::incrementalRelayFee.GetFee(replacement_vsize)));
     }
     return std::nullopt;
 }

src/policy/rbf.h

@@ -35,61 +35,61 @@ enum class RBFTransactionState {
 RBFTransactionState IsRBFOptIn(const CTransaction& tx, const CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(pool.cs);
 RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx);
 
-/** Get all descendants of setIterConflicting. Also enforce BIP125 Rule #5, "The number of original
+/** Get all descendants of iters_conflicting. Also enforce BIP125 Rule #5, "The number of original
  * transactions to be replaced and their descendant transactions which will be evicted from the
  * mempool must not exceed a total of 100 transactions." Quit as early as possible. There cannot be
  * more than MAX_BIP125_REPLACEMENT_CANDIDATES potential entries.
- * @param[in]   setIterConflicting  The set of iterators to mempool entries.
- * @param[out]  allConflicting      Populated with all the mempool entries that would be replaced,
- *                                  which includes descendants of setIterConflicting. Not cleared at
+ * @param[in]   iters_conflicting  The set of iterators to mempool entries.
+ * @param[out]  all_conflicts      Populated with all the mempool entries that would be replaced,
+ *                                  which includes descendants of iters_conflicting. Not cleared at
  *                                  the start; any existing mempool entries will remain in the set.
  * @returns an error message if Rule #5 is broken, otherwise a std::nullopt.
  */
-std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, CTxMemPool& m_pool,
-                                                  const CTxMemPool::setEntries& setIterConflicting,
-                                                  CTxMemPool::setEntries& allConflicting)
-                                                  EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs);
+std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, CTxMemPool& pool,
+                                                  const CTxMemPool::setEntries& iters_conflicting,
+                                                  CTxMemPool::setEntries& all_conflicts)
+                                                  EXCLUSIVE_LOCKS_REQUIRED(pool.cs);
 
 /** BIP125 Rule #2: "The replacement transaction may only include an unconfirmed input if that input
  * was included in one of the original transactions."
  * @returns error message if Rule #2 is broken, otherwise std::nullopt. */
-std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTxMemPool& m_pool,
-                                               const CTxMemPool::setEntries& setIterConflicting)
-                                               EXCLUSIVE_LOCKS_REQUIRED(m_pool.cs);
+std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTxMemPool& pool,
+                                               const CTxMemPool::setEntries& iters_conflicting)
+                                               EXCLUSIVE_LOCKS_REQUIRED(pool.cs);
 
 /** Check the intersection between two sets of transactions (a set of mempool entries and a set of
  * txids) to make sure they are disjoint.
- * @param[in]   setAncestors    Set of mempool entries corresponding to ancestors of the
+ * @param[in]   ancestors    Set of mempool entries corresponding to ancestors of the
  *                              replacement transactions.
- * @param[in]   setConflicts    Set of txids corresponding to the mempool conflicts
+ * @param[in]   direct_conflicts    Set of txids corresponding to the mempool conflicts
  *                              (candidates to be replaced).
  * @param[in]   txid            Transaction ID, included in the error message if violation occurs.
  * @returns error message if the sets intersect, std::nullopt if they are disjoint.
  */
-std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& setAncestors,
-                                                   const std::set<uint256>& setConflicts,
+std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& ancestors,
+                                                   const std::set<uint256>& direct_conflicts,
                                                    const uint256& txid);
 
 /** Check that the feerate of the replacement transaction(s) is higher than the feerate of each
- * of the transactions in setIterConflicting.
- * @param[in]   setIterConflicting  The set of mempool entries.
+ * of the transactions in iters_conflicting.
+ * @param[in]   iters_conflicting  The set of mempool entries.
  * @returns error message if fees insufficient, otherwise std::nullopt.
  */
-std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& setIterConflicting,
-                                                 CFeeRate newFeeRate, const uint256& hash);
+std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& iters_conflicting,
+                                                 CFeeRate replacement_feerate, const uint256& txid);
 
 /** Enforce BIP125 Rule #3 "The replacement transaction pays an absolute fee of at least the sum
  * paid by the original transactions." Enforce BIP125 Rule #4 "The replacement transaction must also
  * pay for its own bandwidth at or above the rate set by the node's minimum relay fee setting."
- * @param[in]   nConflictingFees    Total modified fees of original transaction(s).
- * @param[in]   nModifiedFees       Total modified fees of replacement transaction(s).
- * @param[in]   nSize               Total virtual size of replacement transaction(s).
- * @param[in]   hash                Transaction ID, included in the error message if violation occurs.
+ * @param[in]   original_fees    Total modified fees of original transaction(s).
+ * @param[in]   replacement_fees       Total modified fees of replacement transaction(s).
+ * @param[in]   replacement_vsize               Total virtual size of replacement transaction(s).
+ * @param[in]   txid                Transaction ID, included in the error message if violation occurs.
  * @returns error string if fees are insufficient, otherwise std::nullopt.
  */
-std::optional<std::string> PaysForRBF(CAmount nConflictingFees,
-                                      CAmount nModifiedFees,
-                                      size_t nSize,
-                                      const uint256& hash);
+std::optional<std::string> PaysForRBF(CAmount original_fees,
+                                      CAmount replacement_fees,
+                                      size_t replacement_vsize,
+                                      const uint256& txid);
 
 #endif // BITCOIN_POLICY_RBF_H