Skip to content
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

DID DHT Enhancements #334

Merged
merged 3 commits into from
Dec 6, 2023
Merged

DID DHT Enhancements #334

merged 3 commits into from
Dec 6, 2023

Conversation

frankhinek
Copy link
Contributor

@frankhinek frankhinek commented Dec 5, 2023

Summary

This PR primarily addresses Issue #312 by improving the error handling when attempting to resolve a did:dht DID that hasn't yet been published. Additionally, it modifies the key set handling to better align with the original intent and other existing DID method implementations. This was necessary to support the forthcoming switch to did:dht as the default DID for @web5/api package. Lastly it approves test coverage for the DID DHT related code.

Context

As of the creation of this PR, the pkarr library dependency throws an error if the did:dht DID is not found. This could occur because the DID has not yet been published or because a previously published DID is no longer being maintained in mainline DHT. Ideally we could distinguish between error conditions such as an internalError and notFound, but until a better solution is found, this exception is caught and a DID Resolution Result is returned with an internalError.

Issue #331 was created to follow-up on implementing the improvement in the @web5/dids package if/once one is selected.

Changes

@web5/dids

  • Remove extra identityKey property from the DID DHT key set. Unlike DID ION update and recovery keys, all of the keys associated with a did:dht DID reside within the verificationMethods array. As a result, there isn't a need to track this key separately nor duplicate it in two places. The "identity" key is distinguishable due to its well-known 0 key ID.
  • Improve test coverage for did-dht.ts.
  • Minor nit to use expect style testing with stubs to align with monorepo conventions.

@web5/credentials

  • Update DID DHT related test with modified key set structure.

Copy link

codesandbox bot commented Dec 5, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link
Contributor

github-actions bot commented Dec 5, 2023

TBDocs Report

✅ No errors or warnings

@web5/api

  • Project entry file: packages/api/src/index.ts

TBDocs Report Updated at 2023-12-06T02:49:41Z ac3737f

@frankhinek frankhinek self-assigned this Dec 5, 2023
@frankhinek frankhinek added the did:dht did:dht label Dec 5, 2023
Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Merging #334 (ac3737f) into main (acc0020) will increase coverage by 0.47%.
Report is 3 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #334      +/-   ##
==========================================
+ Coverage   92.23%   92.71%   +0.47%     
==========================================
  Files          75       75              
  Lines       16771    16786      +15     
  Branches     1567     1575       +8     
==========================================
+ Hits        15469    15563      +94     
+ Misses       1276     1197      -79     
  Partials       26       26              
Components Coverage Δ
api 96.94% <ø> (ø)
common 97.78% <ø> (ø)
credentials 94.32% <ø> (ø)
crypto 100.00% <ø> (ø)
dids 91.80% <100.00%> (+3.34%) ⬆️
agent 88.08% <ø> (ø)
identity-agent 56.81% <ø> (ø)
proxy-agent 58.43% <ø> (ø)
user-agent 55.22% <ø> (ø)

@frankhinek frankhinek merged commit 8fe94d0 into main Dec 6, 2023
29 checks passed
@frankhinek frankhinek deleted the did-dht-enhancements branch December 6, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
did:dht did:dht
Projects
None yet
3 participants