Rename permitrbf to mempoolreplacement and provide minimal string-list forward compatibility (needs 0.12 backport) #7431

pull luke-jr wants to merge 4 commits into bitcoin:master from luke-jr:permitrbf_rename changing 3 files +17 −7

luke-jr commented at 5:29 AM on January 28, 2016: member

The purpose of supporting -replacebyfee=opt-in is so that future versions can also support things like -replacebyfee=fss or -replacebyfee=opt-in,fss (for First-Seen-Safe behaviour)

Rename permitrbf to replacebyfee

"permit" is currently used to configure transaction filtering, whereas replacement is more to do with the memory pool state than the transaction itself.

77b55a00ed

Accept replacebyfee=opt-in for turning on opt-in RBF

Basic forward-compatibility with more flexible parameters like fss

d65dee961e

luke-jr force-pushed on Jan 28, 2016
jonasschnelli commented at 7:48 AM on January 28, 2016: contributor

The forward compatibility looks good to me. It's a tiny bit speculative (I'm not aware of plans to change [distinct between multiple]) RBF options.

utACK
jonasschnelli added the label Mempool on Jan 28, 2016
laanwj commented at 11:42 AM on January 28, 2016: member

(I'm not aware of plans to change [distinct between multiple]) RBF options.

There are no such plans, but @luke-jr wants to support other kinds of RBF in his own fork. No strong opinion either for or against this. Code looks ok to me.
MarcoFalke commented at 12:08 PM on January 28, 2016: member

How would this work? The release notes mention -permitrbf and you removed that code entirely.
jonasschnelli commented at 12:18 PM on January 28, 2016: contributor

@MarcoFalke: Right. The release notes need an update – IF – we would merge this. IIUTC: The forward compatibility means, that of someone throws in string arg like -replacebyfee=foo,bar,opt-in it would result in enabling opt-in-RBF.

in src/init.cpp:None in d65dee961e outdated

1038 | +    if ((!fEnableReplacement) && mapArgs.count("-replacebyfee")) {
1039 | +        // Minimal effort at forwards compatibility
1040 | +        std::string strReplacementModeList = GetArg("-replacebyfee", "");  // default is impossible
1041 | +        std::vector<std::string> vstrReplacementModes;
1042 | +        boost::split(vstrReplacementModes, strReplacementModeList, boost::is_any_of(","));
1043 | +        BOOST_FOREACH(const std::string& strReplacementMode, vstrReplacementModes) {

jonasschnelli commented at 12:19 PM on January 28, 2016:

fEnableReplacement = (std::find(vstrReplacementModes.begin(), vstrReplacementModes.end(), "opt-in") != v.end()) (and avoid the foreach)?

laanwj commented at 12:25 PM on January 28, 2016:

Yes that should be the same, as this is in if(!fEnableReplacement) so it can never be set to false by this.

MarcoFalke commented at 1:40 PM on January 28, 2016: member

– IF – we would merge this.

It should be part of this pull; Otherwise we'd end up in an inconsistent state. Also adding it to the blacklist may be worth to consider as the option is already in use and promoted/posted.
luke-jr commented at 4:24 PM on January 28, 2016: member

@jonasschnelli RBF-FSS at present seems more popular than opt-in, so I would not be surprised if someone implemented it for 0.13. @MarcoFalke This is the PR against master, so it cannot include changes to 0.12 release notes.
Simplify check for replacebyfee=opt-in 3b66e54457
luke-jr commented at 1:29 AM on January 29, 2016: member

Added @jonasschnelli 's suggestion.
laanwj commented at 11:22 AM on February 1, 2016: member

Suggestion by @gmaxwell on IRC: Though if it's pluggable policy for luke I wonder why he didn't call it -mempoolreplace

I think that makes sense. If this selects a "mempool replacement policy", by fee is just one option.
Rename replacebyfee=opt-in to mempoolreplacement=fee b922fbe063
luke-jr commented at 7:33 PM on February 1, 2016: member

Good idea, done.
luke-jr renamed this:
~~Rename permitrbf to replacebyfee and provide minimal string-list forward compatibility (needs 0.12 backport)~~
Rename permitrbf to mempoolreplacement and provide minimal string-list forward compatibility (needs 0.12 backport)
on Feb 1, 2016

in src/init.cpp:None in b922fbe063

1038 | +    if ((!fEnableReplacement) && mapArgs.count("-mempoolreplacement")) {
1039 | +        // Minimal effort at forwards compatibility
1040 | +        std::string strReplacementModeList = GetArg("-mempoolreplacement", "");  // default is impossible
1041 | +        std::vector<std::string> vstrReplacementModes;
1042 | +        boost::split(vstrReplacementModes, strReplacementModeList, boost::is_any_of(","));
1043 | +        fEnableReplacement = (std::find(vstrReplacementModes.begin(), vstrReplacementModes.end(), "fee") != vstrReplacementModes.end());

laanwj commented at 11:41 AM on February 2, 2016:

I'm not sure 'fee' is a good name. How would you then call non-opt in RBF?

A straightforward name for the strategy would be opt-in-rbf or opt-in-fee

laanwj merged this on Feb 3, 2016
laanwj closed this on Feb 3, 2016
laanwj referenced this in commit 5fd95b4490 on Feb 3, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#7440 [0.12] Rename permitrbf to mempoolreplacement and provide minimal string-list forward compatibility