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: switch from wasm to micro-sr25519 #1971

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paulmillr
Copy link

@paulmillr paulmillr commented Jan 13, 2025

sr cryptography in 467 lines of js code: https://github.com/paulmillr/micro-sr25519

  • All tests pass
  • There are 2 linting issues which I have 0 ideas how to fix:
packages/util-crypto/src/sr25519/deriveHard.ts
  8:9  error  Avoid referencing unbound methods which may cause unintentional scoping of `this`.
If your function does not access `this`, you can annotate it with `this: void`, or consider using an arrow function instead  @typescript-eslint/unbound-method

packages/util-crypto/src/sr25519/deriveSoft.ts
  8:63  error  Avoid referencing unbound methods which may cause unintentional scoping of `this`.
If your function does not access `this`, you can annotate it with `this: void`, or consider using an arrow function instead  @typescript-eslint/unbound-method

As a side note, this repo's dev infrastructure is slightly idiotic and urgently needs to be improved if the goal is to gain any new outside contributors. It took me 1 hour what should have taken 4 minutes. The test runner takes 3 minutes (too slow, not parallel) - would be great to have an option to crash on a first failed test. README and CONTRIBUTING.md have some bogus rules while not mentioning what's polkadot-dev-run-test and how it works. Linting is non-descriptive: for example, I don't understand what the rule above means (even though i've coded a few js libraries).

This is not really something I "want". This is something that you folks struggle with daily. Imagine how much time you're losing!

@TarikGul
Copy link
Member

For the CI errors, it just needs a yarn install pushed up.

As a side note, this repo's dev infrastructure is slightly idiotic and urgently needs to be improved if the goal is to gain any new outside contributors.

I agree, (preaching to the choir per say). That being said the polkadot-js libs are huge, and so coupled that its hard to make headway with such little bandwidth to spare (Getting way more help soon which should really help move some things forward). That being said the dev infrastructure is all custom implementations or wrappers with no documentation which makes it even more annoying (I've been making some progress on changing that). All in all yes, I agree with your sentiment, when maintenance transferred over to me about a year ago I was left with a bunch of burning fires, a lot have been put out, but there is still much work to be done. Luckily bandwidth will be freeing up soon for me to focus on more QOL goals.


As for the content of the PR, I need to look into this a bit more in depth this week - I love the noble libs as it is, and know a bit of your work (Thank you for all the useful libs you publish) - but I still need to do my due diligence as this would be a big change.

@paulmillr
Copy link
Author

paulmillr commented Jan 14, 2025

@TarikGul micro-sr25519 was funded by polkadot treasury BTW. There was a proposal.

@paulmillr
Copy link
Author

Any updates?

@TarikGul
Copy link
Member

I did look into the error, but couldn't find the source of the issue (yet!). Will continue some efforts tomorrow.

I read the treasury proposal - I'm under the assumption that there are ongoing audits and/or suppose to be audits in the future?

@paulmillr
Copy link
Author

Creator of proposal mentioned they could engage auditors; and I told them that the library should be tested somewhere before audit, because if, while testing, some changes would become necessary, that would invalidate the audit.

Since we have kinda tested the library with this pull request, albeit in a limited way, i’m ok with deferring the merge until post-audit.

@TarikGul
Copy link
Member

i’m ok with deferring the merge until post-audit.

This would be ideal. In the meantime I'll make sure I have a good understanding of this error/issue with the linter so the process for getting this in is smoother once the audit is complete. - Thanks

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.

2 participants