Add EIP-1654: Off-chain dapp-wallet authentication process #2995
Conversation
There was a problem hiding this comment.
Most of the content in this specification doesn't seem like stuff that needs to be a standard. While it may be a Good Idea, not all Good Ideas need to be standardized. If two dapps each utilize a different mechanism for signature validation that is OK.
Why is more than just EIP-1271 required here? If a contract implements 1271 then it can validate arbitrary signatures and there is no need to have all apps verify the same signature. Each one can verify something else and that is OK.
|
|
||
| ## Copyright |
There was a problem hiding this comment.
| ## Copyright | |
| ## Security Considerations | |
| TBD | |
| ## Copyright |
| ## Implementation | ||
| Packages implementing the purposed algorithm: | ||
| - Javascript: https://github.com/dapperlabs/dappauth.js | ||
| - Go: https://github.com/dapperlabs/dappauth | ||
|
|
There was a problem hiding this comment.
I believe this section is no longer required, and I encourage removing it if you don't have an inline reference implementation.
|
|
||
| --- | ||
| Thanks to @dete @Arachnid @igorbarbashin @turbolent @jordanschalm @hwrdtm for feedback and suggestions |
There was a problem hiding this comment.
| --- | |
| Thanks to @dete @Arachnid @igorbarbashin @turbolent @jordanschalm @hwrdtm for feedback and suggestions |
EIPs are not the right forum for giving thanks/kudos/props/shout-outs. If people want, they can look at the discussion-to link for some out of band history.
|
|
||
| ## Backwards Compatibility | ||
| - External wallets are backwards compatible with this process. | ||
| - Contract wallets need to support `isValidSignature()` as specified in this document. |
There was a problem hiding this comment.
Why not EIP-1820 so any contract that supports arbitrary execution can also work?
| A user agent intended to work with the contract MUST generate signatures over a EIP-191 hash of a regular Keccak256 hash of the challenge message. | ||
|
|
||
| ## Rationale | ||
| [EIP-1271](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1271.md) has done a great work with standardising the signature validation method for contracts. This proposal builds on top of it a process for dapp-wallet authentication. |
There was a problem hiding this comment.
| [EIP-1271](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1271.md) has done a great work with standardising the signature validation method for contracts. This proposal builds on top of it a process for dapp-wallet authentication. | |
| TBD |
Rationale is a section for explaining why you made some specific choices that you made. This statement seems closer to a motivation... though it doesn't really add any value so I recommend deleting it entirely.
|
|
||
| ##### Contract | ||
|
|
||
| Must implement latest [EIP-1271](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1271.md) with `MAGICVALUE = 0x1626ba7e`. |
There was a problem hiding this comment.
Consider mentioning in the rationale where this number comes from and why it was chosen (could just be a one-liner if it is obvious).
| The client then proceeds to send the result to the dapp backend component along with the requested address to be used for authentication. The dapp backend recovers a public key from the signature, and checks if it has authorized control over the requested address. This check is done under consideration that the address may represent either an external wallet or a contract wallet. This process works with external wallets, as well with contract wallets that support the `isValidSignature()` method specified in this document. | ||
|
|
||
| ## Motivation | ||
| Dapps frequently offer a customised off-chain user experience in addition to their smart-contract interface. For example, a dapp may provide a push notification feature to their users, allowing them to stay notified about successful state changes associated with their public addresses. For these type of features, a dapp needs a way to assert that a user has authorized control over the public address associated with their account. |
There was a problem hiding this comment.
I would like to see a section added to the motivation section that explains why it is reasonable for a dapp to "authenticate" a user before letting them view things. If the data is coming from Ethereum, then it is all public and it gives the user a false sense of security.
| ## Definitions | ||
| - `contract wallet` A contract [account](https://github.com/ethereum/wiki/wiki/White-Paper#ethereum-accounts) deployed with the intent to be used as the ownership address for on-chain assets (including ether, ERC-20 tokens, and ERC-721 NFTs). It has the ability to transfer ether or dynamically execute actions on other contracts (acting as the owner of assets controlled by those contracts). Common examples of contract wallets are `multisig wallets` (such as the ones provided by [Gnosis](https://github.com/Gnosis/MultiSigWallet) and [Parity](https://github.com/ConsenSys/MultiSigWallet)) and `identity contracts`, as defined in [ERC-725](https://github.com/ethereum/EIPs/issues/725). | ||
| - `external wallet` An externally owned [account](https://github.com/ethereum/wiki/wiki/White-Paper#ethereum-accounts), controlled by a private key. Currently, most on-chain assets are owned by such accounts. A common example for an external wallet are the wallets generated by MetaMask. | ||
| - `authorized signer` An entity is considered to have authorized control over a wallet if either: | ||
| - It produced a signature of which the recovered address matches the wallet address. | ||
| - It produced a signature of which the contract at the wallet address responds with the magic value `0x1626ba7e` to the `isValidSignature()` contract call of [EIP-1271](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1271.md). | ||
|
|
There was a problem hiding this comment.
This isn't a section in the template. Recommend moving it to the ## Specification.
| author: Maor Zamski (@pazams), Christopher Scott (@chrisaxiom) | ||
| discussions-to: https://github.com/ethereum/EIPs/issues/1654 | ||
| status: Draft | ||
| type: Meta |
There was a problem hiding this comment.
At best this is Informational. I'm personally pretty against Informational EIPs in general though and think this is just a Good Idea which isn't appropriate for an EIP.
It is ok, but since a SC needs to know which to support in advance, it's better if everyone can agree instead of having separate clusters of dapps and wallets that work together for the purpose of authentication. That will be the blockchain equivalent of the "browser wars" era of the web.
Indeed this proposal is different than most other EIPs and the debate of its worthiness is important. The reason why so many apps were able to come together and use the same process was because of this draft. Now, when a new app developer reaches out to Etheremon to enquire "Hey Etheremon, how did you guys manage to have offchain authentication to work both with Smart Contract wallets and "regular" EOA wallets". Then Etheremon devs can reply with either:
If Ethereum is a dev-first community, the second option would make sense as it will make life easier for them. If this proposal needs to be closed, I just hope people can still reach the information needed via the issue #1654 |
The third option, which I recommend, is "See https://awesome.blog.example.com/how-to-have-off-chain-authentication-work-with-both-contract-and-eoa" |
|
I agree with @MicahZoltu. This EIP delves beyond what Meta EIPs traditionally are. When we talk about "process" in Meta EIPs we are talking about human processes and changes in how things are determined, primarily from a hard fork and EIP process perspective (and we are starting to move away from Meta EIPs that involve hard forks now that we have https://github.com/ethereum/eth1.0-specs/ and https://github.com/ethereum/eth2.0-specs). This EIP is more technical than "process oriented" based on the EIP repo definition of process as it is implied by previous EIPs so I don't think we should merge this. |
Replacing and updating a stale PR
This is a new PR following the kind closure of #2119. This PR is now updated to reflect the recent changes to EIP-1271 and adds it as a required dependency with this commit a1e98ed
Context
The "request for comments" phase on EIP 1654 has converged to a stable spec on #1654 .
Additionally, the process in this spec has been implemented successfully by a large number of dapps, including:
Crypto Kitties - https://www.cryptokitties.co/
Cheeze Wizards - https://www.cheezewizards.com/
MyCryptoHeroes - https://www.mycryptoheroes.net/
Etheremon - https://www.etheremon.com/
Megacryptopolis - https://www.megacryptopolis.com/
Blockchain Cuties Universe - https://blockchaincuties.com/
Hyperdragons - https://hyperdragons.alfakingdom.com/