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

Chain Agnostic Provider Handshake #25

Merged
merged 2 commits into from
Oct 26, 2020
Merged

Conversation

pedrouid
Copy link
Member

As a result of the discussion in #24 I've created a new proposal focused on chain agnostic provider authentication

Copy link
Member

@ligi ligi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<editor_mode>
Thanks for the PR. Formally it is good - so I approve it and am happy to merge.
</editor_mode>
<user_mode>
Not really happy about the wording "authenticate". Wikipedia says about authentication:

is the act of proving an assertion, such as the identity of a computer system user.

and I do not see any proving of assertion here. Maybe something like handshake could be a better name here?

Also maybe we should not reduce the answer to yes or no. An alternative I imagine could be that the client requests a set of chains and functions and the response could be a subset of functions/chains and accounts. (just thinking out loud here as I think there could be cases where functions or chains could be optional)

</user_mode>

@pedrouid
Copy link
Member Author

Not really happy about the wording "authenticate".

Agreed... I wasn't also very happy with the wording "authenticate" since there isn't an actual signature being returned either to verify account ownership or to authenticate the set of methods requested.

Suggested replacements: andshake, disclosure, connection, agreement, initialization

Also maybe we should not reduce the answer to yes or no. An alternative I imagine could be that the client requests a set of chains and functions and the response could be a subset of functions/chains and accounts.

Technically there are multiple error messages to describe the reason for rejecting this method. However my reasoning for not allowing a partial approval is to provide a clear intent of the dapp requirements

In theory a dapp would strictly need these methods to provide complete functionality of its application.

However we could provide a partial approval with the potential of requesting methods later on with a second method. Im not a huge fan of this approach but Im open for discussing it in more detail

@ligi
Copy link
Member

ligi commented Oct 22, 2020

Suggested replacements: andshake, disclosure, connection, agreement, initialization

I think we should go with handshake as stated in the first comment

In theory a dapp would strictly need these methods to provide complete functionality of its application.

However we could provide a partial approval with the potential of requesting methods later on with a second method. Im not a huge fan of this approach but Im open for discussing it in more detail

ack - let's do this in a subsequent CAIP to keep things simple in the beginning - should also cover most paths

let's just say all chains and methods here are mandatory. Later on we can e.g. add the parameters optional_chains and optional_methods so dApps can handshake on and probe for optional chains and methods

@pedrouid
Copy link
Member Author

Alright I will rename all "authentication" mentions on this standard proposal to "handshake"

@pedrouid pedrouid changed the title Chain Agnostic Provider Authentication Chain Agnostic Provider Handshake Oct 24, 2020
@ligi ligi merged commit 0a4b924 into ChainAgnostic:master Oct 26, 2020
@dominicletz
Copy link

Awesome! This is great and totally relates to MetaMask/metamask-extension#1820 -- Didn't even realize this exists when I posted my wallet_requestChainId proposal over there.

Question: Should others APIs that are not specified but supported by the chain still be callable, blocked, or should this decision be up to the wallet implementation?

E.g. when they specifying ["eth_sendTransaction", "eth_signTransaction", "eth_sign", "personal_sign"] would that block eth_getBalance?

What do you think of making this choice explicit e.g.
["eth_sendTransaction", "*"] would mean eth_sendTransaction is required but other calls should still be possible

and then

["eth_sendTransaction"] would mean eth_sendTransaction is the only call made available by the wallet.

@pedrouid

@pedrouid
Copy link
Member Author

Yeah exactly. I want the provider to be explicit about the methods it wants to use. One could add eth_getBalance as well but from my experience with WalletConnect I saw providers being developed with a split responsibility around read methods and signing methods. This is also present with ethers v5 design where you have Provider and Signer classes.

That being said I'm not sure if wildcards should be valid unless they are prefixed, eg: "*" is invalid but "eth_*" is valid. Yet I would like to see methods being very explicit because one of main motivations for this proposal was the lack of support for eth_signTypedData so using a wildcard would essentially loose the purpose here

@pedrouid pedrouid deleted the patch-5 branch January 3, 2021 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants