-
Notifications
You must be signed in to change notification settings - Fork 76
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
add support for all DID doc updates from latest DID core spec #110
Comments
This seems to be reasonable. The PR should then also clean up the DID ETHR spec. Although ideally, we would just use raw bytes instead of strings to be more flexible but I guess that would be a more breaking change. Then we could reserve 1-3 bytes for the different options and maintain a list in the DID ETHR spec for the mapping. |
I agree that raw bytes would fit the bill in terms of efficiency, but would be a tiny bit harder to implement. I guess the string solution is "close enough" The thing that is hardest for me to accept is the |
@mirceanis Is anyone specifically asking for JWK? The spec does not mandate us to support it. IMO, PEM is even worse. We could say we only support base58 or multibase (depending on where the DID Spec lands). |
Good question. I suppose there are no direct requests for JWK, nor for PEM (which, I agree, is even worse). Alternatively, we could provide workarounds that are explicitly called "expensive" to maybe deter folks from using them.
|
Couldn't we provide a utility function that converts b58 -> JWK? I don't think a lot of ppl are using PEM anyways. |
Of course we could, but the question is if this kind of conversion should be part of the spec or not |
@mirceanis @awoie We currently facing the case where we would need to use the initial key of a newly created JSON-LD signing/verification logic plays well here only if we have It's kind of a show-stopper for us now. The main problem is that the key of an initial key-pair of |
@AlexYenkalov if you are starting from a keypair, then you can use The default document for a |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@mirceanis I'm going to fund this work. Is there general agreement on how to move forward updating ethr-did and ethr-did-resolver? Are you the best point of contact for my team to contact? |
@tankcdr I'm your contact, yes. There were some unanswered questions about key encoding. For efficiency and cost reasons, keys should be stored in their raw byte form in the EVM events, and it would be up to the resolver to interpret them back into jwk, pem, base58, etc encodings after they are interpreted. This means that we would need to reduce the possible encodings to a minimum. |
@mirceanis i am working with @tankcdr to work on this enhancement. Can we connect this week? |
I'm unable to commit to a schedule in the next few weeks because I recently went on paternity leave, but I will try to read proposals async, if you make them. Don't count on timely replies from me yet :) Regarding the unanswered questions I posted before, |
@mirceanis Hey Mirceanis. I noticed that in the latest W3C core spec, there are two more keys (controller & alsoKnownAs) which are not shown in the suggested DOCUMENT_SECTION_FLAGS scheme above. Can I ask why and will these two also be added? Looks like someone has asked the similar question for "alsoKnownAs"on another ticket (decentralized-identity/veramo#260) which has been close. |
Hi @leowcy , There is a potential use for
|
Hello @mirceanis, Is there any progress on this topic? I need to store bls public keys in did:ethr and I also need the resolver to be able to return this information. |
@veromassera There was no progress on this, but you can still add other key types using the existing resolver code. See this test case as an example:
|
Thank you very much for the example. |
You're welcome! It's still not a complete solution, though, as it won't work with all key type / verification relationships. The example I posted can't be used to add a key to the |
Reposting the proposal from #99 (comment) as a separate issue:
The current
DIDAttributeChanged
scheme used to populate the DID document is insufficient for covering all the cases described by the more modern versions of the DID spec.@AlexYenkalov has suggested a naming scheme that uses a set of characters to describe where a key should be placed.
Besides this, the
did/
prefix can also be shortened (or dropped?) to make room for more expressive future additionsSo, instead of trying to match the attribute name against:
/^did\/(pub|auth|svc)\/(\w+)(\/(\w+))?(\/(\w+))?$/
the resolver would look for something like this
/^d\/([vaukdis]*)\/(\w+)(\/(\w+))?$/
Examples:
verificationMethod
and also be listed in theauthentication
,assertionMethod
,capabilityDelegation
andcapabilityInvocation
sections, the attribute name would start withd/vaudi/
keyAgreement
key is added, the attribute name would start withd/vk/
d/s/
This can be made even more efficient by recognizing that keys would always be listed in the
verificationMethod
array and referenced from the other sections, so the/v/
flag can be dropped and considered implicit as long as the/s/
flag is not used.There is also the problem of the types of
verificationMethod
s that can be listed.The names of the methods listed in the DID spec registries are quite long, so they would not fit into the 32-byte limitation of the attribute name.
To get around this problem, I propose implementing a mapping to shorter strings:
If the list of these methods is expected to remain relatively stable, an even more efficient mapping can be created:
Continuing along this line of reasoning, the key encoding can be shortened to one of:
There can be an implicit encoding of
base58btc
(publicKeyBase58
) if none is specified.Example:
X25519KeyAgreementKey2019
withbase58btc
encoding, one would publish an attribute with the named/k/x/58
RsaVerificationKey2018
assertionMethod
withpublicKeyPem
encoding, one would publish an attribute with the named/a/r/p
EcdsaSecp256k1RecoveryMethod2020
assertionMethod
andauthentication
withethereumAddress
encoding, one would publish an attribute with the named/au/R/e
Any thoughts?
cc @awoie @rado0x54
The text was updated successfully, but these errors were encountered: