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

Add test vectors #43

Open
Warchant opened this issue Aug 4, 2019 · 4 comments
Open

Add test vectors #43

Warchant opened this issue Aug 4, 2019 · 4 comments

Comments

@Warchant
Copy link

Warchant commented Aug 4, 2019

Please, add test vectors, so other implementers can verify their implementations.

@jacogr
Copy link
Contributor

jacogr commented Aug 4, 2019

Verified the 0.8.4 upgrade via these (assuring old compat) for the WASM interface -

https://github.com/polkadot-js/wasm/blob/4a2249a220b4c3e2744c29cee9979c470b11ce42/packages/wasm-crypto/src/sr25519.rs#L144

Additionally there are some more-pedantic e2e tests as well -

https://github.com/polkadot-js/wasm/blob/4a2249a220b4c3e2744c29cee9979c470b11ce42/packages/wasm-crypto/test/all/sr25519.js

The combination of these actually ensured compat with older keypairs, signatures for this specific wrapper.

@Warchant
Copy link
Author

Warchant commented Aug 4, 2019

@jacogr this The problem is that I will copy these tests into sr25519-crust, and eventually test vectors will change (they already changed at least once after transition from 0.1.0 to 0.8.4). Then, implementers will not know which vectors to use, because implementation(s) are not synchronized.

The best approach is to generate file similar to sign.input of ed25519, and put it somewhere inside repository. Then, this file will be a single source of truth for the test data.

@Warchant Warchant changed the title Add testvectors Add test vectors Aug 4, 2019
@jacogr
Copy link
Contributor

jacogr commented Aug 4, 2019

@Warchant Agree 100%, I think this issue also sums it up #36 - effectively there the current status is "ok, the time is right now, interfaces are stable, go for it"

So I was not suggesting linking the above is the right long-term solution, just trying to show something that at least helps right at this point in time.

@burdges burdges mentioned this issue Aug 5, 2019
@burdges
Copy link
Collaborator

burdges commented Aug 5, 2019

I'm happy for any PRs on this. I'll try to do it myself in the near future if nothing materializes.

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

No branches or pull requests

3 participants