Make RBF policies optional #7219

pull luke-jr wants to merge 2 commits into bitcoin:master from luke-jr:rbf_opts changing 6 files +26 −8
  1. luke-jr commented at 12:18 pm on December 16, 2015: member
    At least I assumed this was part of the original RBF merge
  2. util: GetArg might sometimes want a valueless result besides 0 26096acf94
  3. in src/init.cpp: in 0d0f28592d outdated
    455@@ -456,6 +456,7 @@ std::string HelpMessage(HelpMessageMode mode)
    456         strUsage += HelpMessageOpt("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", !Params(CBaseChainParams::TESTNET).RequireStandard()));
    457     strUsage += HelpMessageOpt("-datacarrier", strprintf(_("Relay and mine data carrier transactions (default: %u)"), 1));
    458     strUsage += HelpMessageOpt("-datacarriersize", strprintf(_("Maximum size of data in data carrier transactions we relay and mine (default: %u)"), MAX_OP_RETURN_RELAY));
    459+    strUsage += HelpMessageOpt("-rbf", strprintf(_("Allow replacement of transactions paying sufficiently higher fees (0 = never, 1 = always, 2 = opt-in, default: %s)"), nRbfPolicy));
    


    jonasschnelli commented at 12:22 pm on December 16, 2015:
    -replacebyfee?

    luke-jr commented at 12:26 pm on December 16, 2015:

    Hm, maybe… a bit long though? Anyone else have any opinions on what to call this?

    I was originally thinking -replacementpolicy, but then started overengineering it and figured that would be better left to a general policy refactoring later…


    tulip0 commented at 12:47 pm on December 16, 2015:
    Go with the longer, more detailed one. Mystery command line flags are no fun.

    dcousens commented at 1:02 pm on December 16, 2015:
    Both -rbf and -replacebyfee are acceptable IMHO. I’d personally opt towards the latter in all cases (API, code, variables, etc)

    jonasschnelli commented at 1:07 pm on December 16, 2015:
    I think abbreviations should not be used for command line arguments or RPC commands. Our scheme looks more (not everywhere) after full length parameters/commands (we use createrawtransaction instead of createrawtx, etc.).
  4. jonasschnelli added the label TX fees and policy on Dec 16, 2015
  5. jonasschnelli added the label Mempool on Dec 16, 2015
  6. dcousens commented at 1:02 pm on December 16, 2015: contributor
    concept ACK, once-over utACK @ 0d0f285
  7. sdaftuar commented at 3:06 pm on December 16, 2015: member

    In general I am sympathetic to the idea of trying to offer up policies that people want as much as reasonably possible, but my initial reaction is that this change isn’t a good idea. I don’t think we should offer policies that don’t make sense for a user to logically choose.

    What would be the motivation for someone turning off opt-in RBF in favor of a no-replacement policy? If it’s because they don’t want to personally accept transactions that might be later replaced, then I think we should just make sure that bitcoind offers whatever tools a user would need to determine whether a transaction is opting-in, so the user can reject the payment if they choose. Not accepting opt-in transactions at all seems much more useful to the user than locally choosing to not accept replacements of such transactions. Breaking relay of opt-in RBF replacement transactions seems to offer no additional local benefits that I can see (at least, not any local benefits that can’t be better achieved a different way), while it would provide some harm to part of the network (namely those users who might use it in the future).

    So offering it as a policy might be confusing to people, since having the option implies there’s a good reason some might want to turn it off, and I don’t think there is such a reason. (If there is currently a reason, say because bitcoind doesn’t currently make it easy for users to know whether a transaction is opting-in, then I think we should make that easy instead, which is not difficult to implement. If there are other reasons, I’d like to hear them.)

    I can understand the motivation for offering full-RBF, as it makes sense to me that some users might want that policy, but given the existence of opt-in RBF, is full-RBF likely to offer meaningfully different outcomes for what gets into your mempool? I expect not, and I don’t see the need for Bitcoin Core to support what has been a controversial policy that we expect would provide minimal actual benefit to users choosing to run it.

  8. luke-jr commented at 0:36 am on December 17, 2015: member

    What would be the motivation for someone turning off opt-in RBF in favor of a no-replacement policy?

    I’m sure there’s more, but a few that come to mind:

    • “I haven’t considered this enough to make a decision, so I want to maintain my status quo policy while still upgrading to 0.12.”
    • “I am mining myself, and would prefer to simply include users’ lower fees to encourage Bitcoin usage.”
    • “I prefer a non-fee based policy, such as priority.”
    • “I personally think Bitcoin would be better if only first-seen transactions were relayed or mined.”

    Breaking relay of opt-in RBF replacement transactions seems to offer no additional local benefits that I can see (at least, not any local benefits that can’t be better achieved a different way), while it would provide some harm to part of the network (namely those users who might use it in the future).

    That’s not our decision to make. People running nodes de facto want to turn this off/on, and have a right to do so; there is no reason to make it gratuitously difficult - that’s merely developers trying to usurp an authority that doesn’t belong to us.

  9. dcousens commented at 2:01 am on December 17, 2015: contributor

    that’s merely developers trying to usurp an authority that doesn’t belong to us.

    +1

    I think there is a fine line between what policies we should choose to bundle by default (and hence, if they can be, they should be configurable) and what we choose to omit. We have included RBF as a embedded policy, it is easily configurable, so why not?

  10. tulip0 commented at 2:29 am on December 17, 2015: none

    Supplying options that are illogical or dangerous does have some risk, there’s commonly distributed bitcoin.conf which users often copy terrible configurations. A quick google shows this one which sets maxconnections so high the node will certainly crash, turns on txindex and binds to 8.8.8.8. Others set gen, keypool=2, par=1, bind the rpc interface to 0.0.0.0, many other weird configurations.

    This probably isn’t up there with high potential for stabbing yourself in the foot, but it’s something to be aware of.

  11. petertodd commented at 7:37 am on December 17, 2015: contributor
  12. morcos commented at 12:54 pm on December 17, 2015: member
    I think without any reason to believe this is a desired feature, this is just another example of putting in too many knobs which complicate the code and the logic and hamstring further improvements without good reason. There should be a high bar for introducing complexity. I’m opposed to this change.
  13. luke-jr commented at 12:58 pm on December 17, 2015: member
    @morcos This isn’t complicated, and is clearly much-desired by users.
  14. policy: Allow configuring replace-by-fee by either disabling or enabling it 786f92d420
  15. luke-jr force-pushed on Dec 17, 2015
  16. luke-jr commented at 1:01 pm on December 17, 2015: member
    -rbf renamed to -replacebyfee
  17. sdaftuar commented at 2:57 pm on December 17, 2015: member
    @luke-jr I don’t think it’s at all intuitive to users that setting -replacebyfee to 0 would mean that you would still accept transactions into your mempool that are signaling opt-in to RBF (just not allowing them to be replaced). I would expect many users to be confused if they were to use that setting and then see transactions in their mempool get double-spent, because the rest of the network has a (known and documented!) different policy. That strikes me as a dangerous configuration and likely to cause any non-expert user significant confusion.
  18. morcos commented at 3:04 pm on December 17, 2015: member

    and is clearly much-desired by users. @luke-jr that would certainly influence my opinion. Can you elaborate? To @sdaftuar’s point, they may want to be able to make txs non-replaceable, but that doesn’t occur just by them changing their local policy. I can’t believe I’m suggesting this, but perhaps a more useful feature would be the ability to flag a single tx as non-replaceable, which might be something miners wanted to do if they want to mine a particular version of a spend. Would require some changes to the RBF logic though..

  19. dgenr8 commented at 6:22 pm on December 17, 2015: contributor
    NACK. I want opt-in RBF to work, and this change can only help it work less well.
  20. btcdrak commented at 7:02 pm on December 17, 2015: contributor
    utACK 786f92d
  21. luke-jr commented at 7:11 pm on December 17, 2015: member
    @morcos The merge of opt-in RBF has been and is highly controversial among users, at least on reddit.
  22. jgarzik commented at 7:15 pm on December 17, 2015: contributor

    @luke-jr Strictly true, but those comments often seemed to (intentionally?) miss the opt-in aspect, and so should be discounted. A typical reddit-esque comment of “$company is trying to kill zero-conf!” is silly when in fact opt-in RBF will not lead to increased attack surface at payment processors.

    opt-in RBF does not lead to ecosystem breakage and theft, unless I’m missing something.

  23. instagibbs commented at 6:21 pm on December 18, 2015: member
    Seems like the “always” option would upset the apple cart, if you are ostensibly caring about people’s concerns about opt-in.
  24. morcos commented at 9:34 pm on December 18, 2015: member
    I won’t say NACK, but I’d say I have a preference not to merge this.
  25. amacneil commented at 3:46 am on December 19, 2015: none

    NACK. Allowing nodes to run full-RBF (nRbfPolicy = 1) is dangerous for existing users who are not expecting their transactions to be replaced. If only a small portion of miners adopted this setting (e.g. 10%), it would allow non-RBF transactions to be reliably double spent 10% of the time.

    It would be better to release opt-in RBF without changing behavior/expectations of existing transactions, and at least get users used to this concept before allowing nodes to opt-in to full-RBF.

  26. luke-jr commented at 7:16 am on December 19, 2015: member
    Again, that is not our decision to make.
  27. petertodd commented at 4:48 pm on December 19, 2015: contributor
    @amacneil IMO that’s a good argument to have an option to always do full-RBF: it reminds people that first-seen reliant zeroconf can be easily broken by a small % of miners.
  28. petertodd commented at 5:09 pm on December 19, 2015: contributor

    Tested ACK 786f92d420650ee2918c482a31f44e1bd9326574

    • Help text correct
    • -replacebyfee=0
    • -replacebyfee=1/-replace-byfee
    • -replacebyfee=2
  29. amacneil commented at 5:31 pm on December 19, 2015: none

    @luke-jr @petertodd well that makes the arguments for this patch the same as full-RBF then.

    While I agree that long term it will be beneficial to move towards full-RBF, for now I was under the impression that the point of releasing opt-in RBF was to not change first seen relay expectations for existing users (at least for the 0.12 release).

  30. petertodd commented at 5:40 pm on December 19, 2015: contributor
    @amacneil It’s a default-opt-in. @luke-jr and I don’t agree 100% here, but I’d make the argument that the default will likely lead to the vast majority of the hashing power leaving it opt-in. As for those that enable full-RBF always, other than Bitcoin XT nodes they’re not going to get double-spends relayed to them anyway… which is I guess a bit ironic.
  31. DavidVorick commented at 8:56 pm on December 19, 2015: none
    concept ACK, there seem to be enough legitimate reasons for a user to want to turn off opt-in rbf, particularly if a miner. I do have concerns that people will disable opt-in RBF for the wrong reasons (they want to protect 0-conf), but I’m expecting that few enough people change the defaults in the first place that it won’t be an issue.
  32. amacneil commented at 7:31 pm on December 20, 2015: none

    @petertodd understood that it’s opt-in by default. My point is that even a small % of mining power running full-RBF will be effectively kill zero-conf for existing users. There is a separate debate as to whether people should be relying on zero-conf (they shouldn’t), but if we are going to allow/encourage even a small percentage of miners to enable full-RBF then there is really no point adding the opt-in semantics to transactions, because zero-conf will be broken anyway for a significant % of blocks. Obviously miners today could still compile and run your full-RBF patch themselves, but that is significantly more work than simply enabling a flag in core.

    To be clear, I am totally fine with the feature “Make RBF policies optional”, but I think it would be dangerous to add a feature “Allow nodes to opt in to full-RBF” today. If the nRbfPolicy = 1 option is removed (allowing only no-RBF or opt-in-RBF) then I have no issue with this PR.

  33. amacneil commented at 7:36 pm on December 20, 2015: none

    other than Bitcoin XT nodes they’re not going to get double-spends relayed to them anyway

    I have seen attacks in the wild where double spends are sent directly to known mining pool nodes, so XT is not required to enable this attack.

  34. aalness commented at 2:09 am on December 21, 2015: contributor
    Hmm. Why isn’t first seen safe an option? If I ran a mining pool I think I’d feel a lot more comfortable with an option where I can still take part in a fee market and not be potentially provably complicit in fraud. But IANAL.
  35. instagibbs commented at 2:13 am on December 21, 2015: member
    @aalness opt-in RBF is sending user signaling their intent to replace.
  36. aalness commented at 2:26 am on December 21, 2015: contributor
    Who knows how various jurisdictions will regard such signals given various circumstances. But allowing mining users to profit from a fee market with no possible fear of violating their ethics or local laws seems like a good thing to do to me.
  37. btcdrak commented at 2:30 am on December 21, 2015: contributor
    @aalness quit trolling. PR comments are for technical feedback.
  38. aalness commented at 2:41 am on December 21, 2015: contributor
    Where do I direct ethical questions about specific code changes?
  39. chaosgrid commented at 3:06 am on December 21, 2015: none

    This does not have economic consensus since 0-conf is being used today (i.e. PoS). It is not clear that FSS 0-conf transactions should be deprecated since they have their use cases. If one miner activates Full-RBF with this switch, 0-conf non-RBF transactions can be abused to double spend. Therefore it does not make sense to include such an option that puts certain economic entities or even unaware private users at risk of being defrauded.

    NACK

  40. HostFat commented at 3:08 am on December 21, 2015: contributor

    Are there other rules where the node can make choices on which txs, “supported from the protocol”, can or can’t be broadcasted to the network?

    This patch is possible, but I think that it should be coded in another way, to continue on putting txs on the mempool, and filtering somewhere else, mainly the on the user interface.

  41. jameshilliard commented at 3:16 am on December 21, 2015: contributor
    utACK https://github.com/bitcoin/bitcoin/commit/786f92d420650ee2918c482a31f44e1bd9326574 @chaosgrid Varied mempool policy among miners already makes double spending 0-confs fairly simple. @HostFat Since this is local mempool policy and not a consensus rule nodes/miners can basically do whatever they want already, BitcoinXT for instance relays double spends making them easily propagate to miners.
  42. chaosgrid commented at 3:29 am on December 21, 2015: none
    @jameshilliard Just because nodes and miners can run the mempool policy they want, it does not make it right to merge a change into the most widely used Bitcoin node software that puts a significant share of the Bitcoin industry at risk. Mempool policy is not a technical consensus rule, but it is very much an economic consensus rule right now (if you like that or not) just like it is economic consensus to run the Bitcoin Core software, at least for the moment.
  43. in src/init.cpp: in 786f92d420
    956@@ -956,6 +957,7 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
    957 
    958     fIsBareMultisigStd = GetBoolArg("-permitbaremultisig", true);
    959     nMaxDatacarrierBytes = GetArg("-datacarriersize", nMaxDatacarrierBytes);
    960+    nRbfPolicy = GetArg("-replacebyfee", nRbfPolicy, 1);
    


    chaosgrid commented at 3:38 am on December 21, 2015:
    Why does not entering a value default to “always”, which is not the global default? This is biased towards Full-RBF and not intuitive, because no value for a setting would imply the default value (opt-in) is used.

    instagibbs commented at 4:01 am on December 21, 2015:
    IIRC this is the default if someone passes the argument. But if the argument isn’t passed, it’s 2. I agree it’s confusing.

    amacneil commented at 4:04 am on December 21, 2015:
    I think this is a typo, GetArg only accepts two arguments.

    instagibbs commented at 4:06 am on December 21, 2015:

    chaosgrid commented at 4:08 am on December 21, 2015:
    Yes, he added it in the other commit. The third value is the default if the value string is empty, i.e. “-replacebyfee” defaults to “-replacebyfee 1”

    amacneil commented at 4:13 am on December 21, 2015:
    I see. Seems unnecessary, it would make more sense to use the default value if an empty/invalid string is passed.

    luke-jr commented at 12:19 pm on December 21, 2015:
    If a user passes -replacebyfee by itself, the logical expected behaviour is that it gets enabled always. The user wouldn’t pass any option at all, if they merely wanted the default behaviour.
  44. HostFat commented at 3:46 am on December 21, 2015: contributor
    @jameshilliard Bitcoin is a p2p network: nodes expect to always receive all the tx (and blocks) from the other nodes, that are compatible with the protocol, and then broadcasting again.
  45. jameshilliard commented at 3:50 am on December 21, 2015: contributor

    Just because nodes and miners can run the mempool policy they want, it does not make it right to merge a change into the most widely used Bitcoin node software that puts a significant share of the Bitcoin industry at risk. @chaosgrid There is significant risk already as a lot of miners patch core already so this doesn’t really change anything. Hopefully more people realize this.

  46. mmeijeri commented at 10:57 am on December 21, 2015: none

    It may be too soon to facilitate full RBF, but if we do it we should at least change the numbering scheme so it goes from less radical to more radical by switching the order of always and opt-in.

    0 = never, 1 = always, 2 = opt-in => 0 = never, 1 = opt-in, 2 = always

  47. luke-jr commented at 12:18 pm on December 21, 2015: member

    I wonder if the NACKs would prefer: 1) Core supports full-RBF-always as an option rather than try to force policies on users against their will; or 2) Miners de facto enable full-RBF-always regardless of what Core supports, in protest of developers’ attempted usurping of their authority over their own node.

    As for why FSS is not an option: it was not a trivial addition to the code, and possibly would jeopardise this making it for 0.12. I encourage adding FSS in a subsequent PR (and as a bitfield-style +4 so it can be combined with either opt-in or always).

  48. luke-jr commented at 12:20 pm on December 21, 2015: member
    @mmeijeri Oh, and the reason why 1=always, is because when looked at as a boolean, the values are 0 or 1.
  49. HostFat commented at 12:28 pm on December 21, 2015: contributor

    It’s a kind of tx supported from the network, so blocking them from being broadcasted again is in contrast with the rules of the protocol.

    It’s like saying that some tx are better than others, when ever someone could develop personally this kind of feature on his own fork, promoting these filters is a bad idea for a p2p network.

  50. luke-jr commented at 12:32 pm on December 21, 2015: member
    @HostFat Bitcoin has always filtered transactions, and must if anyone can be expected to be capable of using it. Please take your philosophical discussion of this principle elsewhere - it doesn’t belong here.
  51. HostFat commented at 3:40 pm on December 21, 2015: contributor

    Then we should add an option to not broadcast (not including on the mempool) tx coming from X addresses, it’s on the same line of thinking.

    Maybe even filtering the broadcast of some blocks from Y pools.

    But this open to situations where even if the user/pool does comply with the protocol, he will not be anymore sure that his tx/blocks will be shared on the network.

    It seems going against a part of the fungibility of the Bitcoin.

  52. jameshilliard commented at 3:49 pm on December 21, 2015: contributor

    @HostFat

    Maybe even filtering the broadcast of some blocks from Y pools.

    Aside from this being a bad idea in general I don’t think this is even possible if a pool is attempting to mask their blocks, the existing block tracking done by block explorers is done via coinbase sigs and known generation addresses, both of which are trivial to change. Most pools don’t mind being tracked as is since it’s basically free advertising.

    IMO any spam filtering should be completely agnostic in regards to the specific sender and receiver otherwise the bitcoin would lose fungibility.

  53. Aquentus commented at 5:57 pm on December 21, 2015: none

    There is a revolt over this pull request and many find it incomprehensible that this suggestion would even be entertained. Satoshi clearly showed how 0conf transactions can be made pretty safe https://bitcointalk.org/index.php?topic=423.msg3819#msg3819 and the vast economic majority sees even the entertainment of this idea as an outright attack on the fundamentals of bitcoin.

    There is no consensus whatever for this pull request which means that this should have been closed without a seconds consideration.

  54. jameshilliard commented at 6:05 pm on December 21, 2015: contributor
    @Aquentus since this isn’t a consensus critical change and only a local policy option I don’t see any issue with allowing a choice, pools can already trivially patch this in.
  55. chaosgrid commented at 6:09 pm on December 21, 2015: none

    @luke-jr

    I wonder if the NACKs would prefer: 1) Core supports full-RBF-always as an option rather than try to force policies on users against their will; or 2) Miners de facto enable full-RBF-always regardless of what Core supports, in protest of developers’ attempted usurping of their authority over their own node.

    Are you now threatening that if this does not get merged, you will go to miners and offer them a full-RBF-always patch and encourage them to use it? @jameshilliard

    There is significant risk already as a lot of miners patch core already so this doesn’t really change anything. Hopefully more people realize this.

    I do not see any miner today running full-RBF-always (for good reason). There clearly is no sound reason for implementing such an option. Also, I don’t really see a use-case for disabling the opt-in behavior altogether. In fact, this patch only contributes to making 0-conf behavior completely unreliable. I believe this PR is an attempt at undermining current 0-conf behavior to create a reality where full-RBF-always behavior can easily get merged.

  56. jameshilliard commented at 6:14 pm on December 21, 2015: contributor

    I do not see any miner today running full-RBF-always (for good reason).

    Are you sure? There could be smaller miners running full-RBF right now, there is already a patch set for it after all. Hard to tell when mempool policy is so varied as is(which makes a succesful 0-conf double spend easy to pull off).

    In fact, this patch only contributes to making 0-conf behavior completely unreliable.

    IMO it basically already is completely unreliable under adversarial conditions so this wouldn’t really change much, most that have lost money on it however don’t go publicizing it all that much.

  57. amacneil commented at 6:15 pm on December 21, 2015: none

    @Aquentus (and anyone else landing here from reddit), please don’t brigade from anonymous accounts, it does nothing to help the discussion. @luke-jr if there is consensus for core to support full-RBF-always, then I would prefer that over offering opt-in transaction semantics and then simultaneously breaking non-opt-in transactions by encouraging miners to run full-RBF. My understanding is that full-RBF is a controversial change and that many developers have argued against it, hence the compromise to support opt-in RBF transactions.

    Currently no pools are running full-RBF that I am aware of, despite @petertodd’s lobbying for this. Most miners have no interest in breaking existing use-cases for bitcoin or supporting merchant fraud. It is true that a single large mining pool could probably break zero-conf for everyone, but currently none have, and many bitcoin users/merchants would like to maintain the status quo (at least until more secure zero-conf alternatives have matured). What I am afraid of is that making this a built-in flag in bitcoin would make it easier to lobby unsuspecting miners into running full-RBF (“hey, just enable this flag to earn more money”), which is significantly more difficult than “hey, just recompile bitcoin with my custom patch”.

    Is there any reason not to simply remove the full-RBF option from this PR, and stick to the stated intent (making opt-in RBF optional)?

  58. sipa commented at 6:19 pm on December 21, 2015: member

    I don’t mind making opt-in RBF optional if people have concerns.

    I don’t like exposing full RBF, though. We have an equally functional and efficient mechanism now that doesn’t require instituionalizing behaviour that a large part of the community frowns upon (whether it is safe or not).

  59. amacneil commented at 6:24 pm on December 21, 2015: none

    IMO it basically already is completely unreliable under adversarial conditions so this wouldn’t really change much, most that have lost money on it however don’t go publicizing it all that much.

    Zero-conf is actually fairly reliable today. I was intimately involved with this at Coinbase, and they are successfully accepting zero-conf transactions for most payments today without any issues (under permanently adversarial conditions). I assume BitPay and other merchant processors have similar success. The only major exploit I’ve seen in the past involved taking advantage of @luke-jr’s spam patch and the Eligius pool, which has since been mitigated.

  60. jameshilliard commented at 6:28 pm on December 21, 2015: contributor
    @amacneil @sipa I think one thing about having this as an option is that it might make merchants take the lack of 0-conf seriously, IMO there is a delusion out there that 0-conf can be made safe when in really can’t be. The only reason it’s not as big an issue is that the majority of people aren’t actually trying to defraud merchants. I also did some testing and noticed that there are actually very few double spends being broadcast on the network(even when connected to nodes that relay 0-confs) so it seems more of an issue in regards to lack of attackers than anything.
  61. chaosgrid commented at 7:15 pm on December 21, 2015: none

    @jameshilliard

    This is getting extremely tiring and old. How many times does it have to be repeated that it is not necessary for 0-conf to be perfectly secure to still have value? It is all a matter of the cost/benefit trade-off for small payments.

    IMO there is a delusion out there that 0-conf can be made safe when in really can’t be.

    I don’t think anybody is saying 0-conf can be made safe. This observation is false.

    I also did some testing and noticed that there are actually very few double spends being broadcast on the network(even when connected to nodes that relay 0-confs) so it seems more of an issue in regards to lack of attackers than anything.

    This just outlines the broken thinking of relying on adversarial conditions all the time and not considering game theory. For example, if pulling off a double spend attack is fairly difficult and expensive to do with FSS-behavior across the network, then that means it is very unlikely that attack actually happens a lot (for small payments). So the issue is not a lack of attackers, but rather your failure of identifying the lacking incentives that causes the attackers to not actually attack.

    Btw.: This is a little off-topic but the exact same applies to block size limits. If there would be no limit, an attacker would be very unlikely to make a really big, over sized block because the orphan risk for himself would be too great. Additionally, the reputation loss would be critical and could lead to an exclusion of the miner from the rest of the network. In reality this attack will probably never happen.

    In conclusion, it does not make sense to design the system in favor of complete adversarial conditions. In both the case of full-RBF and also block size limits, Bitcoin Core is actually threatened to become a dangerous client to run considering the overall health of the Bitcoin network (0-conf reliability and congestion effects). And this is all due to the fact that there is too much fear of adversarial conditions and not enough trust in game theory.

  62. petertodd commented at 7:22 pm on December 21, 2015: contributor

    @chaosgrid: If you don’t need zeroconf to be perfectly sscurd, then surely having an obscure CLI option to enable full-rbf isn’t such a big deal in practice. It does however send a strong message that the Bitcoin Core team is not officially supporting zeroconf.

    On 21 December 2015 11:18:47 GMT-08:00, chaosgrid notifications@github.com wrote:

    @jameshilliard

    This is getting extremely tiring and old. How many times does it have to be repeated that it is not necessary for 0-conf to be perfectly secure to still have value? It is all a matter of the cost/benefit trade-off for small payments.

    IMO there is a delusion out there that 0-conf can be made safe when in really can’t be.

    I don’t think anybody is saying 0-conf can be made safe. This observation is false.

    I also did some testing and noticed that there are actually very few double spends being broadcast on the network(even when connected to nodes that relay 0-confs) so it seems more of an issue in regards to lack of attackers than anything.

    This just outlines the broken thinking of relying on adversarial conditions all the time and not considering game theory. For example, if pulling off a double spend attack is fairly difficult and expensive to do with FSS-behavior across the network, then that means it is very unlikely that attack actually happens a lot (for small payments). So the issue is not a lack of attackers, but rather your failure of identifying the lacking incentives that causes the attackers to not actually attack.

    Btw.: This is a little off-topic but the exact same applies to block size limits. If there would be no limit, an attacker would be very unlikely to make a really big, over sized block because the orphan risk for himself would be too great. Additionally, the reputation loss would be critical and could lead to an exclusion of the miner from the rest of the network. In reality this attack will probably never happen.

    In conclusion, it does not make sense to design the system in favor of complete adversarial conditions. In both the case of full-RBF and also block size limits, Bitcoin Core is actually threatened to become a dangerous client to run considering the overall health of the Bitcoin network (0-conf reliability and congestion effects). And this is all due to the fact that there is too much fear of adversarial conditions and not enough trust in game theory.


    Reply to this email directly or view it on GitHub: #7219 (comment)

    Sent from my Android device with K-9 Mail. Please excuse my brevity.

  63. petertodd commented at 7:22 pm on December 21, 2015: contributor

    James: there’s also surprisingly few vendors that are actually vulnerable. For instance, even shapeshift.io has such restrictive rules about what zeroconf txs they’ll accept that it’s rare for them go accept a tx without a confirmation.

    On 21 December 2015 10:29:27 GMT-08:00, James Hilliard notifications@github.com wrote:

    @amacneil @sipa I think one thing about having this as an option is that it might make merchants take the lack of 0-conf seriously, IMO there is a delusion out there that 0-conf can be made safe when in really can’t be. The only reason it’s not as big an issue is that the majority of people aren’t actually trying to defraud merchants. I also did some testing and noticed that there are actually very few double spends being broadcast on the network(even when connected to nodes that relay 0-confs) so it seems more of an issue in regards to lack of attackers than anything.


    Reply to this email directly or view it on GitHub: #7219 (comment)

    Sent from my Android device with K-9 Mail. Please excuse my brevity.

  64. petertodd commented at 7:22 pm on December 21, 2015: contributor

    Adrian: Any examples of a coinbase merchant that a) irrevocably provides the customer with something of value that can be sold, prior to the 1st confirmation b) doesn’t have the customers identity.

    Without that, you’re not in a position where you are relying on zeroconf transactions.

    On 21 December 2015 10:26:02 GMT-08:00, Adrian Macneil notifications@github.com wrote:

    IMO it basically already is completely unreliable under adversarial conditions so this wouldn’t really change much, most that have lost money on it however don’t go publicizing it all that much.

    Zero-conf is actually fairly reliable today. I was intimately involved with this at Coinbase, and they are successfully accepting zero-conf transactions for most payments today without any issues (under permanently adversarial conditions). I assume BitPay and other merchant processors have similar success. The only major exploit I’ve seen in the past involved taking advantage of @luke-jr’s spam patch and the Eligius pool, which has since been mitigated.


    Reply to this email directly or view it on GitHub: #7219 (comment)

    Sent from my Android device with K-9 Mail. Please excuse my brevity.

  65. jameshilliard commented at 7:27 pm on December 21, 2015: contributor

    @chaosgrid

    if pulling off a double spend attack is fairly difficult and expensive to do with FSS-behavior across the network

    IMO it’s not that difficult, most businesses just don’t offer service where a 0-conf would do much harm since they can just shut off a service or delay a shipment if it gets double spent.

    So the issue is not a lack of attackers, but rather your failure of identifying the lacking incentives that causes the attackers to not actually attack.

    I think most services that are vulnerable don’t accept 0-conf anyways or are very restrictive, not a lot of targets anymore where you could get much benefit.

    If there would be no limit, an attacker would be very unlikely to make a really big, over sized block because the orphan risk for himself would be too great.

    Not relevant in practice because if the miner is big enough, then they gain an advantage since it’s the equivalent to selfish mining.

    In conclusion, it does not make sense to design the system in favor of complete adversarial conditions.

    IMO we need to get as close as we can, there are far bigger issues that we need to worry about than 0-conf security and blocksize such as fungibility and confidential transactions(which is clearly an issue with coinbase tracking transactions).

    And this is all due to the fact that there is too much fear of adversarial conditions and not enough trust in game theory.

    I don’t trust game theory when the incentives can get screwy really fast.

  66. sandakersmann commented at 7:31 pm on December 21, 2015: contributor
    NACK
  67. HostFat commented at 7:32 pm on December 21, 2015: contributor

    I agree with @chaosgrid, and it’s true that accepting zeroconf on face2face trades is very common. Many are completely aware of the risks, but usually the cost of the fraud is higher of the benefit, by losing reputation or even worst. So by knowing this they still accept zeroconf on small tx, even by judging one by one their customers. No one is forced on accepting them.

    Anyway, I think that any feature that can enable filtering broadcasting of tx compatible with the protocol needs to be avoided.

  68. dgenr8 commented at 8:48 pm on December 21, 2015: contributor

    having an obscure CLI option to enable full-rbf isn’t such a big deal in practice @petertodd You must have forgotten about Linux distros who can package default options easily, but are much less likely to include software patches. And what about windows/mac users who are happy to change a shortcut or conf file, but aren’t going to download an untrusted binary? The upstream project, us, has some trust.

    Since you also proposed a straight change to the default earlier that @btcdrak questioned and @gmaxwell closed, your position is clearly pro-RBF, and not pro-opt-in-RBF.

  69. amacneil commented at 8:51 pm on December 21, 2015: none

    @petertodd note for the record I am no longer affiliated with Coinbase, so information may be a little out of date and opinions here are my own.

    The biggest target for 0-conf scammers are gift cards (both dedicated gift retailers, and regular merchants who sell gift cards). These are usually delivered electronically and easily be cashed out via purse.io or similar with little to no verified KYC. Generally the bitcoin payment processor eats the fraud cost once a transaction is marked as accepted via their API, so even though the retailer could simply cancel the gift card (if they were fast enough), they have no incentive to, and these systems are usually not automated anyway.

    Requiring confirmations is also not even possible for some merchants (for example: legacy e-commerce systems which cannot place a hold on inventory longer than 15 or 30 minutes, when a single bitcoin confirmation can sometimes take upwards of 1 hour). So these merchants would probably opt to stop receiving bitcoin payments altogether rather than wait for confirmations. Arguably this is not a huge loss for the bitcoin ecosystem, since very few people are using bitcoin to make online purchases where credit cards are already accepted, but it would be disappointing to see nonetheless.

    Generally I believe fraud costs are currently at an acceptable level (because merchants are still accepting 0-conf). However, if the % of mining power running full-RBF ever increases above % discount which gift cards can be flipped online, it would significantly alter the economics here.

  70. surg0r commented at 9:49 pm on December 21, 2015: none
    nack
  71. sandakersmann commented at 10:02 pm on December 21, 2015: contributor
    The people advocating for full-RBF do not want bitcoin to be used in retail situations. They want to cut this use case out of the ecosystem and full-RBF is their tool to accomplish this. They also think that full-RBF will ease the pain when we run into the 1MB blocksize limit and this is why they are in such a hurry to implement. They don’t wish to be open about this since the public outcry will be even bigger than it already is.
  72. pstratem commented at 10:15 pm on December 21, 2015: contributor

    utACK 786f92d420650ee2918c482a31f44e1bd9326574

    Each node operator has the right to run whatever policy they wish.

    If you oppose this pull request you oppose freedom of choice.

  73. sandakersmann commented at 10:22 pm on December 21, 2015: contributor

    If you oppose this pull request you oppose freedom of choice.

    Freedom of choice is great. Let’s remove the blocksize limit and let the miners make as big blocks that they wish!

  74. JornC commented at 11:06 pm on December 21, 2015: none

    It’s slightly unfortunate that this would negatively alter a small fraction of businesses’ faulty security assumptions while there is not yet a suitable alternative available short of waiting for confirmation or introducing any sort of trust, but considering this has been a topic of strong contention for well over a year now, and that this outcome or anything equivalent in implication (0-conf is not secure) has always been the sensible outcome, unfortunate is all it will have to be.

    utACK, with preference to default full-rbf when application-equivalent secure alternatives to 0-conf are available and practical.

  75. sandakersmann commented at 11:21 pm on December 21, 2015: contributor

    businesses’ faulty security assumptions

    Businesses have done commerce with 0-conf transactions for years now. It is not assumptions, it works. But with full-RBF it won’t work any more.

  76. surg0r commented at 11:25 pm on December 21, 2015: none

    They know exactly what they are doing and breaking. On 21 Dec 2015 23:23, “sandakersmann” notifications@github.com wrote:

    businesses’ faulty security assumptions

    Businesses have done commerce with 0-conf transactions for years now. It is not assumptions, it works. But with full-RBF it won’t work any more.

    — Reply to this email directly or view it on GitHub #7219 (comment).

  77. sipa commented at 11:25 pm on December 21, 2015: member

    I see no reason to provide full RBF in bitcoin core. Despite security concerns, people seem to like transactions being an implicit promise not to be double spent. We offer a way to explicitly opt-out of that promise now for cases where it matters. I think that suffices.

    NAK

  78. sandakersmann commented at 11:55 pm on December 21, 2015: contributor
    Good to hear that Pieter. You have always been the voice of reason from Blockstream.
  79. luke-jr commented at 2:10 am on December 22, 2015: member
    @sipa RBF does not change the implied promise of transactions, nor does setting the opt-out flag remove such a promise. I find it disappointing that even after your excellent post on how developers don’t have authority to do a hardfork on our own, you seem to support developers usurping this authority from node operators. :/
  80. dcousens commented at 2:18 am on December 22, 2015: contributor

    @luke-jr as mentioned above, I agree with this PR personally, probably for the same reasons you do.

    But, I think you are confusing @sipa’s comments on consensus authority with bitcoin/bitcoin’s policy authority, which, is much less important not relevant?

  81. jtimon commented at 2:45 am on December 22, 2015: contributor

    What about a -policy string argument:

    -policy=optinrbf [default]: Allows opt-in RBF. -policy=firstseen: Never allows spending conflicts replacements. -policy=fullrbf: Just does full RBF without looking at the op-in signal.

    That would make it easier to add other policies in the future, for example:

    -policy=random: randomly rejects some consensus-valid txs to the mempool. -policy=test: this would be basically equivalent to –acceptnonstdtxn=1 -policy=optinrbf …

  82. in src/main.cpp: in 786f92d420
    844+
    845             const CTransaction *ptxConflicting = pool.mapNextTx[txin.prevout].ptx;
    846-            if (!setConflicts.count(ptxConflicting->GetHash()))
    847-            {
    848+            if (setConflicts.count(ptxConflicting->GetHash()))
    849+                continue;
    


    jtimon commented at 2:48 am on December 22, 2015:
    There’s no need to use continue here.

    luke-jr commented at 3:19 am on December 22, 2015:
    How not? Please elaborate.

    jtimon commented at 7:37 pm on December 22, 2015:

    There’s never a strict need for a continue statement:

    0while(fCondition)
    1    if (!fA)
    2       continue;
    3    // Do AAA
    4}
    

    Is equivalent to

    0while(fCondition)
    1    if (fA) {
    2      // Do AAA
    3    }
    4}
    

    which is what we had and you are changing for no good reason. The second version (what we currently have) is not only more readable but also more flexible, for example, say we want to change the code to do the following:

    0while(fCondition)
    1    if (fA) {
    2      // Do AAA
    3    }
    4   // Do BBB
    5}
    

    In the second example it’s just adding the new code out of the if statement, in the first case we have to rewrite the code like the first case and indent first. The only reason I can think of for using continue is reducing this PR’s total diff(by avoiding the indent to the code under the new condition, or creating a new sub-function for the code under the new condition), but it doesn’t make the diff (or the final resulting code) more readable, so I really don’t see the point. I would generally avoid using continue unless it’s one of those rare cases where its use actually increases readability and reduces the potential for bug introduction (instead of the opposite as it usually does). For example, many continue statements in the same loop may be more readable than excessive nesting:

     0while(fCondition)
     1    if (!fA)
     2       continue;
     3    // Do AAA
     4    if (!fB)
     5       continue;
     6    // Do BBB
     7    if (!fC)
     8       continue;
     9    // Do CCC
    10    if (!fD)
    11       continue;
    12    // Do DDD
    13    // E, F, G...
    14    // Do ZZZ
    15}
    

    is more readable than, but equivalent to

     0while(fCondition)
     1    if (fA) {
     2      // Do AAA
     3      if (fB) {
     4         // Do BBB
     5         if (fC) {
     6           // Do CCC
     7           if (fD) {
     8              // Do DDD
     9                // E, F, G...
    10           } // D
    11         } // C
    12      } // B
    13    } // A
    14    // Do ZZZ
    15}
    

    Of course, the readability disadvantages of excessive nesting can always be fixed with sub-functions instead:

     0bool DoFromA()
     1{
     2   if(fA) {
     3       // Do AAA
     4       return DoFromB();
     5   }
     6   return false
     7}
     8bool DoFromB()
     9{
    10   if(fB) {
    11       // Do BBB
    12       return DoFromC();
    13   }
    14   return false
    15}
    16// DoFromC(), etc
    17
    18while(fCondition)
    19    if (DoFromA()) {
    20      // Do ZZZ
    21    }
    
  83. in src/util.h: in 786f92d420
    155@@ -156,9 +156,10 @@ std::string GetArg(const std::string& strArg, const std::string& strDefault);
    156  *
    157  * @param strArg Argument to get (e.g. "-foo")
    158  * @param default (e.g. 1)
    159+ * @param novalue What to return if option provided with no value
    160  * @return command-line argument (0 if invalid number) or default value
    161  */
    162-int64_t GetArg(const std::string& strArg, int64_t nDefault);
    163+int64_t GetArg(const std::string& strArg, int64_t nDefault, int64_t nNoValue = 0);
    


    jtimon commented at 2:53 am on December 22, 2015:
    Why not just return nDefault when the option is provided with no value?

    luke-jr commented at 3:18 am on December 22, 2015:
    The use case is given in the PR…

    jtimon commented at 7:10 pm on December 22, 2015:
    The exact same behavior can be achieved on init without touching this function. I repeat the question, why not just return nDefault when the option is provided with no value? In the concrete case of this PR, seems just a hack so that “-replacebyfee” is equivalent to “-replacebyfee=1” instead of “-replacebyfee=2” like it would be if this function received no extra parameters. In my opinion, that doesn’t justify adding a third parameter to this function. Anyway, if we switch to strings as we should, this discussion would be irrelevant (unless you want not using policy to default to one value, but using an empty -policy to default to another value).
  84. dcousens commented at 2:55 am on December 22, 2015: contributor
    @jtimon IMHO that is certainly the pipe dream.
  85. luke-jr commented at 3:22 am on December 22, 2015: member

    But, I think you are confusing @sipa’s comments on consensus authority with bitcoin/bitcoin’s policy authority, which, is not relevant?

    My point is that we don’t have such an authority at all, and trying to force users to exercise their authority in specific ways by making their desired policies explicitly difficult, especially in a case like this where it is easy to provide the configuration options, is an attempt to usurp their authority, and essentially the same as an attempt to hardfork without economic consensus.

    What about a -policy string argument:

    This is definitely where I’d like to see us go in the future, but not something viable for 0.12, which is the target of this PR.

  86. jtimon commented at 3:26 am on December 22, 2015: contributor

    @dcousens “pipe dream”: a hope, wish, or dream that is impossible to achieve or not practical. How is changing an integer enumeration to a string enumeration impossible?

    Re mergeability discussion:

    1. Miners de facto enable full-RBF-always regardless of what Core supports

    This alone justifies supporting full-RBF optionally IMO.

    • The feature does not change consensus critical code
    • The feature will have users (and @petertodd will be able to stop maintaining his fullrbf patch)
    • Maintaining the feature optionally is cheap in terms of code complexity

    The fact that some people so strongly oppose to supporting -policy=fullrbf in bitcoin core, indicates that there will be demand for -policy=firstseen. And since is also trivial to maintain, the same arguments apply, even if I personally think that is probably the stupidest spend conflict replacement policy after -policy=lastseen (yes, I think -policy=randomreplace makes a lot more sense).

  87. jtimon commented at 3:31 am on December 22, 2015: contributor

    This is definitely where I’d like to see us go in the future, but not something viable for 0.12, which is the target of this PR.

    Well, in the future each policy could have their own parameters and you told me those could be easily configurable in the GUI dynamically (see #6423 ), but the command-line interface can look more like that right now. In fact, if people don’t like -policy=test and this is going to be implemented with specific parameters too, I will have a hard time finding the next example to introduce the -policy string argument that truly “scales” command-line-complexity-wise.

  88. dcousens commented at 3:32 am on December 22, 2015: contributor
    @jtimon to clarify, I meant, that is exactly what I’d love to see, but, I’m not sure if it is going to be possible [in master anyway] due to how controversial this PR seems to be.
  89. luke-jr commented at 6:05 am on December 22, 2015: member
    Not worth arguing over this, so I’m just going to throw it in Bitcoin LJR and call it done.
  90. luke-jr closed this on Dec 22, 2015

  91. jtimon commented at 7:53 pm on December 22, 2015: contributor

    Not worth arguing over this, so I’m just going to throw it in Bitcoin LJR and call it done.

    I didn’t saw anyone complaining about offering firstseen (the previous default replacement policy) optionally. That will also make branches outside of Bitcoin Core that support -policy=fulrbf slightly easier to maintain (although it’s currently pretty easy as shown by this PR anyway). If we leave

    0-policy=fullrbf: Just does full RBF without looking at the op-in signal.
    

    out of the scope of this PR to avoid controversy, can we at least support:

    0-policy=default [default]: Allows opt-in RBF.
    1-policy=firstseen: Never allows spending conflicts replacements.
    

    ?

  92. luke-jr referenced this in commit 2586d73fa2 on Jun 28, 2016
  93. luke-jr referenced this in commit 4f6054722c on Jun 28, 2016
  94. MarcoFalke locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-01-21 09:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me