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

Ed25519 returns 5 instead of 6 #9

Closed
alessandrokonrad opened this issue Jan 16, 2022 · 1 comment · Fixed by #10
Closed

Ed25519 returns 5 instead of 6 #9

alessandrokonrad opened this issue Jan 16, 2022 · 1 comment · Fixed by #10
Milestone

Comments

@alessandrokonrad
Copy link
Contributor

The CurveType enum returns 5 for Ed25519 as well as the the simplified builder::EdDSA25519Key class.
This is my result:

{1: 1, 3: -8, -1: 5, -2: h'D058ED6FA27CA8FA454F1BAF2AAC58EDB774531DD2813E7BC91600D3CA8F0D4B'}

rooooooooob added a commit to rooooooooob/message-signing that referenced this issue Jan 18, 2022
Fixes Emurgo#9

Due to how wasm_bindgen works direct enums were not allowed so the rust
values of enums are NOT what their value when converted to `Label`. This
rust value was used in the `crv` field of the `Ed25519Key` builder by
accident.
@rooooooooob
Copy link
Contributor

@alessandrokonrad thanks for noticing this in the CIP-30 discussion. I just put up a fix for this #10 - it was me doing the same thing I thought might be happening in the JS code, but it was in the rust details of Ed25519Key.

I think the ideal solution for these enums would be if we could fix wasm_bindgen to allow arbitrary/negative enum values, but I originally wasn't sure how hard it would be to do that or how long it would take to get that merged/etc. It's possible that if rustwasm/wasm-bindgen#2631 ever gets done that it would resolve this too, depending how they implement that.

@vsubhuman vsubhuman added this to the 1.1.0 milestone Feb 9, 2022
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 a pull request may close this issue.

3 participants