Closed kimdhamilton closed 3 years ago
I have been leaning more towards option 2
...
essentially any universal wallet plugin can define new data models, "json-ld objects" and use new methods to generate them, and then add
them to wallet.contents
.
because storing everything in a giant array probably won't scale, interfaces can be "read" or "write" oriented...
for example:
fancyWallet.getCredentialsMatchingMyCustomQuery
=> custom credentials list....
I would expect a plugin to define getCredentialsMatchingMyCustomQuery
and allow it to "connect" to whatever is needed to resolve that request.
this list might then be used to to adjust the current wallet credentials, before a backup occurs, for example.
The main point being that functions are easier to work with than methods, so whenever possible a plugin should avoid calling wallet.contents
directly... there are exceptions to that such as wallet.lock
which needs to do that.
we could remove the concept of wallet.contents from all interfaces, and require it to be explicitly passed... that might make option 2 even better.
I also endorse second option and see initial groundwork in support - https://github.com/w3c-ccg/universal-wallet-interop-spec/pull/20.
Crucially, pluggability bifurcates the wallet, establishing clearer lines between core and non-core. A few benefits:
because storing everything in a giant array probably won't scale, interfaces can be "read" or "write" oriented...
Agreed, and likely a third store-independent orientation - 'compute'.
I took a stab at sorting responsibilities. One thing I didn't highlight in my original comment was that I'm also starting to nudge towards abstractions for plugin groups; e.g. EDV Plugin conforming to a "Storage" interface (similar with did key plugin). I get that it may be premature, and that we may need to iterate on these over time. But interested in thoughts on the general direction.
Oh, also ignore the fact the "VC Wallet" box is separate for now. I'm still thinking through this.
I like the idea of pulling the storage interface out, the lock / unlock feature is currently connected too content
so we would need to define lock / export in terms of query
, and import / unlock in terms of add.
I am more inclined towards second option.
I just went through the spec & demo for the first time and please correct me if my understanding is wrong.
It is mentioned here that,
Interface functions are abstract, they take wallet contents and options as input, and they produce wallet contents and side effects as output.
This is bit confusing since it doesn't look like most of the interfaces need wallet contents as input.
Few of the interfaces in spec looks are more like JS oriented. Lets take an example of query. I am not sure how a wallet written in some other language like Golang or a REST based implementation of universal wallet can support arguments like map
& reduce
.
For example, if I have to add getCredentialsMatchingMyCustomQuery
in my wallet which supports querying credentials using presentation exchange what should be the right approach?
Shouldn't we also need predefined request-response models for each of the interfaces for more clarity and interop?
@sudeshrshetty certainly mosts of the interfaces are meant to be functions, map and reduce functions are convenient ways of handling JSON, but to they might not be needed at all, there could get more advanced query interfaces, for example encrypted data vaults queries are supported by the edv plugin.
If you want to create a new set of interfaces, I recommend you create a new plugin and define the functions in terms of objects, for example:
"send ethereum"
could be (web3, mnemonic, toAddress, fromIndex, amountWei) => ethTransaction
but it could also be (web3, toAddress, amountEth) => ethTransaction
"issue credential"
could be (vcTemplate, signingKey, format) => VC
Why functions?
They are safer from an interop perspective than assuming OOP or method oriented interfaces, for example, I could implement an "issueVc" interface of "keys"... then you could do the following:
const signingKey = await wallet.getKeyById("keyId");
const vc = await signingKey.issueVc(vcTemplate);
while this interface requires a signingKey class to be aware of every possible type of signature (including vanilla and capabilities)....
Imo its better to use functions, for example:
const signingKey = await wallet.getKeyById("keyId");
const vc = await wallet.issueVc(signingKey, vcTemplate);
But i could be convinced otherwise by a concrete proposal for how interfaces should be designed.
In general, I like functional typed interfaces, the actor pattern, etc...
I don't think these interface should be about network requests / response... i think it should be a set of abstract functions, if folks want to implement remote operation with did comm or grpc, how they features are exposed might be different.
My first objective was to explain how private keys are related to currency, capabilities and credentials, and DIDs... by using existing specs.
for example, the same did:key can be used to control ethereum, sign credentials and invoke capabilities. You might imagine constructing a wallet with these features with the builder pattern, for example:
const wallet = wallet.factory()
.addPlugin(ethereum)
.addPlugin(did)
.addPlugin(capabilities)
.addPlugin(credentials)
const txn = await wallet.sendEth(fromAddress, toAddress);
const vc = await wallet.issue(vcTemplate);
const capability = await wallet.delegateCapability(capability);
const invoke = await wallet.invoke(capability);
Shouldn't we also need predefined request-response models for each of the interfaces for more clarity and interop?
Yes, thats the idea by defining the functional inputs, outputs and side effects.
Do you have a preference for how the interfaces should be modeled?
@OR13 I like the approach of using functions.
My concern was regarding using functional inputs and outputs in interfaces. I was thinking about the scenario of interop with REST based wallet. May be I misunderstood the scope of interop here about supporting set of abstract interfaces.
A REST based wallet can not support 'query' interface from spec because of functional inputs parameters. It can support JSON request/response models.
Sorry I’m late to this party. I like @OR13’s framing as a builder pattern to show the idea for assembly of wallet functionality using plugins. ...If taken literally, the immutability of the created object would limit extensibility, i.e. adding pllugins later, but we could think of updating a wallet functionality with a new plugin by “retooling” the factory then rebuilding the wallet object with the original as input. Maybe I’m overthinking this. The main thing is that plug-ins define wallet instance capabilities beyond the core functions of all wallets.
Sent from Yahoo Mail for iPhone
On Tuesday, January 5, 2021, 8:05 PM, Sudesh Shetty notifications@github.com wrote:
@OR13 I like the approach of using functions.
My concern was regarding using functional inputs and outputs in interfaces. I was thinking about the scenario of interop with REST based wallet. May be I misunderstood the scope of interop here.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.
We had a chat about this, and the consensus was the wallet interfaces should be functions of JSON objects, and produce JSON objects.... this means that internally the function may convert the object to a class instance and then use it, but the function inputs and function outputs are limited to things that can be expressed in JSON... if you really want to write a function in JSON, you can... but I don't advise it :)
This issue now needs a PR which updates the spec to reflect this.
Thanks for the PR @OR13.
Is it a good idea to group interfaces in spec by plugins defined here?
Shall I bring back the 'query' interface by defining supported query types & query params. For Ex: if a wallet supports JSON-LD framing
query({type:"QueryByFrame", dataQuery:{""}})
We can also introduce querying by presentation exchange here for VC wallets.
@sudeshrshetty yes, I think we should expose a top level "query by json" object, and then map that to specs.
Related to https://github.com/w3c-ccg/universal-wallet-interop-spec/pull/52
I think we should provide full json documentation for the interfaces described by the spec, this will also help us align the storybook ui with the spec, instead of duplicating examples, we can link directly to the spec.
I will make a PR very soon
I liked the way @OR13 made a point about adding a data model & corresponding interface functions here
So I simplified the interfaces as below,
Storage interface moved out of core wallet interfaces here. Adding/removing data models should be done through corresponding interface functions.
For example: If I am interested in implementing VC wallet only, exposing a generic add
interface will force me to support any data models and also my query
interface will have to support those data models.
Also this will apply to verifyRaw/signRaw
interfaces too, where I can add/verify proofs through issue/prove/verify
interface functions on data models I am interested in and I don't have to expose those generic interfaces.
Also, remove, query
interfaces can be wallet implementation specific.
@sudeshrshetty @OR13 reflecting on our discussion, I'm not sure that verifyRaw
and signRaw
belong in the list of wallet interface functions. Key
seems much more like a supporting entity whose operations shouldn't necessarily bubble up to the wallet. As @sudeshrshetty points out, these operations will generally be wrapped by higher level ones like transfer
or issue
.
On that note, it might be worth distinguishing between auxiliary interfaces like Key and Store (which corresponds to Sudesh's "core" distinction above), vs interfaces providing distinct wallet functionality, e.g. enabling currency operations, VC operations, etc. Now that I think about it, perhaps this is where Sudesh was going with the use of "plugin" here. To help distinguish between that and the use of "plugin" in the reference implementation, I'll use "plugin interface" for what (I think) Sudesh is saying in that diagram.
Continuing down this rabbit hole, I wonder if terms Store vs Storage are causing confusion. There are two distinct store-like concepts that are interesting in the context of wallets -- one corresponding to "backing storage" and the other corresponding to the wallet as an entity that would participate in credential exchange ceremonies. If this concern doesn't make sense, please ignore. I will come back to it later.
Interested in your feedback for at least the first 2 paragraphs. This should give me enough fodder to break apart this issue into more actionable sub-issues, and finally close this.
I don't know the spec, but signRaw
sounds risky. In cryptocurrency wallets you don't want to ever allow arbitrary signing with keys, as it is an attack vector. The signer must parse what is being signed.
Unfortunately I need to bring in "UML" (scare quotes intended).
What we're calling "Key" and "Storage" (we can quibble over names) are "required interfaces" (as opposed to "provided interfaces"). The VC and Currency interfaces are more like wallet mixins.
The specific interfaces in the diagram are worth calling out because they are basic ones enabling our envisioned wallet use cases, but they are not necessarily the only ones of their types. Briefly:
In addition to providing new instantiations, implementors could introduce new interfaces of either category. We can (over time) iterate over discoverability of all of the above in some sort of wallet marketplace.
This distinction between "plugin interfaces" vs "mixin interfaces" (we can also quibble over these names) may be overly pedantic and we may not need to preserve this, but it's
I don't know the spec, but
signRaw
sounds risky. In cryptocurrency wallets you don't want to ever allow arbitrary signing with keys, as it is an attack vector. The signer must parse what is being signed.
As a developer, I DO want to sign raw, especially if I want to support multiple crypto currencies that require signatures over specific payloads, such as ethereum and bitcoin... a wallet interface should not assume your network...
and higher level interfaces are always built from lower level ones.
Its fine to warn a use not to use signRaw unless they know what they are doing... its terrible idea to block it... see WebAuthN signatures... impossible to use them for anything but auth... billed as a "feature" but makes the standard only useful for one thing, and therefore another standard is needed to do other things...
We need to be careful not to take away interfaces that are implied by plaintext private key visibility.... its a huge usability issue.
For example, IF I can see a P-256 curve private key, I CAN:
If a wallet can see this key, a wallet plugin should support these operations...
https://nodejs.org/api/webcrypto.html#webcrypto_sign_and_verify
These interfaces are used to build the higher order interfaces of:
These interfaces are in turn exposed in the yet higher order interfaces for:
Sure the wallet does not need to expose 100% of the interfaces via a UI... but its a mistake to tell developers they are too stupid to use lower level interfaces when they need them, and especially bad idea to make a spec that supports only higher level interfaces... because there will be massive disagreement on the higher level interfaces, but lots of agreement on the lower level ones.
We should probably discuss conformance classes for wallets, and start with the lowest level... something like support for basic crypto operations like web crypto has.... and then build up to DID Wallets which only expose higher order interfaces like
transferCurrency
and issueCredential
... and don't even let a developer pick the crypto that is used to implement these interfaces.
Finally trying to close this out. Some conceptual issues were clarified in other issues &PRs. There are 2 lingering issues that I opened to capture the remaining tasks
@OR13 @sudeshrshetty please let me know if you think there are any additional remaining tasks. Otherwise I think we can close this beast
I am going to close this issue, i think everything has been captured in smaller / more actionable issues.
Context
Reviewing the Universal Wallet spec vs source code, I identified some interesting differences in core wallet operations. Categorizing these differences led to some possible directions for improved clarity in the universal wallet spec.
There is already an issue tracking separation of the data model from the interface (or at least clarity on normativity status). This discussion sort of punts on that for now, pulling in data model and interface concepts in an attempt to gain clarity on what a wallet is.
Goals
Get feedback, develop a plan, and then submit more fine-grained issues/PRs.
Problem Statement
In source code, the Wallet interface is this:
Note that some of these operations -- like lock, unlock, import, and export -- are present in the Universal Wallet (spec) interface, while others (e.g. add, remove) are not.
Following is a visual grouping of methods by:
The trailing comments are where I attempted to group operations by different types of wallet functionality. Some examples:
This pointed to more general questions, such as:
I also added spec Data Model elements in my consideration below.
Option 1ish: A more generic wallet
One path is to consider whether any abstractions covering credential/currency can be bubbled up to something that makes sense at the wallet level, and whether that’s even a worthwhile exercise (it may not be).
As a rough stab, I came up with a wallet managing "identities" and "assets".
These are not clean abstractions because the examples do not fit neatly within.
Option 2ish: A simpler wallet
An alternative we can consider is whether it makes sense to extract any complicated methods like this from wallet, which would end up with different “brands” of wallet using the simpler wallet.
The first example shows a VC Wallet...
Vs a Currency Wallet...
Additional Thoughts
The Universal Wallet code already introduces a notion of plugins for EDV, did key, but where possible, I'd like to start building towards abstractions of these, e.g. storage plugins, key and DID plugins.