decentralized-identity / did-jwt

Create and verify DID verifiable JWT's in Javascript
Apache License 2.0
333 stars 71 forks source link

Use audited dependencies #270

Closed paulmillr closed 1 year ago

paulmillr commented 1 year ago

Wanna me send a PR?

nklomp commented 1 year ago

Not a maintainer, but I think it would be really nice to use audited packages.

I have one question though. Do these libs work on React Native?

As there is overlap in maintainers between Veramo and this package and one of the requirements of Veramo is that it also has to run on RN, this package also needs to run on RN.

If it does not it would break certain apps, like our wallet.

paulmillr commented 1 year ago

Yes they do

nklomp commented 1 year ago

Cool

@mirceanis What do you think. If you ask me it would be great to have audited packages for these core functionalities needed in this lib

inevolin commented 1 year ago

@paulmillr a PR would be highly appreciated! 🙏 https://discord.com/channels/878293684620234752/933661591264702474/1081295858714296491

mirceanis commented 1 year ago

The discord link refers to this server: https://discord.gg/U5SCRnNFuS

mirceanis commented 1 year ago

I'm in favor of a replacement.

I would have liked to move forward with #170, which aims to refactor this library to allow folks to bring their own crypto implementations instead of us bundling things in, but since I haven't had any time for that it makes more sense to do what you suggest.

paulmillr commented 1 year ago

I see you have ES256K, ES256 signers. How should this rewrite behave? Should I simply replace elliptic with noble in these files, or should I create new signers?

ukstv commented 1 year ago

Great minds think alike: https://github.com/decentralized-identity/did-jwt/pull/280

Have just noticed this thread, otherwise would ping you pals first before doing any code.

uport-automation-bot commented 1 year ago

:tada: This issue has been resolved in version 7.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

inevolin commented 1 year ago

Hey @ukstv , this is wonderful, thank you for doing this! I was wondering if you're able to do the same for the did-provider-key lib, which still has the @transmute/... dependenceis and they marked as unstable releases. There have been discussions of replacing those as well. That would be amazing!

ukstv commented 1 year ago

@inevolin The package you mentioned is not directly used by Ceramic ecosystem AFAIK, so I would like to politely refuse the invitation.

What I could propose instead, is maybe you could depend on key-did-provider-ed25519 or key-did-provider-secp256k1 which are cared for. As soon as https://github.com/paulmillr/noble-curves/pull/32 gets merged, we could fully migrate to noble-crypto and leverage its speed and dependability there. That would transitively apply to your package suite as well.

inevolin commented 1 year ago

interesting! thank you