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

feat(caip-222): multi-chain wallet authentication #248

Conversation

ganchoradkov
Copy link
Member

Updated CAIP-222 to support multi-chain authentication.

Added ability to

  • request signatures for multiple chains in a single request
  • specify supported signing types via signatureTypes request parameter
  • respond with a list of signed CACAOs


```
signatureTypes: {
"eip155": ["eip191", "eip1271" ],
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if I support both eip191 and eip1271 I can just not define this field and get either?

Copy link
Collaborator

@bumblefudge bumblefudge Oct 19, 2023

Choose a reason for hiding this comment

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

yeah that's a good question. does not setting the parameter mean you have no preference and will TRY to verify anything?

We should define explicitly how to interpret the absence of an array, i think

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think wildcards are kind of unsafe and likely to lead to bad UX deadends; better for dapps to have to explicitly list which kinds they support, I would think.

Copy link
Member Author

Choose a reason for hiding this comment

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

The v1 (merged version) is very optimistic that the sender application would be able to validate the returned signature without knowing the signing type especially now that you could request non-evm signatures. This parameter lets you specify exactly which types you're supporting. Whether you can omit it, its up for debate. I've set it as optional to reduce devex friction.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK added a sentence to the definition above, take a look

This comment was marked as spam.

@@ -45,7 +46,7 @@ The application would interface with a provider to make request as follows:
"params": {
"cacaov": string,
"type": string,
"chainId": string,
"chains": string[],
Copy link

Choose a reason for hiding this comment

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

should this be optional and the wallet can just respond with any of those it supports?

Copy link
Member Author

Choose a reason for hiding this comment

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

The rationale is that the dapp would request signatures for all chains included in the chains parameter and the wallet would respond with signatures for those it supports as outlined here

The wallet SHOULD ignore all unsupported chains and SHOULD NOT auto reject the request if there are supported chains to sign without explicit user rejection.

In v1, chainId is required parameter so it makes sense to preserve that to an extended.
Additionally, setting chains to explicitely empty [] array is valid and kinda allows to not set any chains already.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think chainIds is the dapp telling the wallet which kinds of addresses it WANTS exposed (and implicitly, has use for). it might even be worth mentioning (in ### Security Considerations, if nowhere else) that a wallet COULD return addresses not on any of the requested chains (or choose which addresses to return if chains array was empty), but this is unspecified behavior and may lead to confusing UX.

my thought on an empty chainId is that it kinda translates to "any EVM keypair, I don't care" if combined with a set of signatureTypes that are all EVM-equivalent ( "eip155":{'eip191', 'eip1271'}, "polkadot":{'moonBeamSign'}", ... ). if emptyChainId but also un-specified signatureTypes.... I would just drop as a wallet, there's no way of knowing what that means.

not sure all this is worth putting INTO the spec, but it's the corner cases that come to my mind so far.

CAIPs/caip-222.md Outdated Show resolved Hide resolved
@bumblefudge
Copy link
Collaborator

I'm still wondering if it makes more sense for the multi-CAIP222 to be CAIP-248 or CAIP-222; would love input from potential or early implementers.

@ganchoradkov
Copy link
Member Author

ganchoradkov commented Oct 31, 2023

I'm still wondering if it makes more sense for the multi-CAIP222 to be CAIP-248 or CAIP-222; would love input from potential or early implementers.

@bumblefudge imho, it should be an update as it complely encapsulates the merged CAIP-222. Additionally, having them as different standards would cause conflics if one attempts to implement them at the same time in their current form forcing implementers to choose one OR the other

CAIPs/caip-222.md Outdated Show resolved Hide resolved
CAIPs/caip-222.md Outdated Show resolved Hide resolved
CAIPs/caip-222.md Outdated Show resolved Hide resolved
CAIPs/caip-222.md Outdated Show resolved Hide resolved
CAIPs/caip-222.md Outdated Show resolved Hide resolved

```
signatureTypes: {
"eip155": ["eip191", "eip1271" ],
Copy link
Collaborator

Choose a reason for hiding this comment

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

OK added a sentence to the definition above, take a look

@@ -45,7 +46,7 @@ The application would interface with a provider to make request as follows:
"params": {
"cacaov": string,
"type": string,
"chainId": string,
"chains": string[],
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think chainIds is the dapp telling the wallet which kinds of addresses it WANTS exposed (and implicitly, has use for). it might even be worth mentioning (in ### Security Considerations, if nowhere else) that a wallet COULD return addresses not on any of the requested chains (or choose which addresses to return if chains array was empty), but this is unspecified behavior and may lead to confusing UX.

my thought on an empty chainId is that it kinda translates to "any EVM keypair, I don't care" if combined with a set of signatureTypes that are all EVM-equivalent ( "eip155":{'eip191', 'eip1271'}, "polkadot":{'moonBeamSign'}", ... ). if emptyChainId but also un-specified signatureTypes.... I would just drop as a wallet, there's no way of knowing what that means.

not sure all this is worth putting INTO the spec, but it's the corner cases that come to my mind so far.

ganchoradkov and others added 5 commits November 4, 2023 12:28
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
Co-authored-by: Bumblefudge <caballerojuan@pm.me>
@bumblefudge bumblefudge merged commit e31af54 into ChainAgnostic:main Nov 10, 2023
@ganchoradkov ganchoradkov deleted the feat/wallet-authenticate-multi-chain branch November 10, 2023 14:06
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.

6 participants