Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't promote wallets that abuse the blockchain #704

Closed
wants to merge 1 commit into from

Conversation

luke-jr
Copy link
Contributor

@luke-jr luke-jr commented Jan 6, 2015

This causes a delisting of Electrum, which is apparently adding a blockchain spam feature in 2.0. Hopefully before the official release, they'll just get rid of that and maybe properly integrate email/IM as they appear to want. Glad to revise this to only modify README in that case.

Ref: https://bitcointalk.org/index.php?topic=915405.msg10060484#msg10060484

@harding
Copy link
Contributor

harding commented Jan 6, 2015

@luke-jr can you please notify ThomasV about this issue so he can comment if he wants to?

@harding
Copy link
Contributor

harding commented Jan 6, 2015

Oh, I found his GitHub username. @ecdsa: care to comment?

@gmaxwell
Copy link
Contributor

gmaxwell commented Jan 6, 2015

What Luke (in his usual not super effectively communicating way) is referring to is spesmilo/electrum#824 and 2efad717d8af578af321681275009d0bce5865ba.

While I don't endorse Luke's bombastic approach (sorry about that, I commented on #bitcoin-dev saying I thought this was unfortunate and Luke took initiative to open a PR, I would have preferred to talk to ThomasV directly first), the decisions there to include that functionality concerns me.

OP_RETURN is intended for small hash commitments, if people use it to store arbitrary publicly visible data it may need to be more actively blocked on the network due to systemic risks created for Bitcoin-- which would be unfortunate for the applications which are using it in a way that doesn't have those costs. To make this clear: non-transaction data creates some risk for all bitcoin users by reducing privacy, fungibility, and potentially resulting in miners or other service providers (e.g. bitcoin or electrum nodes.) being ordered to suppress transactions carrying certain data. So long as everything is opaque and pseudonymous there is generally nothing to target and so those attacks go elsewhere instead of the Bitcoin network.

From a client safety perspective displaying decoded text from the network has some of its own hazards (consider the OSX and IOS unicode parsing vulnerabilities) and there are some incentive problems: E.g. if "messages" are displayed with transactions it creates a strong incentive for people to rain down tiny unsolicited payments on third parties in order to advertise to people. We've already seen annoying floods of that based on 'ad material in vanity from address' resulting in the need to deploy relay behaviour changes (when we were unable to convince popular websites to stop showing "from addresses" to close the incentive-channel). Freeform ascii would vastly increase the incentives (and justifiable price) and potentially make the dust threshold a less effective tool. (If the justifiable price of an ad gets too close to transaction value a real user might honestly want to make; life is much harder.)

The general advice that I'd give is that this functionality really should only be provided as part of complete tools that do something useful for the user. Failing that, any decode really should display only hex, and any encode should either take a message to be hashed, or hex. Both to avoid the encoding vulnerabilities and discourage the advertising abuses and not set people up to expect the preservation of messaging functionality which the network can not reliably support.

From the perspective of requirements, I'd probably drop the word pollute since it's unnecessarily loaded.

I would also note that we successfully talked bc.i out of adding similar functionality a while back, and historically they've not been very responsive to outside input.

@ecdsa
Copy link

ecdsa commented Jan 8, 2015

@harding thank you for letting me know about this issue.
I am too tired to answer now, but I do share the concerns.

@harding harding added the Wallets label Jan 8, 2015
@atweiden
Copy link

atweiden commented Jan 8, 2015

Just to make sure I understand your position correctly, are you advocating to remove OP_RETURN from the Bitcoin protocol?

non-transaction data creates some risk for all bitcoin users by reducing privacy, fungibility, and potentially resulting in miners or other service providers (e.g. bitcoin or electrum nodes.) being ordered to suppress transactions carrying certain data

If not, then I have to assume OP_RETURN is available. By definition then, Electrum removing access to the feature puts it at a competitive disadvantage to wallets that do allow access to that feature. If you wish to disallow access to that feature network wide, this requires enforcement at the protocol level.

If your concerns are with exploits specific to Electrum, this is reasonable, but it should be fixable.

If your concerns are with Electrum users being spammed by advertisers, as you can see here, this is not the case. The OP_RETURN data is only displayed in a submenu which scarcely gets used. For perspective, there are GitHub issues where Electrum users have difficulty finding the preferences menu. Perhaps a more amicable solution is to disable the OP_RETURN feature by default but leave it accessible in preferences.

Either way, I predict the Bitcoin.com TLD will outrank the .ORG tld in due time if threatening wallets with delistment from your index, needlessly putting them at a competitive disadvantage is commonplace. As is, the software reflects the protocol.

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 8, 2015

@atweiden OP_RETURN is for aborting a script. The datacarrier format of "OP_RETURN " remains in existence as it always was, for storing a hash necessary to process the transaction - not for messages, data storage, or anything of that sort. The concerns are with Electrum promoting misuse/abuse of this standard for purposes it is not designed for.

If Bitcoin.com promotes harm to Bitcoin, then I predict it will end up being yet another "bad idea to pay attention to" website.

@saivann
Copy link
Contributor

saivann commented Jan 8, 2015

For what it's worth, I thought the delisting process would be reserved for protecting the users against potential important risks. If new requirements also include not promoting harmful uses of the network, it may be worth trying to evaluate the severity of these issues, although I guess this would likely prove difficult. All in all, it is unclear to me here if delisting Electrum for that issue alone would do more harm than good. This being said, I also share the concerns expressed here and looking forward to reading @ecdsa comments. Hopefully a solution can be found before the final release.

@harding
Copy link
Contributor

harding commented Jan 8, 2015

@saivann I think that's a very good point. I'd support changing the wallet "Basic requirements" list to "Basic security requirements", which would mean dropping this PR. (Although hopefully the discussion could move elsewhere, as I think there is a real issue here.)

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 8, 2015

It's noteworthy that Electrum has already been in violation of other best practices that haven't (yet) been promoted to hard "rules". If this was merely a first offense, I wouldn't have gone straight to a delisting so quickly.

@harding
Copy link
Contributor

harding commented Jan 8, 2015

@luke-jr to be fair, every single wallet we list fails some of the optional criteria. For example, Bitcoin Core doesn't do:

  • Supports HD wallets (BIP32)
  • Provides users with step to print or write their wallet seed on setup
  • On desktop platform ... encrypt wallet by default

I think @saivann's point was that the guidelines have so far been concerned with protecting users from losing bitcoins---policing other aspects of wallet design may be scope creep. (And it may take additional volunteer time when we already have a backlog of wallet pull requests that need reviewing.)

@saivann
Copy link
Contributor

saivann commented Jan 8, 2015

I was about to say the same thing :) I also think it makes sense to replace s/Basic requirements:/Basic security requirements:/ in the README for now, and maybe this pull request can be kept open so @ecdsa can comment (or close it if Thomas comments on his own repository instead).

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 8, 2015

HD wallets are more of a "nice to have", while this issue is "harmful to Bitcoin as a whole". Quite a difference.

@harding
Copy link
Contributor

harding commented Jan 9, 2015

@luke-jr I agree this issue is important, and I'm happy to contribute polite discussion and code to Electrum to help find an agreeable solution. But I also think the new requirement you have proposed is incongruous with the requirements we currently have listed. We shouldn't be putting network good behavior on a requirements list that is about protecting user bitcoins.

When we delisted Blockchain.info, it was purely for security-related reasons. It wasn't because they abused the network by connecting to thousands of nodes. It wasn't because they significantly reduce the privacy of everyday Bitcoin users, or possibly get innocent full node operators in trouble, by publishing IP-address-to-TXID mappings. It wasn't because they likely significantly slowed multisig adoption---increasing the risk to other peoples' coins---by not supporting payments to P2SH addresses.

I think we owe other wallets the same courtesy. Recognized experts like you and Greg can tell them when they're doing something harmful to the network or its users. Saïvann and other Bitcoin.org volunteers can delist wallets that don't meet the constantly-evolving security requirements. At some point, if wallets maintainers don't heed the warnings, there's likely to be overlap, as there was in the case of BC.i.

It is my personal expectation that volunteer-run open source projects like Electrum will be easy to persuade to follow network best practices---as long as we approach them respectfully and listen to their concerns (and remember that they're busy just like the rest of us!).

@dabura667
Copy link

It's noteworthy that Electrum has already been in violation of other best practices that haven't (yet) been promoted to hard "rules".

@luke-jr I would like to know some examples if you have some stories.

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 9, 2015

@dabura667 For example, Electrum promotes the "from address" myth by displaying "address balances" and similar nonsense. I tried to explain why this is broken-by-design, but it still remains to this day, AFAIK.

@ecdsa
Copy link

ecdsa commented Jan 12, 2015

Sorry for taking time to answer. There are several issues here.

The first issue is the risk of OP_RETURN being misused. I completely share the concerns exposed by @gmaxwell, and I am willing to remove or amend these features in Electrum, in order to reduce the risks for the Bitcoin network.

Please note that the Electrum GUI allows not only to display OP_RETURN fields in clear text, but also to compose OP_RETURN messages, to be added to a transaction. Obviously, both the display and the compose functions should be amended in order to prevent misuse. I believe that making Electrum display and parse only hexadecimal strings instead of clear text would address most of your concerns.

The second issue is whether this kind of policy will work in the long term. I am not so sure about that, because wallet software is subject to competition. If there is some demand for a tool being misused, it will be difficult to go against that. Only the future will tell if this is an effective measure. For now, I believe that it is worth trying. Another additional way to address this demand will be to better integrate wallet software with email/messaging tools.

The third issue is how the small group of developers who control the bitcoin.org website view their role and define their policy. There seems to be a confusion between practical objectives and moral positions here. Preventing blockchain abuse is a practical goal, on which we can agree. Considering that a wallet is guilty of 'violation' or' offense', and refusing to engage in normal communication with developers because of that perceived guilt, is a moral position.

Delisting a wallet from bitcoin.org may allow you to reach your moral objective, in the sense that you will have 'punished' what you consider to be 'evil', and restored what you see as 'order'. I doubt that it would make things evolve in the direction of your practical objective, which is to prevent blockchain misuse; it could even be counterproductive in that respect.

It would be better for Bitcoin if the maintainers of bitcoin.org were able to make the distinction between their moral positions and practical objectives, and if they based their policy only on the latter. If some of them are unable to make that distinction, I believe they should not be granted access to the website.

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 12, 2015

You're confusing what the website promotes or doesn't promote with a form of punishment.

@harding
Copy link
Contributor

harding commented Jan 12, 2015

@ecdsa thank you for reading our concerns, thank you for taking the time to answer, and thank you for agreeing to (at least temporarily) change Electrum's OP_RETURN input/output interface to hexadecimal. That's a major improvement.

I'll ponder the remainder of your post for a while before replying so that I can give your comments the thoughtful response they deserve.

Everyone else: we've discussed (but not agreed) that this issue may be out of scope for Bitcoin.org's wallet security & privacy requirements, and we now know @ecdsa will likely make changes in response to the concerns in this issue. I therefore suggest closing this issue in three days (around 14:00 UTC on Thursday) if nobody has any significant problems with that.

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 12, 2015

ACK closing as not an urgent issue.

@saivann
Copy link
Contributor

saivann commented Jan 12, 2015

@harding Thanks, agreed!

@ecdsa Thank you very much for your consideration, that's truly appreciated. For what it's worth, I disagree with some of the language used by @luke-jr (e.g. punishment, violation, offense) and I agree that it would have been more appropriate to contact you first before submitting a removal on bitcoin.org . Hopefully there won't be enough demand for such feature over time to make it become a real issue.

As suggested by @harding, I have opened pull request #709 for clarifying that current requirements are focused on the security of the user. "Protecting the network" is a different category which I think could possibly be considered eventually, but I think now may be too soon and I am not convinced that this would have more good impact than drawbacks. As opposed to security requirements, such requirements would go against individual interests of the users and developers, which might prove inefficient to deal with.

@luke-jr
Copy link
Contributor Author

luke-jr commented Jan 12, 2015

@saivann Again, I never used "punishment", and I did contact them for the first incident (which they never fixed).

@saivann
Copy link
Contributor

saivann commented Jan 12, 2015

@luke-jr Oh sorry, I scratched that word.

@harding
Copy link
Contributor

harding commented Jan 15, 2015

Closing per discussion. For reference: @ecdsa changed Electrum to only accept and display OP_RETURN scripts in spesmilo/electrum@9a6d98f

Thank you everyone for commenting.

@harding harding closed this Jan 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants