Addition of test vectors to test-crypto test suite for ecdsa and schnorr

[nabinpkl]

An initial pull request that contains test vectors for secp256k1. which includes following :

ECDSA
- Signing and verification successful
- Invalid length for required parameters check
- Signing and verification of pre-image message by hashing it
- Invalid message hash length check for ecdsa
- Invalid verification key check
- Invalid message hash check
- Invalid signature check

Schnorr
- Signing and verification successful
- Invalid length for required parameters check
- Invalid verification key check
- Invalid message check
- Invalid signature check

[lehins]

Thank you for submitting this PR. I'll do a proper review tomorrow, but for now there is one comment

[iquerejeta]

A few comments on the PR:

• Unit tests should not be here. They should live in the DSIGN testing framework. Tests in the Test folder are duplicates, and those who are not should be included here.
• I don't understand what is the purpose of writing the result to the csv-outputs, nor why they are included in the repo.

The way tests should be organised with test-vectors is that given some hardcoded values (the test-vectors), functions should behave in certain ways. e.g. "running verify function on test-vector#1 should output true". I can't find that in this PR, and it seems to me that the test vectors are simply used to create the csv-outputs.

Please let me know if I'm missing something.

[nabinpkl]

@iquerejeta I was also in confusion about where should the vector tests live, is it okay to create a separate folder as done here for vector tests only or should it be integrated into src/Vector and include it in test/Main.hs?

[iquerejeta]

I don't have a strong opinion on where the vector test live. Given that this folder is dedicated only for test over hardcoded test-vectors, it might make sense to put them in Main.hs, but I don't know what are the standard practices in Haskell.

[tdammers]

Looks mostly OK, but the stringly-typed fixtures are a show stopper IMO.

[tdammers]

This is going in the right direction, but could use some cleaning up.

I'd suggest the following:

• Change the HexString type to be a newtype wrapper over ByteString
• Write appropriate IsString and Show instances for it (you can actually do the hex conversions in those, such that the wrapped ByteString is just the raw bytes, and conversion to and from hex happens when reading / printing them)
• Use OverloadedStrings to write HexStrings as string literals directly
• Write a set of conversion functions from HexString to your keys, sigs and messages (4 of them should be all you need: sign key, ver key, sig, message), and use these when defining your fixtures.

See further minor suggestions inline.

[tdammers]

Some minor remarks, but no show stoppers as far as I'm concerned.

[nabinpkl]

Another interesting test to have with ECDSA is testing signAndVerify with all hash functions we have available in cardano-base that output 32bytes (Blake2b, SHA-256, SHA3-256, and Keccak probably? thinking ). Could you add that as a unit test? Note that this does not need to be a test-vector, but rather some randomly generated unit tests that can go in Test/Crypto.

Another pr is created for these tests which are added in quick check tests testGroup Crypto.DSIGN. Review is required.
#332

[lehins]

Looks good. I have no idea why all those tests are called Vector, it doesn't look like it has to do with any vectors that I know of, but I don't particularly care about test names.

Mostly comments are about formatting and usage of utf8

[nabinpkl]

Looks good. I have no idea why all those tests are called Vector, it doesn't look like it has to do with any vectors that I know of, but I don't particularly care about test names.

Mostly comments are about formatting and usage of utf8

Sorry, I don't quite get the question. Are you referring to module Vector? I thought it indicated that it contains tests for hardcoded vectors. Test names are named using Test only at suffix.

[lehins]

I thought it indicated that it contains tests for hardcoded vectors.

@nabinpkl That's my point, there are no vectors in any of the tests. There are some lists, but no vectors.

Could you also please rebase it on master. I'll take a second look in just a little bit

[lehins]

Awesome work. Thank you!

[lehins]

Before merging we also need to squash all commits. Other than that I think this PR is ready to be merged

[nabinpkl]

I thought it indicated that it contains tests for hardcoded vectors.

@nabinpkl That's my point, there are no vectors in any of the tests. There are some lists, but no vectors.

@lehins Are you referring to Vector as in Data.Vector? But i was referring to Test vector as in set of inputs to be used to test the system. But yeah it could be ambiguous. Not sure about living area for hardcoded test values to be sit here on parent module mostly with quick check tests tho.