Scheduled full-RBF deployment

petertodd commented at 4:46 am on June 29, 2015: contributor

Prior to Tuesday April 5th, 2016, at 3pm UTC, this pull-req implements first-seen-safe replace-by-fee; after the switchover date the first-seen-safe restrictions automatically turn off, resulting in full-RBF behavior. On testnet/regtest full-RBF is always supported.

The implementation is from my full-RBF patch series, specifically https://github.com/petertodd/bitcoin/tree/replace-by-fee-v0.11.0rc2, with additional logic and tests from #6176

BIP: https://github.com/petertodd/bips/blob/bip-full-rbf-deadline/bip-full-rbf-deadline.mediawiki

Credit goes to @jgarzik for suggesting a scheduled-in-advance deployment strategy.

Add replace-by-fee logic to the mempool

Replaces transactions already in the mempool if a new transaction is
seen with a higher fee, specifically both a higher fee per KB and a
higher absolute fee. Children are evaluated for replacement as well,
using a fixed depth/breadth limit to prevent DoS attacks.

Includes stand-alone unittests for regtest in qa/replace-by-fee/

32008e8219

petertodd force-pushed on Jun 29, 2015

luke-jr commented at 5:37 am on June 29, 2015: member

NACK treating policy as a software development decision. Make it a runtime option if you want.

petertodd commented at 5:40 am on June 29, 2015: contributor

@luke-jr It is a runtime option, with a default suggested setting: https://github.com/bitcoin/bitcoin/pull/6352/files#diff-c865a8939105e6350a50af02766291b7R866

luke-jr commented at 5:42 am on June 29, 2015: member

@petertodd Not the point.

petertodd commented at 6:02 am on June 29, 2015: contributor

@luke-jr As discussed on IRC, we have practical reasons to want somewhat common mempool policies for the sake of small miners who (currently) have no other way of getting transactions. In the future that may change, but for now we’re stuck with defining at least some broad notion of what miner policy might be.

Anyway, not adopting full-RBF is also a policy decision.

jtimon commented at 9:32 am on June 29, 2015: contributor

My preference would be to support multiple policies simultaneously (ie standard policy, testing policy, standard + full-RBF policy, etc). That would require something like #5180 first. That would clarify the notion that the standard policy is not part of the consensus rules (as well as providing more flexibility without having to necessarily expose a zillion “standard policy runtime options” like we’re doing now). That doesn’t mean that full-RBF can’t become part of the standard policy at some point. I would be in favor of that. I would also be in favor to make FSS-RBF part of the standard policy as soon as possible. Supporting multiple policies would also remove the need for independently maintaining advanced policies patches (like you’re currently doing) even if it’s decided not to make it the standard policy until a later date. At that point, you may still want to offer first seen and FSS-RBF as alternative policies that people who don’t like full-RBF can run.

petertodd commented at 0:21 am on June 30, 2015: contributor

@jtimon Yeah, from a code point of view I also support making policy modular, but keep in mind the point of this pull-req is to set a date for planning purposes so the remaining vulnerable companies can migrate to less harmful, less dangerous, zeroconf technology in a way that minimizes losses and disruption; the situation now is such that for the vulnerable users even a small % of miners adopting full-RBF, or indeed, any change to mempool policy at all, is disruptive. For instance, even something as simple as miners increasing the minimum relay on their own nodes can be easily exploited for double-spends; a few do right now, which makes accepting zeroconf transactions with lowish fees very dangerous.

In short, I think you can make a good argument that this patch is actually a prerequisite to modular mempool policy.

FWIW the patch does include a way to disable full-RBF indefinitely if the user chooses too by setting -fullrbfactivationtime=2147483647 Should I document this command-line argument and provide a more ergonomic way of doing it, like setting the activation time to -1 or 0 to disable? Heck, could call it -fullrbf with =0 being disabled, =1 enabled, and = to set the switchover time manually.

btcdrak commented at 0:53 am on June 30, 2015: contributor

@petertodd I was going to suggest -1 to turn off fullrbf.

Restrict RBF with first-seen-safe rules until switchover date

Credit goes to Jeff Garzik for suggesting a scheduled switchover
deployment strategy.

8821579b08

petertodd force-pushed on Jun 30, 2015

petertodd commented at 1:23 am on June 30, 2015: contributor

Documented -fullrbfactivationtime option, and changed it so -fullrbfactivationtime=0 disables full-RBF entirely to match ergonomics of other options.

mikehearn commented at 9:38 am on June 30, 2015: contributor

As a reminder, this ridiculous and highly controversial idea has been beaten to death elsewhere over and over again. Gavin, Jeff, myself, Adam Back and Coinbase have all explicitly stated opposition to it due to it being driven by fundamentally faulty logic. The counterargument is summed up here

https://medium.com/@octskyward/replace-by-fee-43edd9a1dd6d

jtimon commented at 10:50 am on June 30, 2015: contributor

@mikehearn yes, it is clear that this is controversial. There’s even people who like this but think that deploying it right now would be too risky for some businesses or want to give more time to instantly secure payments (possible with some payment channels like lightning channels). This is certainly not ridiculous though, and that claim is not constructive.

Fortunately this is just node policy and not consensus rules. That means we can support multiple configurable policies simultaneously without causing consensus forks. Other implementations can of course maintain their own policies (a single one or several of them, that’s up to their maintainers). @petertodd travis is not happy

I don’t like a switch-over date here. Policy doesn’t require that kind of coordination. If full-RBF is going to be the default policy, just make it so for a given major release and that’s it. In the meantime I think we can just merged disabled by default. Or my preference, wait until we can select different policies #5180 (which is easy to review after #6335 and trivial to review after #6068) to merge this. And then just change the default at any later major release.

petertodd commented at 11:57 am on June 30, 2015: contributor

@jtimon “Policy doesn’t require that kind of coordination.”

It does though! Unfortunately we have the problem that there’s no way to get transactions to all but the largest, most centralized, miners other than via the p2p network. Without some pretty major changes to how that network works (e.g. hashcash) any transactions that aren’t propagated by the majority of relay nodes just aren’t going anywhere - why I had to define a RBF service bit and include a bunch of hacky preferential peering code in my full-RBF patch series.

One way to look at full-RBF is it’s a common-denominator policy that supports other policies; full-RBF provides opportunities to get a larger range of transactions to miners who in turn may have their own policies in a DoS-resistant way by the simple measure of requiring fees to increase on each replacement, and it’s significantly more efficient at doing that FSS-RBF. From a mining policy point of view, because it allows transactions to propagate that previously wouldn’t have, it reduces the impact of the default policy on what miners can mine and what users can generate. Of course, from a code point of view I’d be happy to port it later to a pluggable policy system, but in the meantime, the code in this pull-req is well-tested and well-reviewed.

Anyway, re: a switch-over date, in many ways that’s us, as Bitcoin Core maintainers, saying “by this date stop assuming everyone has the same mempool policy” - the opposite approach of first-seen-dependent zeroconf requires nodes/miners to implement uniform policies to even begin to work. You are of course free to turn full-RBF off and I’ve made it easy to do so.

re: travis, yeah, looks like it’s broken again; other pull-reqs are failing too.

jtimon commented at 12:14 pm on June 30, 2015: contributor

re: travis, yeah, looks like it’s broken again; other pull-reqs are failing too.

Sure, just a reminder. Similar to rebase reminders…

Anyway, re: a switch-over date, in many ways that’s us, as Bitcoin Core maintainers, saying “by this date stop assuming everyone has the same mempool policy”

Right, so why not just “from this major release, stop assuming everyone else will have the same mempool policy” instead?

petertodd commented at 12:37 pm on June 30, 2015: contributor

Right, so why not just “from this major release, stop assuming everyone else will have the same mempool policy” instead?

I think there’s an important psychological component to actually having a date for people to have in their heads - it’s a clear statement that we’re giving the payment processing companies affected by the change 9 months (max) to prepare, but we’re not going to delay longer for their benefit to the detriment of all other Bitcoin users. (where “we’re” refers to miners to choose to take the advice of the Bitcoin Core maintainers and leave the default settings as-is)

An alternative would be to enable full-RBF for all relay nodes, and force miners to make a choice. (e.g. by disabling getblocktemplate until some kind of config setting was enabled) Ugly from a usability point of view, but doable. There’s also a risk of DoS attack by doing that, as if very few miners support full-RBF you can fill up mempools/use-up-bandwidth with transactions that are unlikely to be mined. (same problem with double-spend relaying)

gavinandresen commented at 1:02 pm on June 30, 2015: contributor

NACK on the code changes. Asking people to pull in somebody’s personal repo to test code is a bad idea (too much risk it gets hacked and pulls in malware or something).

And NACK on the concept. The whole point of the Bitcoin network it to prevent double-spending; making it easier to double-spend is a bad idea.

morcos commented at 2:34 pm on June 30, 2015: member

@petertodd , why not pursue the path that lets some version of RBF get merged. #6176 defaulted to FSS, but had an option to switch to full RBF correct? Would people be ok with that? And then if you want to separately coordinate a date where interested parties can change their command line option to run with full RBF, thats not even the subject of a PR. I agree coordination makes sense, but it doesn’t HAVE to be automated.

petertodd commented at 3:08 pm on June 30, 2015: contributor

@gavinandresen Note how the testing instructions call out a specific version of python-bitcoinlib by SHA1 commit hash; the repo getting hacked can’t cause you to get compromised. Equally, python-bitcoinlib is a widely used and reviewed library, and https://github.com/petertodd/python-bitcoinlib is the “official” repo for it. @morcos It’s a good question! I’m giving people a number of options here, so we’ll see what we get re: consensus; if @jgarzik’s switchover date idea doesn’t get consensus removing it is just a few lines of code change, leaving code that does both types of RBF robustly and in a well-tested fashion. @D9B4BEF9 It would probably be best to save that kind of broader discussion about zeroconf for the mailing list, or perhaps the BIP.

gits7r commented at 3:48 pm on June 30, 2015: none

@D9B4BEF9 @gavinandresen never said that there is 0 assumption of risk for merchants accepting zeroconf transactions. He just said that full-RBF will make the existing risk higher. The risk is not 0 and cannot be 0 regardless, this is just how bitcoin works (and this is why we need mining and proof of work security). I agree bitcoin should make everything possible to lower the double-spend risk as much as possible.

It is clear that full-RBF will make zeroconf a higher risk than it already is, and without zeroconf bitcoin can become a PITA for most merchants (10-60 minutes until you know if you can process an order or not). Less merchants accepting bitcoin has direct impact over the supply and demand market and obviously on the BTC/FIAT rate. I have great appreciation for @petertodd ’s work in bitcoin, and totally agree RBF is helpful, but I am against full-RBF (at least don’t make it the default behavior for relay nodes/miners) and would prefer FSS-RBF. If it matters, my business accepts zeroconf (for values of max 300$, thinking to raise the limit to 600$) and the double-spend losses are currently 0.

coblee commented at 6:53 pm on June 30, 2015: contributor

I just want to chime in from a payment processor (Coinbase) point of view. Full RBF is bad for Coinbase and bad for Bitcoin. If full RBF is turned on today by even a small set of nodes, Coinbase will have to stop accepting 0-conf for all orders. That means customers will need to wait for ~10 minutes for their order to confirm. This makes the checkout experience much worse than existing payment methods. It’s hard enough trying to convince consumers and merchants to adopt Bitcoin as a payment method today. Without 0-conf, you can forget about Bitcoin adoption for payments getting any traction whatsoever.

Accepting payments with 0-conf works ok today. It’s not perfect, but it works. We do acknowledge that as block rewards go to zero, there will be more incentives for miners to implement some sort of RBF. But this will take years before that happens naturally. When it does happen, there will likely be alternative better 0-conf solutions that merchants and payment processors can switch to. There is absolutely no reason to speed that up. Seems silly that anyone would risk hurting the nascent Bitcoin payment system just to prove a point.

morcos commented at 7:10 pm on June 30, 2015: member

@coblee You’re ok with FSS-RBF though right? What if we made the default policy of full-RBF be that it only applied if a transaction had been in the mempool for at least 2 blocks. That way transactions that you estimate have enough fee to likely be confirmed in the next block or two are also “safe”, but for fixing stuck transactions we can have the flexibility of full-RBF.

coblee commented at 7:58 pm on June 30, 2015: contributor

@morcos Yes, FSS-RBF is fine. Why is FSS-RBF not good enough to fix stuck transactions?

Having complicated rules for this in the core code leads to a lot of potential bugs. RBF seems like something miners should implement themselves and maybe charge for it. I don’t think it belongs here as a default policy.

luke-jr commented at 9:33 pm on June 30, 2015: member

@coblee The last time I tried to use Coinbase to pay a merchant, it required me to fund my Coinbase account first, wait for that to confirm, and only then pay in an instant transfer from my Coinbase account to the merchant. Full RBF should have no effect whatsoever on this usage pattern. (Although I would recommend changing it to at least allow automatically paying the merchant, rather than making me wait for the deposit to confirm before I can begin my attempt to pay the merchant).

coblee commented at 9:42 pm on June 30, 2015: contributor

@luke-jr Coinbase has always allowed you to pay a merchant by sending btc from any wallet. There was a flow where if someone was requesting money via email, the sender had to pay from their Coinbase account, but this is different from our merchant tools. And this bug has already been fixed. You can now pay money requests with an external wallet also.

laanwj added the label Validation on Jul 2, 2015

jtimon commented at 10:39 am on July 9, 2015: contributor

So it seems there’s no complains about making FSS-RBF the standard policy instead of FS. We can expose full-RBF as a non-standard policy (doing something similar to #5180), but since that seems controversial, it seems a better use of review time to get the uncontroversial parts merged first. I would even agree to make full-RBF the standard policy for 0.12 directly, but it doesn’t look like that’s going to happen soon and replacing FS with any other sane mempool replacement policy already moves the code the in the right direction for policy flexibility. Probably FS should be also maintained as an option.

And again, big NACK on deployment schedules for policy code, either it becomes part of the standard policy or becomes an alternative policy (although those would require something like #6068 to get started, a factory like in #5180, and then more changes to generically encapsulate the mempool replacement). Without making any moral judgement, maintaining and releasing versions with alternative policies yourself seems contradictory with proposing stricter than necessary restrictions to the use of the policy options when it comes to bitcoin/master.

jtimon commented at 11:01 am on July 9, 2015: contributor

Having complicated rules for this in the core code leads to a lot of potential bugs. @coblee full-RBF is much simpler than FSS-RBF.

btcdrak commented at 12:59 pm on July 9, 2015: contributor

@jtimon @petertodd I think it would be best for this PR to default to FSS-RBF, i.e. -fullrbfactivationtime=0. That way there is no scheduled turn on time and the default policy is FSS-RBF.

btcdrak commented at 1:00 pm on July 9, 2015: contributor

@wumpus looks like travis needs a kick

jtimon commented at 12:46 pm on July 10, 2015: contributor

I think this should be disabled by default with nFullRbfActivationTime = 0;

I think this should be disabled by default with fReplaceByFee = false; (no use of time for activation: activation time is whenever the user wants to).

I have created #6416 as a boilerplate for implementing alternative replacement policies. It would be nice to have a 0conf-safe-RBF built on top of that. We could first support it optionally and then make it the default. At the same time we can support other replacement policies like rbf, cpfp, etc. cleanly and without risk (because it’s just well encapsulated code disabled by default).

petertodd commented at 5:32 am on July 17, 2015: contributor

Closing for now, as this code will need to be re-worked for whatever memory-limited mempool proposal gets merged.

petertodd closed this on Jul 17, 2015

DrahtBot locked this on Sep 8, 2021

Scheduled full-RBF deployment #6352