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

CAIP-300: Wallet Connect JSON-RPC Method #300

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lukaisailovic
Copy link

This CAIP defines a JSON-RPC method to request a batch of RPC methods to be resolved when connecting a wallet in a single roundtrip.

{
"method": "wallet_grantPermissions", // ERC-7715 request
"params": {...}
},
Copy link
Member

@ganchoradkov ganchoradkov Jun 28, 2024

Choose a reason for hiding this comment

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

Does this standard allow for personal_sign to be sent via wallet_connect? As at the time of requesting it, the user address is unknown to the dapp

Copy link
Member

Choose a reason for hiding this comment

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

Not really... you need to use wallet_authenticate

@KannuSingh
Copy link

How will the standard handle the request if one of the rpc method requested via wallet_connect is not supported by the wallet and how in the case where account selected by user for approving the wallet_connect request don't support one of the request in the requested batch?

@pedrouid
Copy link
Member

Then the wallet must reject it with 7001 code for invalid method

Or partially reject it with an error message in the batch

@jxom
Copy link

jxom commented Jun 30, 2024

Can we consider adding wallet_disconnect to this proposal? Few reasons:

  • Just like wallet_connect has been a missing RPC method for years, so has a way to programmatically "disconnect" or "remove a host" from the wallet without the user having to do this in the wallet itself.
  • Adding a wallet_disconnect RPC method will further provide strengthened symmetry of this proposal.
  • The absense of programmatic disconnect has been a painpoint in Ethereum libraries which interface Dapps to Wallets for years (have had to "polyfill" the behavior on client-side).
  • EIP-1193 provides "connect" and "disconnect" events, so having both wallet_connect and wallet_disconnect makes sense.

@bumblefudge
Copy link
Collaborator

bumblefudge commented Jul 16, 2024

@lukaisailovic i'm seeing a commit adding jake as author but not one adding a _disconnect method (or for that matter, pointing to the _disconnect method added by the still-open CAIP-285 PR) 😅

holler at me when there's an approval from jake so we can get this merged and visible on the website 😎

@pedrouid
Copy link
Member

@jxom there is only one concern with introducing wallet_disconnect here... the different payloads have different purposes

So is the intention to batch them together as a batch "disconnection"... for example you have wallet_authenticate and wallet_createSession... the former is providing a SIWE signature while the latter is enabling a live session with the wallet... are you disconnecting both??

Also regarding ERC-7715 would the wallet_disconnect perform a wallet_revokePermissions??

Definitely agree that the absence of disconnect as a pain-point for Ethereum but I want to make sure we understand the expected behavior for CAIP-300 since it's batching multiple requests

caip-300.md Outdated Show resolved Hide resolved
Co-authored-by: Bumblefudge <[email protected]>
@bumblefudge
Copy link
Collaborator

i've lost the plot a bit, what are next steps here, @pedrouid ? this looks complete but I vaguely remember this being blocked on some of the non-DOM connection flows or something? Totally fine to switch it to draft if you're waiting for other implementers or open research questions to finish it.

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