Add ERC-7846: Wallet Connection API #779
Conversation
File
|
| type WalletConnectResult = { | ||
| account: { | ||
| address: `0x${string}`; // connected account address | ||
| supportedChainsAndCapabilities: Record<`0x${string}`,any>; // chain-specific capabilities, mirrors ERC-5792 wallet_getCapabilities | ||
| }, | ||
| capabilityResults: Record<string,any>; // results of this connection request's connection capabilities | ||
| } |
There was a problem hiding this comment.
Speaking on behalf of the larger Wagmi consumers, I would really appreciate if this supported multiple Accounts.
| type WalletConnectResult = { | |
| account: { | |
| address: `0x${string}`; // connected account address | |
| supportedChainsAndCapabilities: Record<`0x${string}`,any>; // chain-specific capabilities, mirrors ERC-5792 wallet_getCapabilities | |
| }, | |
| capabilityResults: Record<string,any>; // results of this connection request's connection capabilities | |
| } | |
| type WalletConnectResult = { | |
| address: `0x${string}`; // connected account address | |
| supportedChainsAndCapabilities: Record<`0x${string}`,any>; // chain-specific capabilities, mirrors ERC-5792 wallet_getCapabilities | |
| capabilityResults: Record<string,any>; // results of this connection request's connection capabilities | |
| }[] |
While the point of "By constraining the account return to a single address, this ERC simplifies developer experience while still supporting richer capability results." is made, I don't really see how supporting multiple accounts with their capabilities really degrades developer experience as 99% of the time applications will be using developer tooling that abstracts over these JSON-RPC methods (ie. Wagmi Connectors, etc).
Even if no developer tooling existed, I still don't think DX would be degraded as applications could just use the first item in the array anyway, and ignore the others (this was Wagmi behavior pre-v1). In fact, user experience would be moreso degraded as users would have to "disconnect their current account, and then connect their other account" to achieve previous behavior.
I think capabilities per-account also makes sense, because you wouldn't really be able to specify fine-grained capabilities (per-account) in the request anyway, so it would always be implied that it is batched across all accounts (for example, a SIWE signature for each account).
There was a problem hiding this comment.
Thanks for the thorough feedback! Will reply in your repost on the Eth Magicians forum.
| type WalletConnectResult = { | ||
| account: { | ||
| address: `0x${string}`; // connected account address | ||
| supportedChainsAndCapabilities: Record<`0x${string}`,any>; // chain-specific capabilities, mirrors ERC-5792 wallet_getCapabilities |
There was a problem hiding this comment.
| supportedChainsAndCapabilities: Record<`0x${string}`,any>; // chain-specific capabilities, mirrors ERC-5792 wallet_getCapabilities | |
| chains: `0x${string}`[]; // supported chains |
Should this just be an array of supported chains? Consumers can use wallet_getCapabilities to extract capabilities from a Wallet.
There was a problem hiding this comment.
The intent was to remove the need to make a separate wallet_getCapabilities request. That may not be necessary at API layer though because SDKs could do this automatically for apps.
| address: `0x${string}`; // connected account address | ||
| supportedChainsAndCapabilities: Record<`0x${string}`,any>; // chain-specific capabilities, mirrors ERC-5792 wallet_getCapabilities | ||
| }, | ||
| capabilityResults: Record<string,any>; // results of this connection request's connection capabilities |
There was a problem hiding this comment.
| capabilityResults: Record<string,any>; // results of this connection request's connection capabilities | |
| capabilities: Record<string,any>; // results of this connection request's connection capabilities |
capabilities for consistency? "result" seems implied as the consumer will be extracting it from a response anyway, and we don't call it capabilitiesRequest.
| } | ||
|
|
||
| type SignInWithEthereumCapabilityResult = { | ||
| message: string, // formatted SIWE message |
There was a problem hiding this comment.
| message: string, // formatted SIWE message |
Do we need the message? I thought the wallet would be the one displaying the formatted message.
There was a problem hiding this comment.
Was thinking that the app needs to verify that the message the wallet prompted signing for matches their requested arguments. For instance a malicious wallet could attempt to reuse an old expiry/nonce if it had access to a previously signed SIWE message. The most succinct way to send back all fields and verify against the signature is to pass back the final message
ilikesymmetry
changed the title
ERCS/erc-7846.md
Outdated
| } | ||
|
|
||
| type SignInWithEthereumCapabilityResult = { | ||
| message: string, // formatted SIWE message |
There was a problem hiding this comment.
Might also be nice to add a payload property so consumers don't have to parse the message string to figure out the inferred parameters (e.g. address, domain, etc)?
| message: string, // formatted SIWE message | |
| message: string, // formatted SIWE message | |
| payload: { | |
| address: `0x{string}`, | |
| nonce: string, | |
| ... | |
| }, |
feat: `wallet_disconnect` + structure improvements
|
The commit abd1c9f (as a parent of 39612d2) contains errors. |
|
While I understand the motivation for creating EVM-specific standards for Wallets... there is nothing new or extra that ERC-7846 provides that isn't already covered by CAIP-222 https://chainagnostic.org/CAIPs/caip-222 Why restrict such an important pattern like "Connect + Sign" to only EVM wallets? |
one huge problem still: For passkey wallets, e.g. Signing Up with Coinbase Smart Wallet, you would still need 2 interactions with this ERC. First TouchID to get the Ethereum Address, second one to sign the message. Sign In With Ethereum is not a good solution. The address should not be part of the signed message. It's implicit from the signature. I'd suggest changing this ERC to allow signing an arbitrary message, makes it simpler and leaves the kind of signature to the dApp. |
This proposal defines a new RPC for wallet connection with an emphasis on extensibility. Builds on the notion of optional “capabilities” defined in ERC-5792 to add new functionality modularly. This proposal defines one capability to reduce separate interactions for connection and authentication, but otherwise seeks to leave capability definitions open-ended.