However, I would note this patch is requesting config action from the node operators to keep the default behavior running.

@ariard This isn't true...

I think there will be an additional communication work with this patch towards users to explain they have to think more now about their node settings...

Users should already think about that. This PR doesn't change that.

Though if I should make a comment on the naming itself, mempoolreplacement is confusing to me as firstly there is no false option.

There is. Just not supported by this version of Core.

Secondly referring to the fee partial argument is redundant as it's already implied by RBF

RBF is not the only possible replacement policy.

and thirdly the optin versus -optin is more likely to bewilder the operators and lead them to misconfigure their nodes.

?

And lastly, after this PR, they do have to take action by setting -mempoolreplacement="fee,optin" to keep the previous node behavior running,

No. fee,optin remains the default.

@ariard This isn't true...

Right I withdraw what I said this patch does not request additional config action from the node operators. Because when -mempoolreplacement is true it implies RBF opt-out is enforced (a replacement policy). However when -mempoolreplacement is false it implies full-RBF (also a replacement policy). I think the patch is confusing by not using the constant DEFAULT_REPLACEMENT_HONOUR_OPTOUT here : https://github.com/bitcoin/bitcoin/pull/25373/files#diff-b1e19192258d83199d8adaa5ac31f067af98f63554bfdd679bd8e8073815e69dR1045...

Anyway, I reiterate I don't care which patch gets in and I'll let other reviewers express which approach they prefer. I won't comment further. If it's just a config naming issue after all, it's not worth my energy to argue on 🤷

Since we're not adding never-replace support to Bitcoin Core, I think the approach in #25353 of just having a simple -fullrbf option is much simpler for users to understand.

Of course, I'd still say the better option is to just remove opt-in RBF entirely rather than having the intermediate step of an option.

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #25522 (test: Remove -acceptnonstdtxn=1 from feature_rbf.py by MarcoFalke)
• #25353 (Add a -mempoolfullrbf node setting by ariard)
• #25302 (build: Check usages of #if defined(...) by brokenprogrammer)
• #25193 (indexes: Read the locator's top block during init, allow interaction with reindex-chainstate by mzumsande)

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. Haven't assessed which I prefer on #25353 or this but will probably be a weak preference (if any).

Thanks for opening this anyway, wasn't aware that Knots had been supporting this since 2016

Whether this one or #25353, i think we ought to give users the possibility to run an incentive-compatible policy.

Note that RBF is not incentive-based for nodes, only for miners. Nodes do not benefit whatsoever from transactions (other than their own) being replacable.

Since we're not adding never-replace support to Bitcoin Core, I think the approach in #25353 of just having a simple -fullrbf option is much simpler for users to understand.

Other implementations (including both old Bitcoin Core versions and current Knots versions) already use -mempoolreplacement. Using a new option makes no sense except to create compatibility issues.

They do? Compact block relay, fee estimation, etc.. As a node you very much want to have your mempool be as close as possible to miners' ones. And the best guess you have to achieve that is to assume they are incentivized by fees internal to transactions and run a miner-incentive-compatible mempool policy.

This is backward. Miners have an incentive to use policies matching nodes, not vice-versa.

Other implementations (including both old Bitcoin Core versions and current Knots versions) already use -mempoolreplacement. Using a new option makes no sense except to create compatibility issues.

We don't support long EOL versions of Bitcoin Core. And for the compatibility concerns, I don't see why a three line patch in Knots to support a setting alias is a strong reason for us to pick one thing or the other. This is no different from being able to set the chain via -regtest or -chain=regtest, which are aliases for each other.

Personally I prefer #25353 because the name of the setting and the value is easier for users to understand, but I don't really care too much.

Also, for disabling replacements completely: It is already possible today to disable it in Bitcoin Core and Knots in a fully compatible way by simply setting the incremental relay fee to infinity.

Hard NACK. This is sliding further down the slope of RBF-only on by default, and eventually prevents normal original usage of Bitcoin, and creates further uncertainty and optionality for merchants to accept 0-conf payments within their own risk tolerance. I appreciate the off-by-default designation, but generally RBF is not a popularly used feature (despite some apps having it on by default already) and does not deserve constant creeping into a cultural norm.

@BitcoinErrorLog

If you do have substantial concerns about this PR or the sister PR (#25353), both proposing a full RBF setting in Bitcoin Core while differing in their implementations, I would invite you to express them on this mail list thread : https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-June/020557.html. To give wider publicity for the conversation rather than lost in GH, I'll follow-up there.

I fully understand the uncertainty and optionality for 0-conf merchants of the existence of a full-rbf topology on the p2p network. However, the rational behind my proposal isn't motivated by making full-RBF a cultural norm or whatever but to solve a really tangible and naive DoS issue affecting the funding flow of p2p coinjoin, on-chain DLCs, dual-funded channels, etc. I think it's always interesting as a community to have a conversation on how we should approach the design and deployment of policy in case of conflicting use-cases (i,e "X is made worst by making better Y").

Note, RBF is also a fundamental piece of Lightning security model w.r.t time-sensitive transactions [0]

[0] https://lists.linuxfoundation.org/pipermail/lightning-dev/2018-November/001697.html

This is sliding further down the slope of RBF-only on by default

I would NACK it when its proposed in future as default not sure about others.

and eventually prevents normal original usage of Bitcoin, and creates further uncertainty and optionality for merchants to accept 0-conf payments within their own risk tolerance.

Original bitcoin had lot of issues that have been fixed in last years. Zero confirmation payment accepted as final is a gamble and protocol cannot prohibit people from gambling. They can do it differently when 2 RBF policies exist.

I appreciate the off-by-default designation, but generally RBF is not a popularly used feature (despite some apps having it on by default already) and does not deserve constant creeping into a cultural norm.

Even I ACKed it because its not default and agree RBF signalling, replacements etc. are less used. @ariard claims this would increase once we have full-rbf and a few things make sense. Not everything though however people are free to use different RBF policies. There is no cultural norm. Users can already try any RBF policy and knots provides some options including full-rbf.

Full RBF could fix some issues including vulnerable LN, DLC and coinjoin although these are not consensus rules and users, miners, etc. (including attackers) can use any policy. Or some of these projects adjust to multiple RBF policies in future.

Finally NACKing this PR is useless, should either NACK #25353 or share your opinions on mailing list as suggested by @ariard

Rebased

I'm going to close this for now, as it's fairly clear (given the number of ACKs and Concept ACKs) that if Core is going ahead with a full RBF setting, it's going to be happen via #25353. This PR could always be re-opened in future if that PR doesn't end up being merged, however I don't think we need to keep a competing PR open at this point.