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

Support ed25519 2020 #78

Merged
merged 20 commits into from
May 21, 2021
Merged

Support ed25519 2020 #78

merged 20 commits into from
May 21, 2021

Conversation

JSAssassin
Copy link
Contributor

@JSAssassin JSAssassin commented Apr 28, 2021

Note: Some tests has been skipped since the documentLoader currently doesn't have did:v1 resolver added to it yet. All those tests pass though when I change the didMethod from v1 to key.
So whenever veres one resolver gets added to the documentLoader, these tests can be unskipped.

Update: All tests are passing now and documentLoader has been updated.

@JSAssassin JSAssassin marked this pull request as ready for review April 28, 2021 22:21
Copy link
Contributor

@mattcollier mattcollier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a changelog entry. Looks like we're effectively removing veres-one DID support temporarily, so that will need to be listed as a breaking change.

ProfileManager.js Outdated Show resolved Hide resolved
ProfileManager.js Outdated Show resolved Hide resolved
@JSAssassin
Copy link
Contributor Author

JSAssassin commented Apr 29, 2021

Note: There should be an additional commit that removes the logs and unused dep. It's not showing up on the UI for some reason, I think github is slow or something. I can see the commit when I click on the branch though.

Edit: It showed up now.

Copy link
Contributor

@dmitrizagidulin dmitrizagidulin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Nicely done

utils.js Outdated Show resolved Hide resolved
utils.js Outdated Show resolved Hide resolved
Copy link
Contributor

@mattcollier mattcollier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TY, more more style nit.

Comment on lines 1150 to 1151
return CapabilityAgent.fromSecret({
secret, handle, keyType: 'Ed25519VerificationKey2020'});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return CapabilityAgent.fromSecret({
secret, handle, keyType: 'Ed25519VerificationKey2020'});
return CapabilityAgent.fromSecret({
secret, handle, keyType: 'Ed25519VerificationKey2020'
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed here b15d5df

utils.js Outdated
Comment on lines 270 to 278
if(signer.type === 'Ed25519VerificationKey2018') {
suite = new Ed25519Signature2018({
signer,
});
} else if(signer.type === 'Ed25519VerificationKey2020') {
suite = new Ed25519Signature2020({
signer,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed, that we should throw an error if we hit the bottom without finding a matching signer.type.

@mattcollier
Copy link
Contributor

tracking: digitalbazaar/bedrock-karma#14

@mattcollier mattcollier merged commit 13f6416 into master May 21, 2021
@mattcollier mattcollier deleted the support-ed25519-2020 branch May 21, 2021 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants