Revisit plugin installation API surface

[danfinlay]

Using string concatenation is a bit awkward. Let's explore alternative plugin installation semantics.

Raised by @whymarrh, but I'd known someone would raise it eventually ;)

[rekmarks]

I think my preferred solution would be to keep the namespaced permissions and define a method that looks like:

ethereum.send('wallet_requestPlugins', [{
  pluginGUID1: { ...stuff },
  pluginGUID2: { ...stuff },
}])

We could also use that method and have a single permission for all plugins, that identifies the specific plugins using a named caveat:

{
  name: 'pluginIdentifiers',
  type: 'someType', // this depends
  value: [ pluginGUID1, pluginGUID2 ]
}

Did you have any other ideas?

[whymarrh]

From Plugin API § Requesting a Connection to a Plugin:

When you request the permission to contact a plugin (as if it were a restrictedMethod named wallet_plugin_${pluginOrigin}), the user is prompted with a permission. If they consent, the plugin is installed if it is not already, and the application is now able to send messages to the plugin via that same method name.

```js // Assume an async function so I can use await: async () => { // In development your plugin URL is probably just on localhost: const pluginUrl = 'http://localhost:8080' const pluginNamespace = `wallet_plugin_${pluginUrl}` // First request permission to send messages to the plugin: await ethereum.send({ method: 'wallet_requestPermissions', params:[{ [pluginNamespace]: {}, }], }) // Once that namespace has been permitted, you can send requests to it! const response = await ethereum.send({ method: pluginNamespace, params: [yourPluginWillReceiveThisObject], }) console.log(response.result) }() ```

A few scattered thoughts:

1. String concatenation does seem a touch awkward/inelegant
2. Plugin installation seems to be coupled with requesting permissions rather than the inverse, which seems a bit odd because install-time permissions are optional
3. I feel like multiple "installations" of the same plugin should be do something different? As in, how do I know a 2nd "install" has worked? If "the plugin is installed if it is not already," how does one update their plugin?

Also, what's the rationale behind allowing for multiple plugin installs at the same time? How would that be represented to the user? Multiple different sets of permissions feels like an easy way to confuse a user.

A rough sketch of a possible alternative:

// There exists some entrypoint for the plugin, this is the package.json file (?)
const pluginEntrypoint = 'http://localhost:8080/package.json'
const pluginHash = 'foo'
const pluginInstallPermissions = {}

// 1. Install the plugin with optional install-time permissions, returning some ID for this particular install and maybe even some indication that the user didn't agree to install/consent to install-time permissions
const pluginId = await ethereum.send({
  method: 'wallet_installPlugin',
  params: [pluginEntrypoint, pluginHash, pluginInstallPermissions]
})

if (!pluginId) {
    // Handle this?
}

// 2. Send a request to the plugin ID
const response = await ethereum.send({
  method: 'wallet_invokePlugin',
  params: [pluginId, {
    method: 'foo',
    params: { /* */ },
  }],
})

This surfaces the reality that this is RPC wrapped in RPC (right?) but maybe this drops functionality that's implicit in the old API? I'm spitballing here.

[rekmarks]

@whymarrh, in our current model, as far as the extension is concerned, a plugin is two things:

1. A manifest file (package.json)
2. A bundle pointed to by the manifest

The manifest is the entry point. Ideally, all things like hashes and so on are contained in the manifest. The URL of the manifest is currently the plugin's identity, but over time, I think we want to move away from that, and let the manifest identify the plugin, perhaps just with an NPM name.

My only problem with your proposed flow is that it requires multiple sends. Of course, we could wrap the calls in a convenience function, but we really want a request for a plugin to be a one-and-done thing for the dapp. The wallet_requestPermissions API is already completely novel, and the less steps we add, the better. That's why plugins are currently installed as a result of the user approving a dapp's permission to use a plugin.

On the other hand, I think we are on some level trying to have our cake and eat it, too. The work of installing plugins is vastly different from everything else that the extension does when it handles a permissions request. I think tightly coupling the two - like we currently do - could both be bug-prone and cause bad UX.

Perhaps the best way to do is to have cleanly separated methods, and then expose a convenience function on the external provider that just makes sequential calls. (Spoiler alert, that's how LoginPerSite handles ethereum.send('eth_requestAccounts')/ethereum.enable().) Making that split is a longer-term solution, but, to expand on your snippet:

const manifest = await fetch('http://localhost:8080/package.json')

// 1. Request permissions
const permissions = await ethereum.send({
  method: 'wallet_requestPermissions',
  params: [{
    [manifest.web3Wallet.id]: {},
    // ...
  }]
})

if (!gotDesiredPermissions(permissions)) {
  // handle this
}

// 2. Install plugin
const pluginId = await ethereum.send({
  method: 'wallet_installPlugin',
  params: [pluginEntrypoint, pluginHash, pluginInstallPermissions]
})

if (!pluginId) {
    // Handle this?
}

// 3. Send a request to the plugin ID
const response = await ethereum.send({
  method: 'wallet_invokePlugin',
  params: [pluginId, {
    method: 'foo',
    params: { /* */ },
  }],
})

It wouldn't be the end of the world to create a ethereum.connectWithPlugins() that does steps 1 and 2 sequentially. Or maybe there is no way to couple these things that isn't bug-prone? Or maybe we don't reject all permissions if installing the plugin fails? Just spitballing here.

I really like wallet_invokePlugin, by the way.

[rekmarks]

I took a stab at rewriting this last week. @danfinlay expressed a desire to keep plugin installations as part of the permission request approval process. I would like for us to do so, but in working through this problem, I have come to believe that it would be a poor design.

In our current design, if an approved permissions request includes permissions to interact with plugins, we attempt to install the plugins. The problem is that the developer can tell which permissions they received, but not whether any plugins were successfully installed.

Let's delineate the steps involved for the extension background when it receives a request for wallet_requestPermissions:

1. Receive permissions request
2. Ask user for approval a. Notify requester of success or failure
3. Attempt to grant the approved permissions a. Notify requester of success or failure
4. For each plugin: a. Attempt to fetch the plugin b. Attempt to install the plugin c. Attempt to grant permissions for the plugin

Collectively, we can refer to 4a through c as adding a plugin. Adding a plugin fails or succeeds silently.

When the user calls a method—any method—they generally expect it to succeed or fail atomically. When a method does fail, they expect to know what the problem was. If we want wallet_requestPermissions to grant permissions and install plugins, we will have a hard time meeting those expectations.

If failure occurs somewhere in steps 1 through 3, everything is rolled back, and failure is atomic. On success, the approved permissions are returned. If plugin addition is also attempted, our responsibilities are:

1. If plugin addition succeeds a. Notify the requester of which permissions were granted and which plugins were added
2. If plugin addition fails a. Notify the requester of which permissions were granted, which plugins were added, and which plugins were not added and for what reason

Because a single Promise can only resolve once, we either have to change the response type of wallet_requestPermissions or use JSON RPC notifications to forward plugin-related errors to the requester. The former is abominable to me because it seems completely counter to consumer expectations, the latter because it’s extremely hard for the consumer to work with.

The permissions system should only be used to expand the capabilities of a domain. It should not concern itself with the success or failure of adding plugins. wallet_requestPermissions should only concern itself with what its name suggests: permissions requests.

Therefore, I believe that we must add a method like wallet_installPlugins that handles plugin installation:

ethereum.send(
  ‘wallet_installPlugins’,
  [{ plugin1: plugin1Details, plugin2: … }],
) => Object<string, PluginObject>

However, we can have our cake and eat it it too, by adding a catch-all/convenience RPC method that does it all. In my previous work, I implemented it on the inpage provider, but it could just as well be implemented in the background. With that, allow me to introduce wallet_connect (name WIP):

ethereum.send(‘wallet_connect’, params) => {
  accounts: Array<string>, // accounts array as normal, if ‘eth_accounts’ permission granted
  plugins: Object<string, PluginObject>,
  permissions: Array<IOcapLdCapability>, // permissions object array as normal
}

Now, plugins: Object<string, PluginObject> is a mapping from plugin IDs (already known to the requester) and objects describing the plugin that was added. If the plugin was not added, the object has an error property explaining why. Note that the properties match the return types from eth_accounts, wallet_requestPermissions, and wallet_installPlugins, respectively. This return format is extensible, and we can add whatever else we can’t currently anticipate to it in the future, without breaking changes.

Thoughts and feedback greatly appreciated.

[danfinlay]

@danfinlay expressed a desire to keep plugin installations as part of the permission request approval process.

I'd like to clarify that my desire is not about keeping anything as it is, and more about "let's reason about any improvement more carefully, and make sure we've agreed on an approach before merging anything".

I believe that we must add a method like wallet_installPlugins that handles plugin installation

This follows well from the problems you laid out.

allow me to introduce wallet_connect

I like this because it achieves a goal I was also trying to express formerly, which is the desire for a "one call setup" for apps. It doesn't need to be requestPermissions, and in fact it's great that we can step one layer above the permissions request, allow it to execute in parallel, and let the application resume only once it's ready.

We could also rename the key plugins to snaps or dependencies, if we wanted to.. achem... avoid.. certain policies barring plugins.

Also, obviously walletConnect is sensitive as it's already a trademarked name from another team. Maybe wallet_enable, where the old ethereum.enable() method could pass its options through transparently, adding a dash of extra prettiness.

Some questions

• Would the permissions object still include plugin permissions? Maybe this is a separate discussion, but since we allow plugins to already enforce their own policies, maybe it's safe for us to allow them to freely receive connections, as long as they can throw an error that is indistinguishable from non-presence (our methodNotFound error?)
• I wonder if there's an opportunity in here to also request the capnode/capTP api object/presence from the plugins in the PluginObject. What are you thinking PluginObject consists of? Is it somewhat of an options-placeholder, like our IOcapLdCapability class is? I guess there is, since like you say, this is very extensible.
• If the permissions object still works the same way, it seems like we did not necessarily improve the developer experience point that @whymarrh brought up about how weird it is to concatenate a string to form a method. Maybe we could treat the plugins field as an implicit permissions request, and drop it from the permissions object entirely.

The PluginObject

Some proposed parameters

• required (default: true): Could allow the call to resolve without waiting for a particular plugin or permission to finish installation or be approved.

Conclusion

I like it! I think I can get behind this.

[danfinlay]

Maybe we should pass our error object into plugins as an endowment. Every single example starts by requiring it. This is a lot of repeated code... or "Errors", the snap?

[rekmarks]

Thanks for the feedback!

Convenience Method Name

Also, obviously walletConnect is sensitive as it's already a trademarked name from another team. Maybe wallet_enable, where the old ethereum.enable() method could pass its options through transparently, adding a dash of extra prettiness.

I was considering wallet_enable, and I think it’s ultimately a fine, uncontroversial choice. I’m down to stick with that.

What's the PluginObject For?

I wonder if there's an opportunity in here to also request the capnode/capTP api object/presence from the plugins in the PluginObject. What are you thinking PluginObject consists of? Is it somewhat of an options-placeholder, like our IOcapLdCapability class is? I guess there is, since like you say, this is very extensible.

Yup, it’s mostly an extensible placeholder. The absence of an error property on the PluginObject indicates that you’re good to go, and then we can tack on whatever else we want, like the precise version of the plugin, a content hash, where we ultimately fetched it from, etc. etc. Getting the API object/presence on that object definitely seems like a good use to me.

What do Plugin Permissions Look Like?

Would the permissions object still include plugin permissions? Maybe this is a separate discussion, but since we allow plugins to already enforce their own policies, maybe it's safe for us to allow them to freely receive connections, as long as they can throw an error that is indistinguishable from non-presence (our methodNotFound error?)

See the next answer for my complete thoughts on this question. Before moving on, I just want to note that I don’t think it’s safe to allow plugins to receive messages without explicit permission. I can see us concluding that it is safe at some point in the future, but there are at the very least privacy concerns of the kind that informs EIP 1102.

If the permissions object still works the same way, it seems like we did not necessarily improve the developer experience point that @whymarrh brought up about how weird it is to concatenate a string to form a method. Maybe we could treat the plugins field as an implicit permissions request, and drop it from the permissions object entirely.

So, I still think that we need plugin permissions. I agree that we don’t want consumers to have to concatenate strings to request them. Ideally, they simply indicate that they want some plugins, then list the plugins by name/GUID. So, how do we accomplish that?

Under the hood, we’ll have to either use namespaced permissions (i.e. concatenate strings) or construct a caveat. We can do either regardless of the API.

Your suggestion of an implicit permissions request is definitely a viable path. The other would be to use the permissions request object format that I’ve sometimes been a little annoyed by, but am now concluding was smartly designed to be extensible (by you, incidentally). That could look like this:

ethereum.send(‘wallet_requestPermissions’, [{
  foo: {},
  bar: {},
  wallet_plugins: {
    ipfsPlugin: { /* stuff */ },
    starkWarePlugin: { /* stuff */ },
  },
}])

I really like that, actually. It's not even breaking. It's like our past selves handed us the solution. :pray:

Proposed PluginObject parameters

required (default: true): Could allow the call to resolve without waiting for a particular plugin or permission to finish installation or be approved.

So, if set to false, we'd attempt to add the plugin, but the request resolution would occur without regard for that process?

I'm thinking, if you're requesting a plugin, it better be required 👊

Besides, in this model, you can always break up the flow and call wallet_requestPermissions and wallet_installPlugins separately!

[danfinlay]

I'm thinking, if you're requesting a plugin, it better be required 👊

We already have a bit of a contradiction to that opinion in our notion of "unselectable" permissions. We've laid a path where an application may ask for one set of things, and the user may accept a subset of those instead.

Imagine a future where we add a justification field to the permissions request options object, and an application can display the reason that the application is giving for each permission it requests alongside the control to toggle/attenuate that permission.

In this way, a user might peruse the requested permissions (and plugins!), and if they did not require a particular feature of the application, they could indicate it by disabling that request at that time.

This could reduce the number of round-trip permissions requests, where an application that is trying to avoid asking for an excessive set of mandatory permissions requests finds itself indicating for many of its features "please accept this next prompt to use this feature".

For a more concrete example, a little sci-fi:

# MetaMask Permissions Request

The site at chat-land.eth requests the following permissions:

- [ ] Permission to decrypt messages on your behalf
*in order to enable secure communications on this site*
- [ ] Permission to install the whisper plugin
*in order to perform communications in a peer-to-peer network, not reliant on centralized servers*

[ Reject ] [ Accept ]

[danfinlay]

The rest of the comment btw I'm good with.

I kinda think we could cheaply achieve this effect by just converting it to the current string method:

ethereum.send(‘wallet_requestPermissions’, [{
  foo: {},
  bar: {},
  wallet_plugins: {
    ipfsPlugin: { /* stuff */ },
    starkWarePlugin: { /* stuff */ },
  },
}])

[danfinlay]

(I'm in favor of cheap superficial implementations at this early stage when we might still want to change how it works more)

[rekmarks]

Re: the 'required' flag for permissions/plugins: that's fair, I'm okay with that!

I kinda think we could cheaply achieve this effect by just converting it to the current string method:

You mean to say, we can use the API I suggested, but just convert to namespaced permissions when we receive the request for now (rather than constructing a caveat)?

I guess we'll see what @whymarrh says on Monday before I'm off to the races with this.

[whymarrh]

Lots of good ideas here—I think @rekmarks has done a great job of describing the problems and how the solutions work.

(I'm in favor of cheap superficial implementations at this early stage when we might still want to change how it works more)

I guess we'll see what @whymarrh says on Monday before I'm off to the races with this.

I too am in favour of "cheap superficial implementations" in the spirit of iteration and trying on a few different APIs for style. I'm not sure what here would be implemented first, but as long as we're open to changing things I think we can start implementing ideas.

I like #87, I think it's a good approach. I like wallet_installPlugins. I think having some sort of wallet_invokePlugin-type thing avoids needing to concatenate any strings.

[rekmarks]

Thanks @whymarrh for the feedback. I agree with you re: wallet_invokePlugin. I think the next step is to update #87 per this discussion. Here's what I plan to do.

The Plan

1. Replace ethereum.authorize(...) with ethereum.send('wallet_enable', { ... }) as defined above, preserving the one-function-call connection for dapps
2. Largely keep the plugin installation flow defined in #87, including wallet_requestPermissions and wallet_installPlugins, updating its internals as necessary.
3. Adopt the required flag for permissions, passed to the object value of the permission name keys in the wallet_requestPermissions param
4. Implement the wallet_invokePlugin RPC method
5. Adopt the new wallet_plugins parameter format for wallet_requestPermissions:

   ethereum.send(‘wallet_requestPermissions’, [{
   foo: {},
   bar: {},
   wallet_plugins: {
   ipfsPlugin: { /* stuff */ },
   starkWarePlugin: { /* stuff */ },
   },
   }])

6. Similarly, we have to change the return value of e.g. wallet_getPermissions. We'll simply destructure namespaced permissions before returning them, so that they match the parameter passed to wallet_requestPermissions, i.e.:

   ethereum.send('wallet_getPermissions') => {
   foo: {},
   bar: {},
   wallet_plugins: {
   ipfsPlugin: { /* stuff */ },
   starkWarePlugin: { /* stuff */ },
   },
   }

Tentative Items

• If it makes sense, I'll add a wallet_getPlugins RPC method for completeness

Note that items 4 through 6 completely abstract away permission namespacing from the client. By "namespacing," I mean those permission that go through a special namespaced workflow in rpc-cap, not e.g. wallet_-prefixed internal permissions.

Miscellanea

For completeness, here's the signature of `wallet_installPlugins:

ethereum.send(‘wallet_installPlugins’, [{
  plugin1: PluginParams,
  plugin2: PluginParams,
}]) => Object<string, PluginObject>

PluginParams and PluginObject will follow the Object format pattern established by wallet_requestPermissions/wallet_getPermissions

Closing Thoughts

Please let me know if anything is missing from this list or if you have any other feedback at this time! @whymarrh @danfinlay