-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
sr25519-donna #23
sr25519-donna #23
Conversation
Thanks for the application. Could you provide a little bit more background/update the project description? Also for anything cryptography related we usually ask teams to provide links to prior work and their general experience in the field. |
Thanks for the review, just updated some background info and my previous work links in the pr. |
@Noc2 any feedback so far? |
I asked our Jeff to take a look at the application. He will probably comment on it here, once he has some time. |
What is the background of the person doing the implementation? How much of the existing new code is home grown? Any common code taken from https://github.com/Ristretto/libristretto255 or https://github.com/isislovecruft/ristretto-donna or https://sourceforge.net/p/ed448goldilocks/wiki/Home/ Why the choice to reimplement instead of using libsodium? |
We've fixed the VRF so it could be implemented, but about the only reason for VRF on iOS seems like games. We've actually discussed using VRFs for games recently, so maybe.. Is there any immediate need for the mulisig somewhere? We're still trying to prove one new two round version to be secure, which might simplify the code. Also w3f/PSPs#7 requires I clean up the code considerably before anyone adds some DKG. |
This library is originated when I build a mobile wallet for polkadot, the core is built in C, so I think it's better to have a C version for the algorithm. I dig into the code of https://github.com/isislovecruft/ristretto-donna, and found its usable after fixing some issues https://github.com/isislovecruft/ristretto-donna/pull/4/files, I think it's more light weight than libsodium. For those projects using ed25519-donna (such as trezor), it's more convenient to integrate polkadot into their system. |
mulisig might not require currently, but since the lib's goal is to keep sync with rust version, we can support that once it's ready. |
I see, so the Ristretto code came from you fixing Isis Lovecruft's implementation? Very good. :) From where did the merlin code originate? Right now, we've this |
The merlin code is based on the author's implementation (https://github.com/hdevalence/libmerlin), also I fixed a little issue and add some stuff. |
Could you provide some more information about your iOS app, since I’m currently not sure how useful it would be for our ecosystem. Apart from this, I think it makes sense to remove the third milestone for now. Also, it obviously helps if you could reduce the price a little bit further. Thanks |
@Noc2 apple's infrastructure dose not support rust very well (rust-lang/rust#35968). Although it's possible to use rust lib as static lib for ios, that requires you disable bitcode, which prevents you from taking advantage of apple's further optimizations (such as reducing bundle size) for you. That's why some famous libraries such as react native dose not use rust currently (https://twitter.com/sebmarkbage/status/1213199159840260096). |
Looks good to me. Libsodium also has a minimum build option, but ristretto is one of the features excluded there. |
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.
Thanks for updating the proposal. At this stage, I would be interested in funding it. Just one more thing, could you integrate some basic documentation into the milestones to make it easier for others to use your implementation.
@Noc2 Ok, just added documentation into the milestones |
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.
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 just noticed that you also applied via the general grants program. If you are interested in receiving DOTs as part of your grant (x %). I suggest that you close this application and update the application via the general grants program. Let me know how you want to proceed.
@Noc2 I prefer the open grants program, BTC is ok for me. |
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.
In this case, I guess we won’t discuss the application via the general grants committee. Let me know in case you change your mind.
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.
Thanks for the application, looks good.
@Nittiyadc I deleted the comment, since it’s against our code of conduct and not related to this grant. |
Grant Application Checklist