Enable mempoolfullrbf=1 by default
#26305
Conversation
|
Concept ACK Thanks for going to the trouble of writing #25600 But this is definitely the simpler approach. |
|
Concept NACK There is no need to make it default at this point. Details: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021010.html |
|
Releasing v0.24 with the |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
Concept ACK, Approach ACK for reasons stated here. There is a release cycle conversation (24.0 or 25.0) to be had but perhaps that can be done in an issue linked to this PR rather than on this PR. (I am assuming this goes ahead and the default is changed. I believe it should be and it seems many others do. But of course if the default isn't changed these PRs and any associated issues are mute) |
|
Concept ACK for 25.0 |
| static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336}; | ||
| /** Default for -mempoolfullrbf, if the transaction replaceability signaling is ignored */ | ||
| static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{false}; | ||
| static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{true}; |
There was a problem hiding this comment.
I wonder how much value there is in keeping the setting. If the default is true, a single miner turning it off (or even a substantial percentage of hashrate) isn't going to prevent fullrbf on the network. All it does is that the miner will be missing out on fees and have their compact blocks propagate slower. Offering the setting to a non-mining node will also slow their compact blocks (which shouldn't hurt), but it just seems like a false promise that they could achieve anything by toggling it.
So my preference would be to just remove the setting here.
There was a problem hiding this comment.
If or when we can drop the -mempoolfullrbf config arg, it seems that would allow a number of further simplifications in our codebase as well.
There was a problem hiding this comment.
I wonder if there is still value for 0conf operators who would like to implement first-seen transactions among a set of restricted/trusted mempools: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/021011.html I have no idea if such scheme, if implemented would be robust though I think it's good to offer mechanism flexibility (If no one leveraged it after few releases we can still deprecate it ofc)
mzumsande
left a comment
There was a problem hiding this comment.
Concept ACK
CI fails, so functional tests still need a fix
|
Note to the reviewers see #26323, which is an alternative deployment approach achieving the same effect, while committing to a flag day activation, as suggested by the opening description of this PR. Personally, I'm fine with both approaches, though diverging on the activation parameters and merging timeline of #26323. |
|
I think you can replace this PR with a cherry-pick of the top commit of #26323 to fix the CI errors (well, changing the commit description would be smart). (EDIT: or at least, I think it does now) |
|
Concept ACK |
|
concept ACK, provided we collect data on which release this should make it into to allow businesses to transition. |
|
I'm in favour of switching the default rather than doing preferential peering. However i'm unsure about shipping it into 25.0. Ideally, i'd prefer if we only adapted the default to the behaviour of the network. That is, to only switch to |
Considering the technical complexity of 0confs services/applications discussed on the mailing list, I think too 25.0 is likely to be too early. A 12/18 timeframe might be more adequate. Though ideally, we should still reach out to more 0confs service operators to collect more feedback. |
|
Closing this PR in the lack of clear consensus on |
This is an alternative to #25600 in the deployment of full-rbf. After listening to feedback, rather to introduce a new service bit and automatic preferential peering to enable full-rbf transaction-relay paths among the interested peers, the default value of
mempoolfullrbfis switched to 1. This change implies all the Bitcoin Core nodes will accept replacement of opt-out transactions at the next release (at the earliest 25.0). Rather than a gradual construction of full-rbf transaction-relay paths due to node operators activatingmempoolfullrbf, turning on by default the setting offer more visibility to the zero-conf services node operators.Mailing list post coming soon to seek what has the preference between this deployment approach and the smoother #25600 approach among the development community and zero-conf services node operators.
(One more approach could be to lock-in
mempoolfullrbf=1with some flag day activation if one release cycle of time is gauged is not enough or not predictible enough)