Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase mempool expiry time to 2 weeks #9312

Merged
merged 1 commit into from Jan 5, 2017
Merged

Conversation

@morcos
Copy link
Member

morcos commented Dec 9, 2016

As discussed in the weekly dev meeting, I think it makes sense to increase the mempool expiry time.
https://botbot.me/freenode/bitcoin-core-dev/2016-12-08/?msg=77683289&page=4

I propose 2 weeks.

3 days (the old time) was already not sufficiently short to protect against an attack that could fill the mempool with high fee rate txs that were somehow not attractive or possible to mine. A longer expiry time will reduce network traffic by less rerelay of low fee txs and will allow transactions to take advantage of weekly cycles in tx volume. By keeping the txs in the mempool, future revisions of fee estimation will be able to provide lower estimates for transactions which are low priority and can wait days or a week to be included in a block.

I've been running nodes with this code for many months.

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented Dec 9, 2016

Nodes which listen frequently have old transactions blasted back at them by "helpful" parties, so anyone counting on that expiration for something already can't really count on it.

Concept ACK.

@paveljanik

This comment has been minimized.

Copy link
Contributor

paveljanik commented Dec 10, 2016

Concept ACK

@rebroad

This comment has been minimized.

Copy link
Contributor

rebroad commented Dec 10, 2016

seems to me like a fix in the wrong place - surely the algorithm for keeping txs should be the same (or at least) similar to the algorithm used by miners for which txs to include in blocks.

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented Dec 10, 2016

@rebroad They are the same.

@rebroad

This comment has been minimized.

Copy link
Contributor

rebroad commented Dec 12, 2016

Why expire them at all?

@morcos

This comment has been minimized.

Copy link
Member Author

morcos commented Dec 12, 2016

@rebroad I can think of 2 reasons to leave an expiry time.

  1. If we do end up in a situation where mining policy doesn't match our mempool policy, then at least we will eventually clear transactions that aren't getting mined. Neither 3 days nor 2 weeks are sufficiently aggressive to resist an attack, but probably both still help in the case of accidental network misbehavior.

  2. It might make sense in the future to separately treat our own wallet transactions that are expired after 2 weeks. They become good candidates for fee bumping or some other type of reuse. It seems to me just abandoning them is probably unsafe advice, but perhaps we'd have a feature which respends the outputs back to ourselves if the original tx is no longer desired. For now we will rebroadcast them, but hopefully this is rare.

@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 14, 2016

NACK. This new policy will make it more cumbersome for the average user to transact, and will push the network towards the toxic RBF policy.

@morcos

This comment has been minimized.

Copy link
Member Author

morcos commented Dec 15, 2016

@sandakersmann Can you explain how it will make it more cumbersome for the average user to transact? Keep in mind that transactions that are expired from your mempool are automatically reaccepted when you restart your node in versions until 0.13.1 and every 20 mins in 0.13.2+.

@dcousens

This comment has been minimized.

Copy link
Contributor

dcousens commented Dec 15, 2016

Keep in mind that transactions that are expired from your mempool are automatically reaccepted when you restart

What mechanism are you referring to here out of interest?

@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 15, 2016

@morcos When your transaction is stuck in limbo it is better that it is only stuck for 3 days and not 14.

@morcos

This comment has been minimized.

Copy link
Member Author

morcos commented Dec 15, 2016

@dcousens I should have said your wallet transactions. ReacceptWalletTransactions is called on startup and after #9290 RelayWalletTransactions also tries to reaccept the transactions to the mempool and runs every 20 mins.

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Dec 16, 2016

@sandakersmann

@morcos When your transaction is stuck in limbo it is better that it is only stuck for 3 days and not 14.

When a transaction is stuck in limbo, it is usually due to too low fees. In that case, it will have been evicted from the mempool already, and the timeout doesn't affect it at all. The reason the timeout exists is for robustness against unexpected events such as unknown new network rules, that result in apparently valid but high fee transaction not getting confirmed.

@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 16, 2016

@sipa Valid and medium/low fee transactions are some times not getting confirmed within 3 days, due to new transactions constantly paying a higher fee. The congestion is real and makes transactions unreliable.

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Dec 16, 2016

@sandakersmann I am aware. But this PR will not affect those; they're already evicted from the mempool for other reasons.

@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 17, 2016

@sipa What reasons do you refer to? I'm talking about transactions with sufficient min relay fee and sufficient fee to stay inside the default max mempool size.

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Dec 17, 2016

@sandakersmann In that case they're already subject to rebroadcasts.

@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 17, 2016

@sipa True if you use Core as your wallet, but most users don't.

@morcos

This comment has been minimized.

Copy link
Member Author

morcos commented Dec 17, 2016

@sandakersmann This conversation has gone a bit off the rails for a PR. This is a configurable command line option. Always has been. It makes sense for us to ship it with a default that we think is best. Anybody that disagrees is welcome to change the setting on their node, it will still interoperate perfectly well with the rest of the network. As already mentioned:

  • There is no practical change for Core wallet users.
  • For wallet users in general, it is my belief that they'd rather have placed transactions make it into a block after some delay rather than not at all, and in many cases there is also no practical difference for them either because of other people rebroadcasting transactions.
  • Finally and most importantly, having the transactions stay consistently in the mempool will make it easier to properly track how long they are taking to be included in a block and provide more reliable fee estimates at lower fees. This provides an important feedback effect if even only a few people take care of it because it allows fee estimation to detect when those lower feerates are being included in a block relatively quickly and can cause everyone to start paying lower fees.
@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 17, 2016

@morcos Fine. You guys can and will do whatever you want. Soon there will be no users of Bitcoin the way you're going. Maybe Luke-Jr gets his wish and we have < 500 kB blocks for next Christmas.

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented Dec 18, 2016

@sandakersmann The accusatory tone you're using is not constructive, please try a different approach in the future.

I think if you take the time to read the comments here with an open mind you'll see that your concerns had already been answered: Third parties already frequently re-transmit other people's transactions which degrades the effectiveness of the expiration, this cannot be prevented. Moreover, due to full replacing miners, nodes restarting, and the transactions being evicted for having fees too low in practice I've found that replacements of non-RBF transactions work long before the expiration. So they'll work after this but can't be counted on even already due to the frequent third party rebroadcasts.

@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 19, 2016

@gmaxwell So since you already have merged toxic stuff that frequently re-transmit transactions, it should not be an issue to merge this also? This crippled RBF coin will have no future.

@sipa

This comment has been minimized.

Copy link
Member

sipa commented Dec 19, 2016

@btcdrak

This comment has been minimized.

Copy link
Member

btcdrak commented Dec 19, 2016

utACK

@sandakersmann

This comment has been minimized.

Copy link
Contributor

sandakersmann commented Dec 19, 2016

@sipa Was mainly referring to #9290

@MarcoFalke

This comment has been minimized.

Copy link
Member

MarcoFalke commented Dec 19, 2016

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented Dec 20, 2016

@sandakersmann Bitcoin has always rebroadcast transactions.

#9290 is about retrying mempool insertion more frequently for mempool rejected transactions, which themselves have little expiration interaction because they weren't going mempools in the first place! ... it's mostly related to long chains of unconfirmed transactions, not low fees or such... and it's necessary for to achieve broadcast not rebroadcast.

@gmaxwell

This comment has been minimized.

Copy link
Contributor

gmaxwell commented Jan 4, 2017

ACK.

@TheBlueMatt

This comment has been minimized.

Copy link
Contributor

TheBlueMatt commented Jan 5, 2017

utACK

@laanwj laanwj merged commit 5f0e27f into bitcoin:master Jan 5, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
laanwj added a commit that referenced this pull request Jan 5, 2017
5f0e27f Increase mempool expiry time to 2 weeks (Alex Morcos)
@bimmerdriver

This comment has been minimized.

Copy link

bimmerdriver commented May 24, 2017

I'm not a bitcoin developer or expert, but rather a regular periodic bitcoin user impacted by the recent glut of unconfirmed transactions caused by load and/or insufficient fees. My perspective as such a user is that keeping unconfirmed transactions in the mempool for two weeks is not helpful. I've learned the errors of my ways (and not to trust blockchain.info "dynamic" fee recommendations), but I have transactions (including double-spends) with no prospect of being confirmed stuck in the mempool. The sooner they get discarded the better. Alternatively, make two weeks the maximum and allow users to specify a shorter lifetime.

@jnewbery

This comment has been minimized.

Copy link
Member

jnewbery commented May 24, 2017

@bimmerdriver - users can specify the -mempoolexpiry option if they want a different expiry time for the local mempool.

I don't think this is actually what you need though. You're asking about your own transactions being stranded because of insufficient fee. There are a couple of ways to resolve that - Child Pays For Parent (send a transaction with a large fee which relies on your transaction) or Replace Be Fee. You should be able to find information on both of those if you search. I suggest you use https://bitcoin.stackexchange.com/ if you still need help.

@bimmerdriver

This comment has been minimized.

Copy link

bimmerdriver commented May 25, 2017

@jnewbery - Thanks for your reply. I guess I don't see what the benefit to anyone is for transactions that have no hope of being confirmed cluttering up the mempool (and the respective wallet) for two weeks. If they aren't going to get confirmed within a reasonable amount of time, they should go away. I wish I could use RBF or CPFP, but unfortunately, blockchain.info doesn't support it. As soon as the unconfirmed transactions go away, I will move to another wallet.

@sipa

This comment has been minimized.

Copy link
Member

sipa commented May 25, 2017

The reason for increasing the timeout is because there are transactions that confirm over such timeframes. There is typically less load on weekends, allowing a number of transactions from the week(s) before to still confirm. Also, the timeout generally has very little effect, as wallets can always rebroadcast to get another timeout period.

@pstratem

This comment has been minimized.

Copy link
Contributor

pstratem commented May 25, 2017

@bimmerdriver Transactions do not expire. There are nodes which rebroadcast transactions and have no expiration limit on mempool transactions.

You must invalidate a transaction before assuming it will not be included in a block.

codablock added a commit to codablock/dash that referenced this pull request Jan 18, 2018
5f0e27f Increase mempool expiry time to 2 weeks (Alex Morcos)
andvgal added a commit to energicryptocurrency/energi that referenced this pull request Jan 6, 2019
5f0e27f Increase mempool expiry time to 2 weeks (Alex Morcos)
CryptoCentric added a commit to absolute-community/absolute that referenced this pull request Feb 26, 2019
5f0e27f Increase mempool expiry time to 2 weeks (Alex Morcos)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.