Replace by Fee (RBF) is a method of fee-bumping a transaction by creating a higher-feerate
replacement transaction that spends the same inputs as their original transaction.
BIP125 specifies how users can
signal replaceability in their transactions and a set of conditions that replacement transactions
must meet in order to be accepted.
RBF is an example of mempool policy, a set of validation rules applied to unconfirmed
transactions.
PR #22675 extracts the RBF logic into a
policy/rbf module and adds documentation. This allows each function to be unit tested and reused
for future projects such as RBF in package validation and Witness Replacement.
What are some benefits of extracting the RBF logic into its own module?
Why is the error returned
here a TxValidationResult::TX_CONSENSUS instead of
TxValidationResult::TX_POLICY, given that this is RBF-related logic?
In BIP125 Rule
#2, why
is it important for the replacement transaction to not introduce any new unconfirmed inputs?
The code in PaysMoreThanConflicts() doesn’t seem to directly correspond to a rule in BIP125. Why do
we have it, and why don’t we merge it with the other fee-related checks in
PaysForRBF()?
In BIP125 Rule
#4, what
does it mean for a transaction to “pay for its own bandwidth?” Why don’t we just allow any
replacement as long as it has a higher feerate?
In BIP125 Rule
#5, how
is it possible for a transaction to conflict with more than 100 mempool transactions, given that
mempool ancestor and descendant limits don’t allow a chain of more than 25 transactions?
In
PaysForRBF(),
why is hash passed by reference but original_fees by value? Should the fee parameters be
const? Why or why not?
When considering a transaction for submission to the mempool, we check to see if it would cause any
mempool entries to exceed their descendant limits. How do we make sure we don’t overestimate by
counting both the new and to-be-replaced transactions, since they would share ancestors?
This PR introduces a circular dependency: policy/rbf -> txmempool -> validation -> policy/rbf.
This can be avoided by not making policy/rbf depend on txmempool or by using the approach in
PR #22677. Which approach do you prefer, and why?
<larryruane> yes I think it's that currently, if we get a tx that is the same as one already in our mempool except that its witness is different, we currently reject it, but we should accept it if its witness is smaller (higher fee rate), at least by a certain margin
<larryruane> and what is "nice" about that type of replacement is that we don't have to evict any ancestors or decendents, since the replacement tx must specify exactly the same of those
<dunxen> for it being a consensus error, i'm not sure about it in depth, but isn't it basically checking for a tx that spends outputs that would be replaced by it. seems like consensus issue
<larryruane> glozow: ".. CONSENSUS .." because if the replacement were to happen (let's say B replaces A), then B couldn't possibly be valid, since one of its inputs is one of B's outputs!
<glozow> dunxen: larryruane: yes exactly. this is a consensus error because this transaction must be inconsistent if it's dependencies and conflicts intersect
<t-bast> If I'm side-tracking, you can answer later: will we have a way to get rid of this rule for package-RBF? It's an annoying pinning vector and needed the special carve-out rule to be added, which is somewhat hackish...
<t-bast> glozow: we should double-check with BlueMatt, but I think it would be nice to get rid of it, it made anchor outputs useless for lightning until Matt added the carve-out exception...
<glozow> dunxen: yes! :) you can imagine that, if the new replacement transaction adds higher fees, but has a low ancestor fee, it's not actually going to be a better candidate for miners
<glozow> this isn't an "attack" but it's a case we want to consider when we're deciding whether or not a new transaction is a better candidate for mining
<glozow> next question: The code in `PaysMoreThanConflicts()` doesn’t seem to directly correspond to a rule in BIP125. Why do we have it, and why don’t we merge it with the other fee-related checks in `PaysForRBF()`?
<larryruane> as a higher-level point then, we're trying to run our mempool management as if we're a miner (like, what would a miner think is good), really (i think?) so that we can *forward* transactions appropriately ... but miners could act differently than we expect! E.g. a miner *could* replace that tx (that has a new unconfirmed input)
<glozow> larryruane: yes, I consider this to be one of the main goals of mempool - to keep the most incentive-compatible transaction candidates for mining, even if we're not a miner
<sipa> there is a fundamental conflict between DoS protection (e.g. not allowing people to use the P2P network to broadcast transactions that will never confirm), and miner incentives (maximizing joint fee of the top 1 block worth of the mempool)... our goal so far is minimizing / specifying where that conflict occurs, but to an extent, it's inevitble
<sipa> i guess there is an additional one: our mempool is also _our_ prediction of what will confirm, and in theory, miners could keep mempools with e.g. conflicting transactions around, and pick the best combination at the time of mining
<michaelfolkson> sipa: And BlueMatt said the other day Lightning wants the most relaxed mempool policies possible (partly in jest perhaps). So that is another factor in the conflict
<glozow> dunxen: yes! the best way to avoid expensive operations is to disallow replacing transactions, but we also want to allow replacements so that we can get better-fee transactions. so this is a happy middle
<sipa> it's particularly worrisome for Lightning and perhaps other 2nd layer protocols, as their correctness relies on being able to predict what will confirm, which is... eh
<glozow> i feel like dust doesn't fall into the buckets of anti-DoS and useful mempool, but into a 3rd category of "(non-consensus) rules we think are good for the network to follow"
<glozow> anyway, I'll answer the last question: `PaysMoreThanConflicts()` is an early-exit thing. We haven't gone digging in the mempool for descendants-of-conflicts yet, but we can compare fees with the direct conflicts because we'd be able to exit immediately if we're not paying more than them.
<michaelfolkson> A big mempool and the prediction element isn't important. A small mempool and then we're potentially talking retrieving transactions we don't have in our mempool to validate a block (not ideal)
<glozow> in `PaysMoreThanConflicts()`, we're making sure the replacement transaction fees >= direct conflicts fees. if it's not better, we can quit immediately. anybody have an idea why?
<sipa> glozow: well i'd consider dust accumulation in the mempool and p2p bandwidth/validation cost both DoS concerns, though very different ones admittedly
<glozow> In BIP125 Rule #4, what does it mean for a transaction to “pay for its own bandwidth?” Why don’t we just allow any replacement as long as it has a higher feerate?
<larryruane> dariusparvin: I think you're right -- we already relayed the original tx(es) and if we accept the replacement, we have to replay it too (?)
<glozow> dariusparvin: larryruane: yep. we'll be relaying the transaction. and we already relayed the original one(s). so it needs to pay for that second relay
<dariusparvin> i guess having rule #4 rather than just #3 is also a kind of DoS protection? So that the sender doesn't keep incrementing the replacement tx by 1 sat
<dunxen> we could just change the fees by 1 sat if it didn't pay for its own bandwidth right? that would mean we could replace a lot pretty cheaply and DoS?
<glozow> next question: In BIP125 Rule #5, how is it possible for a transaction to conflict with more than 100 mempool transactions, given that mempool ancestor and descendant limits don’t allow a chain of more than 25 transactions?
<larryruane> i think reference versus non-reference is often (not always!) a performance consideration.. we don't want to pass a 32-byte thing on the stack (copy), but a `CAmount` is just 8 bytes, usually the same size as a pointer (which is what a reference really is)
<glozow> larryruane: yes! we often want to pass by reference because (1) we want to mutate it in the function or (2) the size of the reference is smaller than the object itself, e.g. `CAmount`
<larryruane> specifying a pass-by-value (as in this case with the CAmount) as a `const` in the prototype actually is always wrong, because the caller doesn't care whether the called functions changes the variable *internally*
<larryruane> in pass-by-value, the called function gets a copy, and the scope of that copy is only within that called function (out of scope when the function returns)