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

sr25519-donna #23

Merged
merged 4 commits into from
May 25, 2020
Merged

sr25519-donna #23

merged 4 commits into from
May 25, 2020

Conversation

TerenceGe
Copy link
Contributor

@TerenceGe TerenceGe commented May 15, 2020

Grant Application Checklist

  • The application-template.md has been copied, renamed ( "project_name.md") and updated.
  • A BTC address for the payment of the milestones is provided inside the application.
  • The software of the project will be released under the Apache license version 2.0 as specified in the terms and conditions.
  • The total funding amount of the project is below 30k at the time of submission.

@CLAassistant
Copy link

CLAassistant commented May 15, 2020

CLA assistant check
All committers have signed the CLA.

@Noc2
Copy link
Collaborator

Noc2 commented May 15, 2020

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.

@TerenceGe
Copy link
Contributor Author

Thanks for the review, just updated some background info and my previous work links in the pr.

@TerenceGe
Copy link
Contributor Author

@Noc2 any feedback so far?

@Noc2
Copy link
Collaborator

Noc2 commented May 18, 2020

I asked our Jeff to take a look at the application. He will probably comment on it here, once he has some time.

@burdges
Copy link
Contributor

burdges commented May 18, 2020

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?

@burdges
Copy link
Contributor

burdges commented May 18, 2020

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.

@TerenceGe
Copy link
Contributor Author

TerenceGe commented May 18, 2020

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.

@TerenceGe
Copy link
Contributor Author

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.

@burdges
Copy link
Contributor

burdges commented May 18, 2020

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 add_trusted method in musig that violates the extra initial hash commitment's assurances. We've multiple (pre)witness values confusingly called REWINDS that we "delinearize" to produce the real witness. We actually believe this suffices making add_trusted secure making the initial hash commitment unnecessary. We're making progress on the security proof, but it's really been a slog since nobody could work on this full time, and it'll depend upon non-standard assumptions. Also, the musig code also needs a cleaner interface to DKGs like I said. We'd ideally sort this stuff out first before encouraging people to put things in airgaped devices, but the world is not perfect so this all depepends upon who wants what when. :)

@TerenceGe
Copy link
Contributor Author

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.
It's ok that I can remove the muti-sig feature in this application, and just focus on the first 2 mile stores currently. And I can add additional features in the future plan if required.
How do you think about this? If it's ok, I'll update the application.

@Noc2
Copy link
Collaborator

Noc2 commented May 18, 2020

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

@TerenceGe
Copy link
Contributor Author

@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).

@Lederstrumpf
Copy link
Contributor

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.

Looks good to me. Libsodium also has a minimum build option, but ristretto is one of the features excluded there.
We have another grant for a C-library targeting HSMs for key management - the library is currently on libsodium, so it may be of interest to them to switch to a lighter weight ed25519/sr25519 dependency for their embedded devices.

Copy link
Collaborator

@Noc2 Noc2 left a 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.

@TerenceGe
Copy link
Contributor Author

@Noc2 Ok, just added documentation into the milestones

Copy link

@retotrinkler retotrinkler left a comment

Choose a reason for hiding this comment

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

Looks good.

Copy link
Collaborator

@Noc2 Noc2 left a 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.

@TerenceGe
Copy link
Contributor Author

@Noc2 I prefer the open grants program, BTC is ok for me.

Copy link
Collaborator

@Noc2 Noc2 left a 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.

Copy link
Contributor

@RouvenP RouvenP left a 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.

@RouvenP RouvenP merged commit 5e9fbb2 into w3f:master May 25, 2020
@w3f w3f deleted a comment from Nittiyadc Jun 17, 2020
@Noc2
Copy link
Collaborator

Noc2 commented Jun 17, 2020

@Nittiyadc I deleted the comment, since it’s against our code of conduct and not related to this grant.

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.

7 participants