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

Error on wallet_requestSnaps #193

Open
vandan opened this issue Mar 1, 2024 · 2 comments
Open

Error on wallet_requestSnaps #193

vandan opened this issue Mar 1, 2024 · 2 comments

Comments

@vandan
Copy link
Contributor

vandan commented Mar 1, 2024

Looks like there is an issue with the wallet_requestSnaps method (possibly with the format of the example)

From @Montoya

It’s this one: https://docs.metamask.io/wallet/reference/wallet_requestsnaps/
If you click Send Request you get an error, I think because the parameters are formatted wrong

@vandan vandan changed the title Error on Error on wallet_requestSnaps Mar 1, 2024
@shanejonas
Copy link
Contributor

shanejonas commented Mar 8, 2024

Here is what it looks like in the current docs as by-position:

image

if I just add by-name, we get this:

image

which isn't correct.

in JSON-RPC you should really be able to swap between by-params and by-name, but the way that the method for wallet_requestSnaps is built now it does not conform to the by-name spec for JSON-RPC

by-name: params MUST be an Object, with member names that match the Server expected parameter names. The absence of expected names MAY result in an error being generated. The names MUST match exactly, including case, to the method's expected parameters.

Source: https://www.jsonrpc.org/specification#parameter_structures

Currently each "npm:@metamask/example-snap" is in the place of the parameter name.

My recommendation here is that the implementation for wallet_requestSnaps should be by-params:

await window.ethereum.request({
  "method": "wallet_requestSnaps",
  "params": [
    {
      "npm:@metamask/example-snap": {},
      "npm:fooSnap": {
        "version": "^1.0.2"
      }
    }
  ]
});

This also lines up with the rest of our wallet API (except for the mistake that was wallet_watchAsset that got out with by-name).

@shanejonas
Copy link
Contributor

shanejonas commented Apr 30, 2024

we removed the snap methods in this PR because they are dynamic by-name, which isnt supported, the downstream tools needs some work to allow that to happen. maybe an x- extension like x-dynamic-by-name: true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants