diff options
| -rw-r--r-- | src/policy/rbf.cpp | 74 | ||||
| -rw-r--r-- | src/policy/rbf.h | 18 | ||||
| -rw-r--r-- | src/util/rbf.h | 14 |
3 files changed, 44 insertions, 62 deletions
diff --git a/src/policy/rbf.cpp b/src/policy/rbf.cpp index 8fe897dcea..15527afb8a 100644 --- a/src/policy/rbf.cpp +++ b/src/policy/rbf.cpp @@ -57,14 +57,13 @@ std::optional<std::string> GetEntriesForConflicts(const CTransaction& tx, uint64_t nConflictingCount = 0; 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. + // 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", - txid.ToString(), - nConflictingCount, - MAX_BIP125_REPLACEMENT_CANDIDATES); + txid.ToString(), + nConflictingCount, + MAX_BIP125_REPLACEMENT_CANDIDATES); } } // If not too many to replace, then calculate the set of @@ -82,28 +81,22 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, AssertLockHeld(pool.cs); std::set<uint256> parents_of_conflicts; for (const auto& mi : iters_conflicting) { - for (const CTxIn &txin : mi->GetTx().vin) - { + for (const CTxIn &txin : mi->GetTx().vin) { parents_of_conflicts.insert(txin.prevout.hash); } } - for (unsigned int j = 0; j < tx.vin.size(); j++) - { - // We don't want to accept replacements that require low - // feerate junk to be mined first. Ideally we'd keep track of - // the ancestor feerates and make the decision based on that, - // but for now requiring all new inputs to be confirmed works. + for (unsigned int j = 0; j < tx.vin.size(); j++) { + // We don't want to accept replacements that require low feerate junk to be mined first. + // Ideally we'd keep track of the ancestor feerates and make the decision based on that, but + // for now requiring all new inputs to be confirmed works. // - // Note that if you relax this to make RBF a little more useful, - // this may break the CalculateMempoolAncestors RBF relaxation, - // above. See the comment above the first CalculateMempoolAncestors - // call for more info. - 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. + // Note that if you relax this to make RBF a little more useful, this may break the + // CalculateMempoolAncestors RBF relaxation, above. See the comment above the first + // CalculateMempoolAncestors call for more info. + 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 (pool.exists(tx.vin[j].prevout.hash)) { return strprintf("replacement %s adds unconfirmed input, idx %d", tx.GetHash().ToString(), j); @@ -117,11 +110,9 @@ std::optional<std::string> EntriesAndTxidsDisjoint(const CTxMemPool::setEntries& const std::set<uint256>& direct_conflicts, const uint256& txid) { - for (CTxMemPool::txiter ancestorIt : ancestors) - { + for (CTxMemPool::txiter ancestorIt : ancestors) { const uint256 &hashAncestor = ancestorIt->GetTx().GetHash(); - if (direct_conflicts.count(hashAncestor)) - { + if (direct_conflicts.count(hashAncestor)) { return strprintf("%s spends conflicting transaction %s", txid.ToString(), hashAncestor.ToString()); @@ -135,23 +126,18 @@ std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& i const uint256& txid) { for (const auto& mi : iters_conflicting) { - // Don't allow the replacement to reduce the feerate of the - // mempool. + // Don't allow the replacement to reduce the feerate of the mempool. // - // We usually don't want to accept replacements with lower - // feerates than what they replaced as that would lower the - // feerate of the next block. Requiring that the feerate always - // be increased is also an easy-to-reason about way to prevent - // DoS attacks via replacements. + // We usually don't want to accept replacements with lower feerates than what they replaced + // as that would lower the feerate of the next block. Requiring that the feerate always be + // increased is also an easy-to-reason about way to prevent DoS attacks via replacements. // - // We only consider the feerates of transactions being directly - // replaced, not their indirect descendants. While that does - // 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. + // We only consider the feerates of transactions being directly replaced, not their indirect + // descendants. While that does 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 original_feerate(mi->GetModifiedFee(), mi->GetTxSize()); - if (replacement_feerate <= original_feerate) - { + if (replacement_feerate <= original_feerate) { return strprintf("rejecting replacement %s; new feerate %s <= old feerate %s", txid.ToString(), replacement_feerate.ToString(), @@ -169,8 +155,7 @@ std::optional<std::string> PaysForRBF(CAmount original_fees, // 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 (replacement_fees < original_fees) - { + if (replacement_fees < original_fees) { return strprintf("rejecting replacement %s, less fees than conflicting txs; %s < %s", txid.ToString(), FormatMoney(replacement_fees), FormatMoney(original_fees)); } @@ -178,8 +163,7 @@ std::optional<std::string> PaysForRBF(CAmount original_fees, // Finally in addition to paying more fees than the conflicts the // new transaction must pay for its own bandwidth. CAmount additional_fees = replacement_fees - original_fees; - if (additional_fees < ::incrementalRelayFee.GetFee(replacement_vsize)) - { + if (additional_fees < ::incrementalRelayFee.GetFee(replacement_vsize)) { return strprintf("rejecting replacement %s, not enough additional fees to relay; %s < %s", txid.ToString(), FormatMoney(additional_fees), diff --git a/src/policy/rbf.h b/src/policy/rbf.h index 55baae0fa2..56468a09b2 100644 --- a/src/policy/rbf.h +++ b/src/policy/rbf.h @@ -39,8 +39,8 @@ RBFTransactionState IsRBFOptInEmptyMempool(const CTransaction& tx); * 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] iters_conflicting The set of iterators to mempool entries. - * @param[out] all_conflicts Populated with all the mempool entries that would be replaced, + * @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. @@ -59,11 +59,11 @@ std::optional<std::string> HasNoNewUnconfirmed(const CTransaction& tx, const CTx /** 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] ancestors Set of mempool entries corresponding to ancestors of the - * replacement transactions. + * @param[in] ancestors Set of mempool entries corresponding to ancestors of the + * replacement transactions. * @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. + * (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& ancestors, @@ -81,9 +81,9 @@ std::optional<std::string> PaysMoreThanConflicts(const CTxMemPool::setEntries& i /** 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] 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] 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. */ diff --git a/src/util/rbf.h b/src/util/rbf.h index 4eb44b904f..6d44a2cb83 100644 --- a/src/util/rbf.h +++ b/src/util/rbf.h @@ -11,15 +11,13 @@ class CTransaction; static const uint32_t MAX_BIP125_RBF_SEQUENCE = 0xfffffffd; -/** Check whether the sequence numbers on this transaction are signaling -* opt-in to replace-by-fee, according to BIP 125. -* Allow opt-out of transaction replacement by setting -* nSequence > MAX_BIP125_RBF_SEQUENCE (SEQUENCE_FINAL-2) on all inputs. +/** Check whether the sequence numbers on this transaction are signaling opt-in to replace-by-fee, + * according to BIP 125. Allow opt-out of transaction replacement by setting nSequence > + * MAX_BIP125_RBF_SEQUENCE (SEQUENCE_FINAL-2) on all inputs. * -* SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by -* non-replaceable transactions. All inputs rather than just one -* is for the sake of multi-party protocols, where we don't -* want a single party to be able to disable replacement. */ +* SEQUENCE_FINAL-1 is picked to still allow use of nLockTime by non-replaceable transactions. All +* inputs rather than just one is for the sake of multi-party protocols, where we don't want a single +* party to be able to disable replacement. */ bool SignalsOptInRBF(const CTransaction &tx); #endif // BITCOIN_UTIL_RBF_H |