-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
EIP: Payment request URL specification #681
Conversation
Great to see work on a proper specification! Just have 2 concerns currently after reading it initially:
This just as some initial thoughts - will think deeper about this later today. cc @kumavis , @manuelsc , @alexvandesande , @jbenet , @prusnak |
About a year ago I was all for including data in the URI / QR codes / and so forth. I'm not as for it, but I think it's a necessary thing to include. I think that it should come with a very, very large red warning to wallets that if you accept data via URI, you need warn the user about the potential downsides to this (e.g. a malicious data string could send all your tokens). For example, we parse the signed transaction and show users, so sending a standard ERC-20 token would show "sending 1 GNT" over "sending 0 ETH". Some others do not, and show This is what MEW currently doesThis is not saying it's the correct way, simply what we currently use. The data and possible gas limit do not currently work due to ENS revisions, its never been a widely used or promoted feature, so I urge you to take this with a grain of salt as your progress. However, these keys match the raw / unsigned tx (found at bottom of this wall of text), which is a obvious benefit.
No fields are mandatory. Obviously, not really that useful if you don't include some things, but whatever. You never know what the use case would be. Maybe someone wants to show someone how to send a message to whatever address with data string. Maybe they just want to ensure the user sets a gasPrice of 60 GWEI bc there is a token sale going on. Leave it up to the people providing these QR codes to provide what is necessary. That said, exchanges / wallets / shops that are building these URIs / QR codes should be discouraged from including any unnecessary data. e.g. in 99% of cases, including Example of a URI that used to work on MEW:
TokensThere are a few ways to handle this:
Regardless, should have a key that aligns to value ContractsWe have been speaking with Matt @ Etherscan about contracts. It would be super freaking cool if there was a was for a user to click a link from Etherscan that had the specific (e.g. one function) loaded up in the user's wallet, where the user could then interact with the
This is slightly unrelated but figured I would include this anyways. Totally probably best to ignore this :P Raw / Signed TransactionsAnd now we are really off topic but if we are going to discuss QR codes, someone speak up if there is a better way than just throwing the full signed / unsigned TX data in a QR code. Potential issues:
Above 2 maybe as separate EIP to keep this one super focused Reference: a raw (unsigned) TX:
Final Thoughts
|
Agree that |
I think it would be nice to require the checksummed address format. |
EIPS/pay_req_url_fmt.md
Outdated
URLs embedded in QR-codes, hyperlinks in web-pages, emails or chat messages provide for robust cross-application signaling between very | ||
loosely coupled applications. A standardized URL format for payment requests allows for instant invocation of the user's preferred | ||
wallet application (even if it is a webapp or a swarm đapp), with the correct parameterization of the payment transaction only to be | ||
confirmed by the (authenticated) user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style: Recommend not including hard-line breaks in prose text. As is, it is a bit of a pain to read on a 1900px wide window (split diff), and similarly hard to read on a phone.
I agree with @tayvano that we shouldn't be striving for bitcoin similarity. I see minimal value in such similarities and it constrains design decisions. I think if we are going to use This means that the pieces of information that should be included are:
As @tayvano indicated, most of these things will be left out in most calls but I think there is value in allowing the caller to specify any subset of them. Another thing that I strongly recommend is some form of protocol versioning. While the above list makes sense now, Ethereum is still very young and the set above may not make sense in a year or two. For example, there has been discussion about changing ETH to no longer be special and instead be a traditional token. If this were to go through, then A simple means of versioning would be to have the version be the first segment like Re @tayvano's suggestion to denominate in ETH, not WEI. I quite strongly disagree with this. While it is nice when a URI is semi-readable, the intent is not to make them user friendly, especially when it introduces risk into the system. If values are denominated in ETH then that means many transactions will contain fractional ETH. It is incredibly easy to get the parsing wrong and parse the URI into a double/float and accidentally truncate it! On top of that, the receiving system will need to send that number as WEI anyway so you are just making their life hard. The sending system also probably is using WEI under the hood (at least I hope they are), so both systems are having to convert to/from ETH/WEI and deal with the fact that you need a 300-bit (or something) floating point number to accurately store ETH (and any token with "decimals"). |
After sleeping on it.......
|
Really not sure about adding ENS here - and if we really add it here IMHO it should be at least prefixed because:
|
I think that this is a higher-level protocol than the one in ERC #67 and it is really unfortunate that they use the same protocol name. I am willing to change it to something else. @fjl proposed |
…ed to "ethpay" for distinction and compatibility.
EIPS/pay_req_url_fmt.md
Outdated
@@ -7,31 +7,25 @@ | |||
Category: ERC | |||
Status: Draft | |||
Created: 2017-08-01 | |||
Requires: #20, #137 | |||
Requires: #20, #67, #137 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does it require ERC-67?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, it should not be required. It is merely referenced.
To highlight that the purpose of this URL format is different from that in ERC #67 and to remain compatible with it, I have changed the I am very hesitant to switch to WEI (and atomic units in general) as suggested, because human readability and writability are very important. More important than the offline use case, which should be handled though ERC #67 links, as those also help with gasprice and other low-level stuff, which cannot be decided safely offline anyway. |
can you elaborate on why you think these are important in this case. Would also appreciate some use-cases |
@ligi basically, this ERC is to provide the same functionality as BIP21. People write BIP21 links and QR-codes by hand all the time. The standard, most anticipated and most important use case is that the user clicks (scans, etc.) such a link, it brings up their wallet app asking for confirming the payment. In this case, the wallet needs to know about decimals anyway in order to display a meaningful message to the user. Thus, no matter if the value in the URL is in decimal representation or in atomic units, the wallet app needs to have access to the contract to convert between the two. |
Can you support this claim with data? Would also love the input of @schildbach in this regard. |
I disagree about wei vs ETH - URIs are supposed to be human readable, and decimal ETH is far more readable for practical purposes. This is not a machine interchange standard, but rather a human one.
I promise we'll never issue 42 character TLDs starting with 0x.
You are welcome to invent your own squatting-free name service and then submit a proposal to add support for it to a standard like this.
I would not expect offline or hardware wallets to parse these URIs themselves, though - they would get a transaction in ready-to-sign form from something online. |
@tayvano , I disagree with you that using WEI would save complexity and bad things. Please read our discussion with @ligi above; in both cases, the user-facing app will need to do a conversion for which it will need to access the contract in case of token payments. It would, however, make it near-impossible and highly error-prone for human beings to construct payment request URLs, should they require amounts in WEI. @ligi , I obviously have no data, but I have seen it done many times and regularly do it myself. Whenever I invoice in BTC, I include a |
If you really NEED to include decimal point in the URI, please include decimals as well (so the offline signer, does not need to interact with blockchain to get the correct value for a token). Something like the following could work:
|
OK - so we need the prefix - I don't think it will be me as I am busy with other projects currently - but we need an upgrade path here - and this is what I was stating in the original comment
Is there anything in code that prevents it? EDIT: with registrar.ens.domains I am at least able to spend money on a domain that is a valid address (https://etherscan.io/tx/0x5262ed605bb4c924ae627b38b26db4066c097d346f1d262d2e97011858bf5b65) - but then it is just buggy - I don't see the bids anymore and the 0.01ETH are just wasted - the ens-bids-backup does not contain the bid .. |
FWIW, MEW doesn't allow any ENSs names to be registered that start with Adjusting ENS to address 42 char / 0x is relatively easy thing to do. This is not to say that more consideration & discussion isn't warranted, but let's eliminate the technical feasibility from the discussion. ENS meetup / panels / discussions are on Aug 11-14. I am for moving discussion of ENS to new EIP in order to keep it focused & moving. |
@Arachnid I don't agree with this assessment (and this is probably the source of disagreement in general on this topic). The purpose of a URI is to make it easy for application A to talk to application B. If they are human readable that is a nice advantage, but if you look at most URIs on the internet, that isn't a primary concern. Even Google's URIs which include the URI encoded query string in them also include a bunch of crap that I still don't fully understand even though I am often fiddling with them manually. IMO, the purpose of this spec (and #67) is to facilitate data transfer between applications. I do not think we should be targeting manually constructed URIs or human read URIs. It is the job of the tool on either end to present the data to the user in a user friendly way. In the case of eth vs attoeth, I have personally seen too many errors around parsing values into JavaScript doubles to be comfortable with anything other than hex encoded values. Just the other day some ICO lost millions of dollars due to this bug (don't remember the name). If the values are hex encoded they aren't human readable, and its much easier to encode/parse a 256-bit hex integer than a 300-bit (depending on decided mantisa size) floating point value. |
@MicahZoltu , I disagree. What you write is true about ERC #67 , but not this ERC. It is important that payment requests be readable and writeable by humans; we know that much from Bitcoin (BIP21). |
no - you assume this and I have not yet seen any data that supports this. |
@ligi if I see something left and right, I do not need "data" to prove that it occurs sufficiently frequently. Android intents get fired automagically upon opening URLs, and those URLs are often hand-crafted in case of BTC payment requests (in emails and chats, for example). Generating a QR-code from a hand-crafted URL is also something for which there are many on- and offline tools. I really doubt that everybody except me uses specialized bitcoin QR code generators instead of simply hand-crafting a URL and using whatever QR code generator is at hand. |
I strongly disagree - and I think it all boils down to who you see as the target audience. Everyday humans (who I see as the future target audience for Ethereum and maybe this EIP) will not hand-craft a URL - they will use tools for this. Hand-crafting URLs will only be done by done by tech-savvy people - and I hope we overcome the time where cryptocurrencies are only used by us ..
QR-Code could also be replaced with NFC/Bluetooth solutions - same applies |
yea - but they have other problems than missing 681 .. Anyway this goes way out of topic. They could easily implement it - not sure why they don't. Guess they are busy fighting x-code and trying to get their apps in the store. Really sad people waste their time this way. |
5 major wallets (trust, imtoken, toshi, cypher, blockchain). none have support. seriously, WTF am I missing here? |
yes I also think we need more support in general: https://ethereum-magicians.org/t/we-need-more-support-for-erc-681/897 |
dude, i can't just tell my customers to use android. they tell me what they use. i also did write to them today. this is kind the most important feature of ethereum. they will probably implement it eventually. but once again, wtf? also seems like QR does not even work on Trust anymore for regular send input. unbelievable. |
@ligi Challenge accepted! We will do that in ETHBerlin! 🔥 |
Hey guys, just wanted to share two tools here that might be useful for people trying to implement this on their end:
Note: I wrote both in a rush so please lmk if you find any issues with it (or even better submit a PR and fix it!) |
I am really puzzled upon this commit: It breaks compatibility and I really do not see any advantage of it being there except for improved human readability. I signal removing this. |
I disagree with this change though - so only make it so we *can* parse it - but not really exposing it. I really hope this change gets taken back. Context: ethereum/EIPs#681 (comment)
fix syntax error copied from ethereum#681 Co-Authored-By: MrChico <martin.lundfall@gmail.com>
Hello guys. I'm sorry for digging this up, but I have a serious doubt with this. For ERC-20 tokens, such as USDT, which has 6 decimals, to request 1.5 USDT, the payment URI whould look something like this, right?
However, when I parse that with MetaMask, it shows 1.5 million USDT to send, which makes no sense. In order to make it parse 1.5 USDT I'd have to put the URI as
which makes even less sense, since it's an uint, shouldn't even accept decimal numbers. I went through this entire thread, but I couldn't come to a conclusion how are we supposed to handle decimals? Is MetaMask's implementation wrong, or am I doing something wrong? Thanks |
sounds like the MM implementation is wrong there - seems to assume 18 decimals where it is only 6 |
no, even worse, it assumes the value as the main denomination, not atomic units.
|
yea this is wrong. Please report upstream. Is this in the mobile wallet or the browser plugin? |
Android app |
I see here: #67 |
There is no EIP-67. #67 is just an idea for an EIP that never actually made it to a standard. |
I know, but I've seen it informally called "eip 67" all over the place. |
You probably want to continue this over at the discussions-to for this EIP. It can be found at the top of the EIP at https://eips.ethereum.org/EIPS/eip-681 |
* Create eip-non_wallet_keys * Update and rename eip-non_wallet_keys to eip-non_wallet_keys.md * Update eip-non_wallet_keys.md correct typo * Update and rename eip-non_wallet_keys.md to eip-1581.md * Update eip-1581.md * Transaction Hash URI Draft * update EIP number * spellcheck * rename title, more clear semantics * grammar fix * spellcheck * fix EIP to valid * move discussions to ethereum-magicians * Update EIPS/eip-2400.md Co-Authored-By: Alex Beregszaszi <alex@rtfs.hu> * Update EIPS/eip-2400.md Co-Authored-By: Alex Beregszaszi <alex@rtfs.hu> * Update EIPS/eip-2400.md Co-Authored-By: Alex Beregszaszi <alex@rtfs.hu> * use github usernames * Use correct term * adapt eip-831 copied text to 2400 type * fix duplicated link * add examples * better rationale * submitted transaction * better abstract * Update EIPS/eip-2400.md fix syntax error copied from #681 Co-Authored-By: MrChico <martin.lundfall@gmail.com> * tx status, err msgs, event details, + rationale * error details, some `should`s must be `must`s * spacing * fix title * fix EIP title and add 155 requirement * move to review status * Apply suggestions from code review Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com> * fixed links * remove references to ERC-831 * Fix references to EIP-681 * Remove references section * Remove external links from body * Rename Title "Simple Summary" to "Description" * Move use cases to motivation * Elaborate on motivation * move description from body to header * reorder header elements * reorder header element required to after created * remove reference to EIP-20 * Update title and description Co-authored-by: Bitgamma <michele@bitgamma.com> Co-authored-by: Nick Johnson <arachnid@notdot.net> Co-authored-by: Alex Beregszaszi <alex@rtfs.hu> Co-authored-by: MrChico <martin.lundfall@gmail.com> Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
* Create eip-non_wallet_keys * Update and rename eip-non_wallet_keys to eip-non_wallet_keys.md * Update eip-non_wallet_keys.md correct typo * Update and rename eip-non_wallet_keys.md to eip-1581.md * Update eip-1581.md * Transaction Hash URI Draft * update EIP number * spellcheck * rename title, more clear semantics * grammar fix * spellcheck * fix EIP to valid * move discussions to ethereum-magicians * Update EIPS/eip-2400.md Co-Authored-By: Alex Beregszaszi <alex@rtfs.hu> * Update EIPS/eip-2400.md Co-Authored-By: Alex Beregszaszi <alex@rtfs.hu> * Update EIPS/eip-2400.md Co-Authored-By: Alex Beregszaszi <alex@rtfs.hu> * use github usernames * Use correct term * adapt eip-831 copied text to 2400 type * fix duplicated link * add examples * better rationale * submitted transaction * better abstract * Update EIPS/eip-2400.md fix syntax error copied from ethereum#681 Co-Authored-By: MrChico <martin.lundfall@gmail.com> * tx status, err msgs, event details, + rationale * error details, some `should`s must be `must`s * spacing * fix title * fix EIP title and add 155 requirement * move to review status * Apply suggestions from code review Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com> * fixed links * remove references to ERC-831 * Fix references to EIP-681 * Remove references section * Remove external links from body * Rename Title "Simple Summary" to "Description" * Move use cases to motivation * Elaborate on motivation * move description from body to header * reorder header elements * reorder header element required to after created * remove reference to EIP-20 * Update title and description Co-authored-by: Bitgamma <michele@bitgamma.com> Co-authored-by: Nick Johnson <arachnid@notdot.net> Co-authored-by: Alex Beregszaszi <alex@rtfs.hu> Co-authored-by: MrChico <martin.lundfall@gmail.com> Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
Hi Guys, Sorry for digging up the conversation again, I need to build a QR code that can have the account address and only the data in it the amount can be entered by the user itself or i can add it in the QR code too. Can someone tell me is there any standard to do this. Or is that even possible to have this type of QR code that can be readable by the different mobile crypto wallet like metamask and trust wallet. Like they read all data address, data and the amount |
few wallets support this at the moment |
Payment request URL specification for QR codes, hyperlinks and Android Intents.