Opt-in RBF must be strictly enabled or disabled before GBT can be called

wangchun commented at 10:06 pm on February 3, 2016: none

Pool operators must strictly specify their preference to opt-in RBF.

luke-jr commented at 10:07 pm on February 3, 2016: member

Concept ACK, but see #3229 for past discussion.

dcousens commented at 10:34 pm on February 3, 2016: contributor

Concept ACK, light utACK 07bd7c5

jtoomim commented at 10:56 pm on February 3, 2016: none

This probably breaks unit tests. Perhaps you should add a check for regtest or testnet mode first? #3229 had the same issue.

Require configuration of opt-in RBF before creating blocks 02bc16ab0e

wangchun force-pushed on Feb 4, 2016

wangchun commented at 1:03 am on February 4, 2016: none

Updated with check for testnet and regtest

laanwj added the label Mining on Feb 4, 2016

sandakersmann commented at 2:36 pm on February 4, 2016: contributor

ACK

laanwj commented at 2:41 pm on February 4, 2016: member

It would be better to update the RPC tests to pass the option explicitly instead of adding a flag to CChainParams. This makes sure that the switching code is (or at least, can be) actually tested.

sandakersmann commented at 2:41 pm on February 4, 2016: contributor

This should be in 0.12

luke-jr commented at 6:05 pm on February 4, 2016: member

Agree with @laanwj on passing the flag in tests.

I also think it would be better to just reopen #3229 if the NACKs there are determined to be outdated/overruled… @sandakersmann Way too late for any changes to 0.12 now.

sandakersmann commented at 6:10 pm on February 4, 2016: contributor

@luke-jr Then we should set default for -mempoolreplacement to false in 0.12.

petertodd commented at 7:50 pm on February 4, 2016: contributor

Weak NACK

I’m dubious about making it more difficult for users to mine out-of-the-box. p2pool is a thing that we want to encourage, and every additional setting that you must set to use it is a major barrier to adoption; there are literally dozens of settings that you might want to set.

A better way to deal with this kind of choice is choice between different Bitcoin protocol implementations with different philosophies behind them, e.g. Bitcoin Core and Bitcoin Classic.

Also, for this setting in particular, there was very little controversy associated with it pre-merging, including ACK’s by Jeff Garzik and others with strong pro-zeroconf feelings. This is notably unlike things like OP_RETURN, where even among the technical community there was controversy pre-merging. The controversy post-merge seems to be based mainly on misunderstandings of how well zeroconf works; note my blog post on the subject: https://petertodd.org/2016/are-wallets-ready-for-rbf

sandakersmann commented at 10:24 pm on February 4, 2016: contributor

@petertodd 0-conf works well enough today. Too bad you have to destroy this use case and make bitcoin less useful as a currency.

gmaxwell commented at 10:30 pm on February 4, 2016: contributor

@sandakersmann Please treat contributors here with respect. None of the contributors here want to make Bitcoin less useful. It sounds like you may be confused about the functionality in Bitcoin Core, please check out the FAQ: https://bitcoincore.org/en/faq/optin_rbf/

sandakersmann commented at 10:42 pm on February 4, 2016: contributor

@gmaxwell I have already checked out the FAQ and I’m not confused. When it comes to showing respect I’m sure I’m not the worst around here.

luke-jr commented at 11:53 pm on February 4, 2016: member

@sandakersmann If you are not confused, then you should be aware that RBF does not break unconfirmed/off-chain transactions at all. They continue to work fine with the same lack of security they already have. Anyhow, this is not the place to discuss the merits of specific policies.

Aquentus commented at 8:16 pm on February 7, 2016: none

utACK. Developers, with no modern mining or economic experience, should not be making policy choices in regards to defaults anyway, but only offer options so that the miners can choose policy aspects in a decentralized manner.

in src/chainparams.cpp: in 02bc16ab0e

243@@ -242,6 +244,7 @@ class CRegTestParams : public CChainParams {
244         vSeeds.clear();  //! Regtest mode doesn't have any DNS seeds.
245 
246         fMiningRequiresPeers = false;
247+        fMiningRequiresConfiguration = false;

MarcoFalke commented at 10:34 am on February 9, 2016:

Please remove those. Each difference between regnet and mainnet makes our tests a bit less useful.

You can adjust this in qa/rpc-tests/test_framework/util.py or even better in the respective qa/rpc-tests that cover this.

laanwj commented at 4:00 pm on April 1, 2016: member

Closing this, there is no agreement to do this and furthermore, comments on the code are not being addressed.

laanwj closed this on Apr 1, 2016

DrahtBot locked this on Sep 8, 2021

Opt-in RBF must be strictly enabled or disabled before GBT can be called #7464