Reverted double spend relaying (for now) #4550

issue jgarzik opened this issue on July 17, 2014
  1. jgarzik commented at 1:41 PM on July 17, 2014: contributor

    The double spend relaying stuff isn't quite baked fully yet.

    Links: #4484 #4450 #3883

    Related FLOSS principle: Make it socially OK to revert a recently applied change. It is easy to "pull the trigger" and merge. The side effect of that policy is sometimes a change is pulled too soon, and needs reverting.

    It can be merged again later once fully analyzed and tested.

  2. dgenr8 commented at 7:53 PM on July 17, 2014: contributor

    Drat, I should have thought earlier to ask mainnet nodes running git master to connect to me so their double-spends would show up here. I was just about to do that on the dev list.

  3. laanwj added the label P2P on Jul 17, 2014
  4. laanwj commented at 10:12 AM on July 18, 2014: member

    Reluctant ACK on reverting it for now. For two reasons

    • We are not convinced of the robustness of the new code. @gmaxwell (or was it @sipa) experienced some memory corruptions that were likely introduced with these changes. This means more testing and review is necessary.
    • Because the change itself is controversial. People are polarized around it. Does the current solution provide protection against double spends, or does it open up new angles to do so? (according to @SergioDemianLerner it does) I think the proper place for this higher-level discussion is the mailing list, not this github.
  5. gmaxwell commented at 11:03 AM on July 18, 2014: contributor

    ACK on reverting

  6. sipa commented at 4:16 PM on July 18, 2014: member

    Agree on reverting.

    Summarizing earlier arguments:

    • Either people don't use it, and it just adds complexity to the code, extra bandwidth, and potentially increased chance that fraudulent double spends make it in. (given that the ultimate goal here seems to make the usage of 0-conf more safe, I assume that's a property that is unwanted).
    • If people do start using it to increase safety of using 0-conf transactions, attackers can move to either sending double spends directly to miners (in which case again it is just extra code complexity and bandwidth) or potentially to flooding the rate-limited double-spend channel (even worse).
    • It removes the ability to rely on the property that bitcoind only relays what it considers valid itself (which is unreasonable on the p2p network, but reasonable if you're running a trusted own node acting as a gateway for other services). (though this can be solved by making the relaying optional, or using a separate message for it).
    • There have been several implementation problems with it since it was merged, indicating insufficient review. @laanwj: not memory corruption, but a mempool inconsistency (see #4517).

    Overall, I believe this is not a feature that the P2P network can perform. Things might have been different if our signing scheme supported short proofs of double spending, but it doesn't (such proofs are as long as the transactions themselves + inputs consumed). Moving the functionality to more specialized transactions (as suggested by @petertodd) or a separate double-spending relay network seems more viable.

  7. dgenr8 commented at 5:41 PM on July 18, 2014: contributor

    The lack of double spend alerts is costing bitcoin payees or their processing agents in the real world, today. Even if they don't see the alert in time in a particular instance, they need to be alerted.

    The perfect cannot be the enemy of the better.

  8. jgarzik commented at 5:44 PM on July 18, 2014: contributor

    @dgenr8 Can you quantify that assertion?

  9. petertodd commented at 7:05 PM on July 18, 2014: contributor

    NACK

    I think the underlying approach of strongly limiting max bandwidth consumed to a safe level is a good one. Yes there have been other issues, but we've got a very motivated contributor, @dgenr8, who is working hard to fix the issues as they're found. I'd be inclined to give him some more time to do that, and it'll be easier to do so if the patch is left merged. Of course, this does need to be re-evaluated again closer to release.

    The bigger picture is regardless of which side is right, the patch is better, if not perfect. If it really does help prevent doublespends then it's better. If it doesn't it help it gets us further towards a replace-by-fee model. I don't believe it will do significant, if any, harm to non-zeroconf usage of Bitcoin, and if anything, it will be a useful testing ground for applications like fee-bumping, which again has proven hard to get consensus on. @dgenr8 We should temporarily allocate a NODE_RELAY_DOUBLESPENDS service bit in the experimental range and set it. You could then find nodes on the network that had adopted this new behavior and interconnect them manually to get info on how the behavior works in practice. We can drop the service bit prior to the 0.10 release.

  10. sipa commented at 7:10 PM on July 18, 2014: member

    If we want a replace-by-fee model, we should enable replace by fee, not unintentionally encouraging it by a patch that seems to want to do the opposite (making successful double spending attacks harder).

  11. petertodd commented at 7:19 PM on July 18, 2014: contributor

    @sipa Getting consensus on that appears to be impossible. Meanwhile the "non-replace-by-fee" camp appears to strongly think double-spend relaying is a good idea, so I'm inclined to let them test out that theory in the real-world.

    Of course, full disclosure: I've got paying clients who want to see replace-by-fee implemented and they think relaying double-spends is a good halfway measure.

  12. sipa commented at 7:24 PM on July 18, 2014: member

    I don't know what will or should happen - a world where the order of transaction creation is irrelevant (and the highest fee-paying one wins) or one where some best-effort mechanism remains to prefer the first spend (potentially combined with a mechanism for getting information about other spends). I don't want to take a side here, but If there's no consensus on what should happen, then IMHO the behavior shouldn't be changed.

    You seem to believe this helps replace by fee; others seem to believe this improves safety of 0-confirmation transactions, two radically conflicting positions. Either or neither seems possible to me, but not both.

    Let's not change network behavior for the wrong reasons.

  13. petertodd commented at 7:45 PM on July 18, 2014: contributor

    I would have thought differently a few months ago, but right now I see no way to get more consensus on this issue. The change does appear to be relatively low risk to me so long as @dgenr8 continues the work he's been doing fixing the issues; I do not consider "might make zeroconf even less safe" to be a risk, given that we do that on basically every release.

    Regardless of whether we change something for one right reason, and one wrong reason, or one wrong reason, and one right reason, we're still changing something for at least one right reason.

  14. dgenr8 commented at 9:19 PM on July 18, 2014: contributor

    @jgarzik This data records double-spends seen by just 2 mainnet nodes since 5/3/2014.

    It can actually be deduced fairly surely that one merchant suffered at least 16.5 BTC in losses between 5/3/2014 and 6/20/2014. This is only possible because 1) the merchant is identifiable by his addresses 2) the merchant personally confirmed to me that he was suffering these losses (he did "hand over the goods") and 3) tx2 paid only the sole non-merchant tx1 address (so "matched decrease" doesn't overstate the loss).

    Due to being alerted (out of band, not by bitcoin core), this merchant has modified his services and no successful double-spends against him have been recorded by these 2 nodes since 6/20/2014.

    The data set implies an upper bound of ~346 BTC from 5/3/2014 through today on losses suffered due to confirmed double-spends that the 2 nodes recorded. Transactions where tx2 actually paid more to tx1 addresses have been netted from this number. It overstates any actual losses by an unknown and probably large amount, because address ownership, quid pro quo, and other transactions between the parties are unknown, just to name a few factors.

  15. petertodd commented at 10:25 AM on July 19, 2014: contributor

    @drak No-one is paying me who has experienced losses; those who have contacted me about losses got their help for free. After all, at least some of the attacks were done with my code. Anyway, I think it's ok to say that one of the people I talked to was Bitzillions - their losses are pretty easy to figure out by just looking at p2p network logs. Everyone else cut their losses to zero pretty quickly by changing their practices to avoid depending on zeroconf for security.

    As for clients, I can say that among other things Mullvad is interested in using replace-by-fee scorched-earth to secure micropayment channels.

  16. dgenr8 commented at 4:49 PM on July 19, 2014: contributor
    • There is no reason to believe that the 16.5 BTC victim's losses weren't also happening for months before 5/3/2014 (this merchant was not Bitzillions BTW).
    • Upper-bound losses on attempted respends where tx1 was confirmed were 152 BTC, with the same caveats on the amount as above. So on all relayable attempts, upper-bound losses were 498 BTC. A double-spent bitcoin was 2x more likely to get confirmed than not. Double-spends that were first seen in a block have only been included since 6/22/2014, so the ratio is actually higher.
    • Regarding that point, just a reminder that influencing confirmation is not the reason for alerts. The reasons are to influence real-world delivery and advise of potential losses.
    • With no relay today, the other 7500 nodes saw many respends that these 2 nodes didn't see.
    • Starbucks takes 3.6 minutes to deliver coffee. For them, accepting 0-conf payments with alerts would make attackers at least a little uncomfortable.
  17. dgenr8 commented at 5:08 PM on July 20, 2014: contributor

    Focusing in on quick double-spend attempts where tx1->tx2 elapsed interval < 1 minute:

    • Upper-bound losses on all relayable attempts: 103 BTC
    • ...where tx2 confirmed: 57 BTC
    • ...where tx2 confirmed, assuming spender financed all fee bumps: 55 BTC
  18. mikehearn commented at 10:54 PM on July 20, 2014: contributor

    Disagree with revert for these reasons:

    1. The entire purpose of having long test periods between releases is to find bugs that aren't shaken out by tests. In this regard it seems to be working - a bug that took days to trigger consistency checks was found. As new features are being merged I guess the next release is not imminent, so more "baking time" is a good thing. If there still are issues being found close to release time, then sure, it'd make more sense to back it out or disable it. Otherwise it's expected that master can be less stable than release code.

    2. New tests are being written and new improvements being made already. So I think this will help find any further issues that may be lurking.

    3. Philosophical discussions can go around in circles forever. There must be a decision point: that decision point was Gavin merging the change. A lack of such decision points will just turn Bitcoin into Wikipedia, subject to constant back-and-forth edit wars with no clarity over direction and no ability to make progress on anything beyond simple optimisations.

  19. rebroad commented at 12:47 AM on July 21, 2014: contributor

    I notice someone mentioned "out of band" double-spend alert system - what system is/was this? In the same way that I don't think the "alert" message should be part of the P2P protocol (because it is client specific), I'm also tempted to suggest that perhaps double-spend functionality should also not be part of the core protocol.

  20. dgenr8 commented at 3:20 AM on July 21, 2014: contributor

    @rebroad Email

  21. gmaxwell commented at 5:00 AM on July 21, 2014: contributor

    I'm afraid dgenr8's example above is misleading. I was unable to find in my own data the example he was referring to, and so I inquired off thread with Tom.

    It turns out that what he's referring to were recent double spends against SatoshiBones— this is a 'blockchain gambling site' which, like 'satoshidice' communicates with the gamblers by return transactions. Attackers can attempt to profitably double spend these sites by selectively conflicting them if and only if they discover they've lost their bets via the loss spend. Double spend relay is unhelpful in this case because its inherently too late and, if anything, would reduce the security of these (very ill advised) cases since it would enable more miners to engage in a highest-fee policy which would increase the success rate of the later adaptive conflicts.

    (Though I don't find the reduction argument terribly compelling here, since it's easy enough to do replace-by-fee regardless— and already the notion that that kind of service is completely insecure (https://bitcointalk.org/index.php?topic=327767.0) is already fairly well socialized)

  22. laanwj commented at 5:45 AM on July 21, 2014: member

    @mikehearn The decision point is clearly 'revert'. @jgarzik @sipa, @gmaxwell and me are in favor in reverting. So I'm going to revert this.

  23. mikehearn commented at 10:30 AM on July 21, 2014: contributor

    So we have an edit war between @gavinandresen and @laanwj ?

    What is the criteria you will require for the feature to get re-merged?

  24. laanwj commented at 11:38 AM on July 21, 2014: member

    Don't pretend that this was just my initiative. The entire core dev team (save @gavinandresen) was for reverting.

  25. mikehearn commented at 12:14 PM on July 21, 2014: contributor

    Alright. Then what's the next step here?

  26. laanwj commented at 12:18 PM on July 21, 2014: member

    Read my first post here.

  27. mikehearn commented at 12:25 PM on July 21, 2014: contributor

    Yes, I did. Your first point is "needs more testing". That's completely reasonable, and it seems the best way to get testing is to have it be in git master. Indeed would the mempool consistency issue have been found without that?

    The second point is it's controversial. I don't know if there's any resolution for that. Almost any change can be controversial forever.

  28. laanwj commented at 12:34 PM on July 21, 2014: member

    It is not clear that it actually does what it claims to. There is a lot of confusing and different people claim it will have different effects on the network. Applying this right now is not software engineering it's snake-oil medicine.

  29. dgenr8 commented at 5:28 PM on July 21, 2014: contributor

    @gmaxwell Only the very first loss was it too late for relaying to prevent.

    With relaying, their wallet could have told them every single time, and the rest of the network could have identified it too. He is moving off blockchain. Being alerted (offline) got him to go there.

    The reason these facts are not in most data sets is precisely because this data is tossed out by the network.

  30. gmaxwell commented at 5:50 PM on July 21, 2014: contributor

    @dgenr8 I had the data but assumed it couldn't have been what you were talking about because its irrelevant— if you note, when I emailed you, I didn't just ask what it was— I asked and then said "Are you referring to SatoshiBONES?" because it was the only thing that matched at all, even though it didn't make any sense to cite it.

    I'm unclear about what you're saying with "Being alerted (offline) got him to go there", I warned the operator of that service of it's vulnerability and urged him to change its behavior many months ago. It's also been double spent at several times in the past (and see also the betcoin dice thread I linked where 3k btc were stolen by ghash.io, and most of the people responding are blaming the victim for their insecure transaction practices which were the same as satoshi bones). ... Bitcoin-QT already tells you when a transaction conflicting with your wallet shows up in a block (wallet conflicts field, and confirm count -1). So between a direct personal warning from and the wallet telling you when you've been hosed, I'm not sure what more notice you expect would have helped.

    Each of the interactions with the site are independent. You can't know that an individual interaction with the site is going to be double-spent until its too late (because the double-spender inherently must wait until loss-notification to double-spend). So every attempt is the "first" from this perspective. (And if, instead, you mean knowing that the whole transaction style was vulnerable the fact that this is well know, was reported directly to the operator of the site, and that bitcoin-qt already tells you when you lost relative to what your wallet was showing you, means that there wasn't a question of knowing the service was vulnerable and being ripped off)

  31. dgenr8 commented at 5:55 PM on July 21, 2014: contributor

    Regardless of who alerted him and when, it's not scalable for people to be emailing and telephoning victims.

    If it's a good thing that victims are alerted, then being alerted sooner is better.

    PS I'm impressed that you are storing raw dropped double-spends.

  32. laanwj renamed this:
    Revert double spend relaying (for now)
    Reverted double spend relaying (for now)
    on Aug 1, 2014
  33. MarcoFalke added the label TX fees and policy on Aug 19, 2016
  34. MarcoFalke removed the label P2P on Aug 19, 2016
  35. MarcoFalke commented at 9:33 AM on August 19, 2016: member

    Anything left to do here?

  36. laanwj commented at 10:11 AM on August 19, 2016: member

    No, there are no plans in this regard anymore, let's close this.

  37. laanwj closed this on Aug 19, 2016

  38. rebroad commented at 4:16 AM on November 15, 2016: contributor

    I'm confused - although I originally ACKed this it seems that it might still be beneficial to have some double spend notification ability in the client - but to ensure that the UI mentions that not every double spend will necessarily be noticed by the software.

    Also, this was more than two years ago this it was reverted "for now" - and I see no linked pulls to suggest that it's been re-added. How long is "for now" going to be?

  39. laanwj commented at 9:12 AM on November 15, 2016: member

    How long is "for now" going to be?

    Forever, I suspect. Please notice that this issue is more than two years old. Plans and priorities changed.

  40. DrahtBot 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: 2026-04-13 18:15 UTC

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