-
Notifications
You must be signed in to change notification settings - Fork 135
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
First draft of did-pkh method. #1052
First draft of did-pkh method. #1052
Conversation
- Create new pkh-did-provider - Add resolve did-pkh tests - Add create did-pkh identidfier test - Add VC creation/verification using did-pkh identifier Currently not using pkh-did-resolver package (error import). Code momentarily copied to did-provider-pkh resolver.
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.
Thank you for contributing this and apologies for taking so long to review.
I made some suggestions to fix the failing test.
The DID string for eip155 is formed using the ethereum address, not the compressed public key. This was also the cause of the test failures you were seeing.
Also, CAIP10/eip155 require chainIDs to be used in the blockchainAccountId
, not network names.
eip155:1:0xaddress
instead of eip155:mainnet:0xaddress
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.
I added another couple of suggested fixes to the tests.
Please take a look.
-Use chainId only and not network name -clean up params -fix tests
- Create new pkh-did-provider - Add resolve did-pkh tests - Add create did-pkh identidfier test - Add VC creation/verification using did-pkh identifier Currently not using pkh-did-resolver package (error import). Code momentarily copied to did-provider-pkh resolver.
-Use chainId only and not network name -clean up params -fix tests
Co-authored-by: Mircea Nistor <mirceanis@gmail.com>
Co-authored-by: Mircea Nistor <mirceanis@gmail.com>
Co-authored-by: Mircea Nistor <mirceanis@gmail.com>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## next #1052 +/- ##
==========================================
- Coverage 80.25% 79.76% -0.49%
==========================================
Files 118 130 +12
Lines 4056 4699 +643
Branches 875 1011 +136
==========================================
+ Hits 3255 3748 +493
- Misses 798 951 +153
+ Partials 3 0 -3 |
now that tests are passing, it would probably be a good time to rebase this branch against |
- Create new pkh-did-provider - Add resolve did-pkh tests - Add create did-pkh identidfier test - Add VC creation/verification using did-pkh identifier Currently not using pkh-did-resolver package (error import). Code momentarily copied to did-provider-pkh resolver.
-Use chainId only and not network name -clean up params -fix tests
- Create new pkh-did-provider - Add resolve did-pkh tests - Add create did-pkh identidfier test - Add VC creation/verification using did-pkh identifier Currently not using pkh-did-resolver package (error import). Code momentarily copied to did-provider-pkh resolver.
-Use chainId only and not network name -clean up params -fix tests
Co-authored-by: Mircea Nistor <mirceanis@gmail.com>
Co-authored-by: Mircea Nistor <mirceanis@gmail.com>
Co-authored-by: Mircea Nistor <mirceanis@gmail.com>
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.
Thanks for contributing this!
Create new pkh-did-provider
The pkh-did-provider supports only eip155 network id (did:pkh:eip155:...) so for now the network id is hardcoded, but we could easily parameterize to support other networks.
Add resolve did-pkh tests
Tests passing
Add create did-pkh identifier test
Tests passing
Add VC creation/verification using did-pkh identifier
Tests passing
Detail
Currently not using pkh-did-resolver package (error import). Code momentarily copied to did-provider-pkh resolver.
What issue is this PR fixing
Resolves #1024
Quality
Check all that apply:
yarn
,yarn build
,yarn test
,yarn test:browser
locally.