search

decentralized-identity / ion-sdk

TypeScript SDK for ION
Apache License 2.0
29 stars 14 forks source link

Add support for ed25519 in IonKey (1/2) #20

Closed gjgd closed 2 years ago

gjgd commented 2 years ago

Part 1 of adding support for Ed25519 in IonSDK:

Part 2 will be updating IonRequest and IonDid

csuwildcat commented 2 years ago

So if this addition makes it possible to use Ed25519 for operation keys, will there be a PR in Sidetree Core to allow them to be used in the actual nodes?

gjgd commented 2 years ago

My plan was to use this change to generate Ed25519 sidetree fixtures, but it can also be used to allow Ed25519 in Sidetree core node.

thehenrytsai commented 2 years ago

Will take a look in the next couple of days, adding @xinaxu in case he has bandwidth first.

thehenrytsai commented 2 years ago

@gjgd, thanks for taking the time to start to adding ED25519 support. The PR looks great overall, given that it really isn't hooked up with any thing really, so I see this more as a preparation to more PRs to come, so just a few comments, please do keep the code coverage at 100%.

OR13 commented 2 years ago

We are blocked by this, and need a timeline for merge. @thehenrytsai are you able to approve and merge? or should we fork the implementation and push support for this here, removing the ION SDK dep?:

https://github.com/transmute-industries/sidetree.js/blob/main/packages/wallet/src/operations/update.ts#L4

We are trying to wrap this SDK to maintain interop and support ION at the same time, but we are required to support EdDSA.

thehenrytsai commented 2 years ago

@OR13, I reviewed the PR couple of days ago with a few comments, expecting them to be addressed or responded to before I can approve the merge.

OR13 commented 2 years ago

@thehenrytsai thanks, I was surprised not to see a "change request" from you on the PR... you left comments, but did not request changes.

cc @gjgd do you understand the "comments that are requesting changes"?

gjgd commented 2 years ago

I didn't get a chance to review the comments yet, will try to do that today and make the appropriate changes

gjgd commented 2 years ago

@thehenrytsai Thanks for the review, I believe I addressed all of the comments. Part 2 coming next

csuwildcat commented 2 years ago

@thehenrytsai what's the status on this? Let's sync as a WG after the holidays to ensure this PR doesn't fall to the wayside.