Remove mempool expiry, treat txs as replaceable instead

MarcoFalke commented at 11:03 pm on July 17, 2019: member

There is no reason for tx expiry in the tx pool, as wallets occasionally rebroadcast txs when they are not confirmed. Thus they will only reset the timer and throw away their chance bumping the fee.

Fix that by removing expiry (so that miners don’t miss out on long-term fee revenue if the tx stays the same), but treat all txs older than 2 weeks as replaceable in the mempool to give wallets the chance to increase the fee.

Remove mempool expiry fa014de63e

Make txs replacable after 2 weeks fa1638d8fd

MarcoFalke commented at 11:04 pm on July 17, 2019: member

Will write:

doc
rpc changes
tests

if this gets more than zero Concept ACKs

fanquake added the label Mempool on Jul 17, 2019

sipa commented at 11:39 pm on July 17, 2019: member

There is a nontrivial policy change here (I think; haven’t checked the code) which may or may not be desirable: replacing a transaction is subject to the “replacement pays marginal extra fee for relay of now non-confirming old transaction”, while clearly new submissions after expiry are not subject to this.

In other words, replacement isn’t guaranteed to converge to the highest fee-paying state in the long term, as the optimal state may be so close to a suboptimal earlier one that replacement isn’t allowed. However, this isn’t an issue right now as expiry will eventually remove non-comfirming things, allowing them to be replaced for free. With this change, that is no longer the case.

An alternative would be to after some age, drop this minimum marginal rate rule, or even making it decay over time.

DrahtBot commented at 0:31 am on July 18, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16421 (Conservatively accept RBF bumps bumping one tx at the package limits by TheBlueMatt)
#16401 (Package relay by sdaftuar)
#16400 ([refactor] Rewrite AcceptToMemoryPoolWorker() using smaller parts by sdaftuar)
#16398 (rpc: testmempoolaccept for list of transactions by MarcoFalke)
#15921 (Tidy up ValidationState interface by jnewbery)
#15681 ([mempool] Allow one extra single-ancestor transaction per package by TheBlueMatt)
#15192 (Add missing cs_main locks in ThreadImport(…)/Shutdown(…)/gettxoutsetinfo(…)/InitScriptExecutionCache(). Add missing locking annotations. by practicalswift)
#10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake added the label Needs Conceptual Review on Jul 18, 2019

laanwj added the label TX fees and policy on Jul 18, 2019

sdaftuar commented at 1:34 pm on July 18, 2019: member

I think there’s another reason to have mempool expiry – if there is a policy change adopted by miners in the future, then software that doesn’t enforce the policy rule and that lack any kind of mempool expiry might keep transactions violating the rule in their mempools forever.

MarcoFalke commented at 1:52 pm on July 18, 2019: member

In other words, replacement isn’t guaranteed to converge to the highest fee-paying state in the long term, as the optimal state may be so close to a suboptimal earlier one that replacement isn’t allowed

I think you are focussing too much on a currently theoretical use case that is not applicable in today’s network. And assuming that miners are running with mempool expiry to support your use case, they have no way to protect against the case where they are sent a replacement transaction that has less fees. (Expiry will completely erase the tx and any feerates associated with it)

If you look at use cases that have been observable on the current network, my change seems like a clear improvement: Assume that a tx hasn’t confirmed after 2 weeks, so the original wallet has a few options:

1. No rebroadcast to “cancel the tx” (https://duckduckgo.com/?q=bitcoin+cancel+tx)
1. Rebroadcast to explicitly keep it alive
1. Broadcast a replacement tx with: a) less fee b) marginally more fee (your use case) c) more fee

With my proposed changes:

1. Miners have no incentive to throw away income, so this gives a false sense of security and my changes would make it explicit that the tx is still alive.
1. Rebroadcasting might hurt privacy and imposes a cost on the network, so might as well keep the tx in the first place (as is done with my changes)
1. . a) Again, miners don’t have an incentive to throw away income, so this is fragile. My changes would prevent it from happening b) Imo a theoretical use case. If miners are willing to accept marginally more fee they can reduce the -minincrementalfee. And I think a fee decay should be done in a follow up pull request. Though, I am happy to “drop this minimum marginal rate rule” as suggested by you, if others think this is helpful. c) Bad actors on the network can starve propagation of those txs by just flooding out the original tx. Again, my change would prevent that from happening.

sdaftuar commented at 2:01 pm on July 18, 2019: member

@marcofalke Regarding the fee difference between expiry and marking something replaceable – if a small, low-feerate transaction is RBF-pinned with a large, low-feerate descendant (for instance, created by someone else), then with this proposal, the original transaction author has no way to RBF the transaction without spending a lot of fee, correct?

While expiry doesn’t guarantee that the original transaction author will be able to replace it with a nominally higher feerate in the future (ie without having to pay for all descendants), my guess would be that this should generally work.

MarcoFalke commented at 2:01 pm on July 18, 2019: member

might keep transactions violating the rule in their mempools forever

Expiry does not protect against that, as anyone can connect to you once every two weeks and re-send you the txs you just dropped. Also, if you are running an EOL release of Bitcoin Core, you are encouraged to only connect it to trusted peers that run a recent version of Bitcoin Core.

MarcoFalke commented at 2:03 pm on July 18, 2019: member

then with this proposal, the original transaction author has no way to RBF the transaction without spending a lot of fee, correct?

Yes

jnewbery commented at 7:36 pm on July 18, 2019: member

@sdaftuar has convinced me. Mempool expiry is a useful feature, if only to protect old nodes from having their mempools permanently filled with transactions that are fail policy for newer nodes. I don’t see that the benefit from this PR outweighs the potential risk of old nodes having their mempools filled with effectively unconfirmable txs.

Concept NACK.

DrahtBot added the label Needs rebase on Jul 19, 2019

DrahtBot commented at 6:07 pm on July 19, 2019: member

MarcoFalke commented at 10:57 pm on July 19, 2019: member

Thanks for the input everyone. I will close this for now based on @sdaftuar’s and @jnewbery’s feedback.

MarcoFalke closed this on Jul 19, 2019

MarcoFalke deleted the branch on Jul 19, 2019

laanwj removed the label Needs rebase on Oct 24, 2019

DrahtBot locked this on Dec 16, 2021

Remove mempool expiry, treat txs as replaceable instead #16409

Conflicts