-
-
Notifications
You must be signed in to change notification settings - Fork 57
Revisit plugin installation API surface #94
Comments
I think my preferred solution would be to keep the namespaced permissions and define a method that looks like:
We could also use that method and have a single permission for all plugins, that identifies the specific plugins using a named caveat:
Did you have any other ideas? |
From Plugin API § Requesting a Connection to a Plugin:
A few scattered thoughts:
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. |
@whymarrh, in our current model, as far as the extension is concerned, a plugin is two things:
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 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 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 I really like |
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
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 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:
Because a single Promise can only resolve once, we either have to change the response type of 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. Therefore, I believe that we must add a method like 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 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, Thoughts and feedback greatly appreciated. |
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".
This follows well from the problems you laid out.
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 We could also rename the key Also, obviously Some questions
The PluginObjectSome proposed parameters
ConclusionI like it! I think I can get behind this. |
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? |
Thanks for the feedback! Convenience Method Name
I was considering What's the
|
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 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:
|
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:
|
(I'm in favor of cheap superficial implementations at this early stage when we might still want to change how it works more) |
Re: the 'required' flag for permissions/plugins: that's fair, I'm okay with that!
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. |
Lots of good ideas here—I think @rekmarks has done a great job of describing the problems and how the solutions work.
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 |
Thanks @whymarrh for the feedback. I agree with you re: The Plan
ethereum.send(‘wallet_requestPermissions’, [{
foo: {},
bar: {},
wallet_plugins: {
ipfsPlugin: { /* stuff */ },
starkWarePlugin: { /* stuff */ },
},
}])
ethereum.send('wallet_getPermissions') => {
foo: {},
bar: {},
wallet_plugins: {
ipfsPlugin: { /* stuff */ },
starkWarePlugin: { /* stuff */ },
},
} Tentative Items
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 MiscellaneaFor completeness, here's the signature of `wallet_installPlugins: ethereum.send(‘wallet_installPlugins’, [{
plugin1: PluginParams,
plugin2: PluginParams,
}]) => Object<string, PluginObject>
Closing ThoughtsPlease let me know if anything is missing from this list or if you have any other feedback at this time! @whymarrh @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 ;)
The text was updated successfully, but these errors were encountered: