Updates did:dht implementation to the latest spec
#183
Conversation
andresuribe87
requested review from
mistermoe,
nitro-neal,
tomdaffurn and
phoebe-lew
as code owners
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #183 +/- ##
==========================================
+ Coverage 78.96% 79.69% +0.72%
==========================================
Files 33 34 +1
Lines 1930 2088 +158
Branches 265 317 +52
==========================================
+ Hits 1524 1664 +140
- Misses 274 280 +6
- Partials 132 144 +12
|
| @@ -146,11 +154,17 @@ public sealed class DidDhtApi(configuration: DidDhtConfiguration) : DidMethod<Di | |||
| } | |||
|
|
|||
| // map to the DID object model's verification methods | |||
| val verificationMethods = (opts.verificationMethods?.map { (key, purposes) -> | |||
| val verificationMethods = (opts.verificationMethods?.map { (key, purposes, controller) -> | |||
| VerificationMethod.builder() | |||
| .id(URI.create("$id#${key.keyID}")) | |||
| .type("JsonWebKey2020") | |||
There was a problem hiding this comment.
this needs to be JsonWebKey
There was a problem hiding this comment.
Assumed we were waiting as stated in decentralized-identity/web5-spec#73
If we want to include it, I'd rather do so in #181.
|
|
||
| val keyType = when (publicKeyJwk.algorithm) { | ||
| JWSAlgorithm.EdDSA -> 0 | ||
| JWSAlgorithm.ES256K -> 1 |
There was a problem hiding this comment.
could we add ES256 here too?
| add("id=${verificationMethod.id.rawFragment}") | ||
| add("t=$keyType") | ||
| add("k=$base64UrlEncodedKey") | ||
| if (verificationMethod.jsonObject.containsKey("controller")) { |
There was a problem hiding this comment.
probably fine to add this but not in the spec yet
There was a problem hiding this comment.
Yeah, figured it would be added really soon :)
| Name("_aka._did."), | ||
| DClass.IN, | ||
| ttl, | ||
| didDocument.alsoKnownAses.joinToString(PROPERTY_SEPARATOR) |
There was a problem hiding this comment.
if this is a single value what happens?
There was a problem hiding this comment.
didDocument.alsoKnownAses always returns a list.
Join returns the the value of the single element when the list is of size 1.
| Name("_cnt._did."), | ||
| DClass.IN, | ||
| ttl, | ||
| didDocument.controllers.joinToString(PROPERTY_SEPARATOR) |
| @@ -492,22 +585,28 @@ public sealed class DidDhtApi(configuration: DidDhtConfiguration) : DidMethod<Di | |||
| else -> throw IllegalArgumentException("Unknown key type: ${data["t"]}") | |||
| } | |||
|
|
|||
| verificationMethods += VerificationMethod.builder() | |||
| val builder = VerificationMethod.builder() | |||
| .id(URI.create("$did#$verificationMethodId")) | |||
| .type("JsonWebKey2020") | |||
| public val services: Iterable<Service>? = null, | ||
| public val publish: Boolean = true, | ||
| public val controllers: Iterable<String>? = null, | ||
| public val alsoKnownAses: Iterable<String>? = null, |
There was a problem hiding this comment.
is this spelling right?
There was a problem hiding this comment.
I don't know :S
I considered it good enough so it conveys that it has possibly multiple items. Any suggestions you have in mind?
Co-authored-by: nitro-neal <[email protected]>
Overview
Fixes #182 by implementing the updates in the spec.
Description
This includes:
;as a property separator.,as an array separator.verificationMethod.controller.Changes to the public API add the possible new fields, in a way that was consistent with the existing methods.
How Has This Been Tested?
Added fields to the tests.