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: add initial version of Domain Wallet Authentication CAIP #275

Merged

Conversation

FedericoAmura
Copy link
Contributor

@FedericoAmura FedericoAmura commented Apr 29, 2024

This CAIP is an identity standard. It proposes a method to associate crypto-domains and other account names with authorization providers in order to provide a portable web3 experience.

@bumblefudge
Copy link
Collaborator

bumblefudge commented May 8, 2024

Hey there-- sorry for the slow response time, I have been AFK for 2 weeks straight.

This definitely sounds like a CAIP that people would use, and I am glad to get this merged as a Draft soon so that people can see it's in progress/prototype phase and chime in from ENSDAO and other NS communities and ecosystems. My only requests before it be merged as draft is that:

  1. we clean up the formatting and terminology a tiny bit to maximize likelihood of good feedback coming from casual readers/reviewers, and
  2. you make some decisions about chain-specificity, both within and beyond EVM
  3. be more explicit about the keys in authFlows.platform -- are browser, mobile, and wc the only three options, and this CAIP an exhaustive list of those types? is there a registry somewhere? are a consenting dapp and a consenting wallet free to invent a fourth option in the privacy of their own home? can a 4337 extension EIP be drafted tomorrow that adds a fourth for onchain wallets? i don't have strong opinions about what the answers to these questions should be, as long as they are contained in this spec!

every other in-line comment and suggestion can be considered non-urgent and picked up later if you're in a hurry to merge! speaking of non-urgent, i'd also recommend you put each sentence on its own line, it helps a lot with wordsmithing through github, as diffs display as whole sentences :D

Copy link
Collaborator

@bumblefudge bumblefudge left a comment

Choose a reason for hiding this comment

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

Quick initial review-- apologies if it's a little inconsistent or incomplete!

CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
CAIPs/caip-x.md Outdated Show resolved Hide resolved
glitch003 and others added 4 commits May 9, 2024 20:16
Co-authored-by: Bumblefudge <[email protected]>
Co-authored-by: Bumblefudge <[email protected]>
Co-authored-by: Bumblefudge <[email protected]>
Co-authored-by: Bumblefudge <[email protected]>
@arein
Copy link

arein commented May 12, 2024

Can y'all please add sequence diagrams so network calls & context switches become more clear?

@arein
Copy link

arein commented May 12, 2024

How does this handle multiple accounts? Would this require me to have multiple profiles? How does it handle if my account resolves to different addresses on different chains which is a more emerging scenario with the rise of contract accounts?

@davidlsneider
Copy link

How does this handle multiple accounts? Would this require me to have multiple profiles? How does it handle if my account resolves to different addresses on different chains which is a more emerging scenario with the rise of contract accounts?

We discussed this extensively and decided that this is best handled by the authenticator, if a user has multiple accounts with a given authenticator. In other words, the spec calls for associating one authenticator with a given name and then that authenticator can have multiple accounts associated with it. For example, when metamask opens, a user can then pick a wallet from there.

…ing, sequence diagram, chain id and further explanation of involved topics
@pedrouid
Copy link
Member

@davidlsneider as you are aware I'm a big fan of this concept but I'm concerned about the authflows being too generic or not descriptive enough

Describing "extension", "mwp" and "wc" is not sufficient to establish a connection and it will essentially create more problems than it solves

I think this CAIP requires us to put together a consistent experience between these authflows such that it's "automatically" discovered rather than communicated as a response

My proposal is that we create a new CAIP that emulates the EIP-6963 experience but make it chain-agnostic and potentially leverages web standards such as window.postMessage for discoverability

Therefore you can overlap between wallets present in the web plus wallets associated with the identifier

Untitled - Frame 1 (1)

@davidlsneider
Copy link

My proposal is that we create a new CAIP that emulates the EIP-6963 experience but make it chain-agnostic and potentially leverages web standards such as window.postMessage for discoverability

Pedro, thanks for your feedback! Your point is understandable, which is that ‘extension’ is not enough and has created problems for desktop users which are addressed with EIP 6963.

The main intent of this CAIP is to solve for portability, which means that all interfaces should be compatible. For example, mobile apps don’t support window.postMessage. You’re right that specifications per interface type will make this cleaner for users.

One change that'd be great to add to this Domain Wallet Authentication method is that when an authentication provider is resolved via this CAIP and the "extension" connection type is available for that platform, then the dApp should check the available EIP 6963 providers for a match. And if one is found (aka, the user has the extension installed) then the dApp talks to it via EIP 6963. The dApp calls eip6963:requestProvider and the extensions returns the provider in the response.

@bumblefudge
Copy link
Collaborator

bumblefudge commented May 23, 2024

Hey all, sorry to be offline for so long, was on (actually still am on) a mini-sabbatical.

I share Pedro's concern that this flattens categories a bit, and I think the success or failure of the endeavor hinges on how tightly specified each authFlow "type" is. Without diving too far into hypotheticals or doing any research, my first impulse is-- why "extension" and not two different types, "eip1193" and "eip6963"? what exactly is the type "wc"-- is it wc v2 (i.e. caip25)? or is it something more general, that includes wcv1 connections (regardless of whether WC the network/company still supports the legacy form of its protocol)? I think I would be supportive of merging this as "status: draft" but I don't really think it's safe or constructive to bump that status to "review" or "final" until we've got multiple "interfaces" (native mobile, wcv2/caip25, extension) specified in detail with sample/test dapps (or at the very least test vectors) to show them working well with, e.g., EIP-6963, WC, MM, etc.

In other words, if people want to implement this and take it all the way, it looks like a CAIP to me, but Pedro's advice on how to do that strikes me as fairly apt!

@bumblefudge
Copy link
Collaborator

p.s. really excited for the authflow type "wearables"😉

Copy link
Collaborator

@bumblefudge bumblefudge left a comment

Choose a reason for hiding this comment

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

I think this makes sense as a working draft to invite review and contribtion, but it won't really be done until there are multiple authflow types specified, as I said in more detail above!

@obstropolos
Copy link
Contributor

Second - it meets the criteria to be approved as a draft but welcome further review as it goes through the process here.

@bumblefudge bumblefudge merged commit 0c972de into ChainAgnostic:main May 23, 2024
@Sednaoui
Copy link

Great initiative. My main concern is the optional chain field when resolving a domain name. Domain names has properties to resolve to different addresses, not just a single address on a single network.

Take ENS as an example. ENS allows a domain name to resolve to different addresses. This means I can resolve a domain name to an eth (mainnet) address, and also to an bitcoin/arbitrum/optimism address. Unfortunately, everyone (libraries, developers, etc..) defaults to resolve the name to the same address on all EVM chains, which is inaccurate for smart contract wallets, as their ownership is not defaulted to all chains.

Having the chain field as optional may be the norm, where no one includes the chain field, leading to connectivity problems with smart contract wallets.

@FedericoAmura
Copy link
Contributor Author

Great initiative. My main concern is the optional chain field when resolving a domain name. Domain names has properties to resolve to different addresses, not just a single address on a single network.

Take ENS as an example. ENS allows a domain name to resolve to different addresses. This means I can resolve a domain name to an eth (mainnet) address, and also to an bitcoin/arbitrum/optimism address. Unfortunately, everyone (libraries, developers, etc..) defaults to resolve the name to the same address on all EVM chains, which is inaccurate for smart contract wallets, as their ownership is not defaulted to all chains.

Having the chain field as optional may be the norm, where no one includes the chain field, leading to connectivity problems with smart contract wallets.

Hi Sednaoui! We just opened PR #292 adding some more about how to treat the chain field and how it affects smart wallets

@octavio12345300
Copy link

Aprovado

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.