-
Notifications
You must be signed in to change notification settings - Fork 57
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
Fix handling of custom service properties #413
Conversation
Signed-off-by: Frank Hinek <[email protected]>
TBDocs Report 🛑 Errors: 0 @web5/api
@web5/crypto
@web5/crypto-aws-kms
@web5/dids
TBDocs Report Updated at 2024-02-16T09:09:30Z |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
==========================================
- Coverage 93.37% 93.33% -0.05%
==========================================
Files 79 79
Lines 23776 23782 +6
Branches 1894 1890 -4
==========================================
- Hits 22202 22198 -4
- Misses 1534 1544 +10
Partials 40 40
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good!
I believe it should look like {
id : 'did:dht:5cahcfh3zh8bqd5cn3y6inoea1b3d6kh85rjksne9e5dcyrc1ery#dwn',
type : 'DecentralizedWebNode',
serviceEndpoint : 'https://example.com/dwn',
enc : 'enc',
sig : 'sig',
} {
id : 'did:dht:5cahcfh3zh8bqd5cn3y6inoea1b3d6kh85rjksne9e5dcyrc1ery#dwn',
type : 'DecentralizedWebNode',
serviceEndpoint : 'https://example.com/dwn',
sig : ['auth', 'assert'],
} since the identifiers are |
With custom properties it doesn't seem like we should have special handling for the values. At least that's how I've been thinking about it. In other words, there's nothing to say that the values of custom properties are referring to an |
@frankhinek agree if they do not reference values in the doc itself then no issue |
Are you thinking there should be logic to check the DID document to try and figure out whether the values are referring to other document properties? It seems like that could be challenging to do reliably. |
the guidance is currently only present around keys, but I'd like to update this to recommend fully qualified identifiers for all properties https://did-dht.com/#representing-keys, I'll open an issue for this. generally I think we should avoid fragments with # since - at least to me - they only make sense in the context of a full identifier |
Signed-off-by: Frank Hinek <[email protected]>
* Fix handling of custom service properties --------- Signed-off-by: Frank Hinek <[email protected]>
* Fix handling of custom service properties --------- Signed-off-by: Frank Hinek <[email protected]>
* Fix handling of custom service properties --------- Signed-off-by: Frank Hinek <[email protected]>
This PR will:
Enable the use of custom service properties such as
enc
andsig
in the examples below:Add test coverage for handling string and array custom service properties.