CodeChain-io / foundry

A programmable open source blockchain engine
GNU General Public License v3.0
38 stars 12 forks source link

Add new error types for signatures and public keys #200

Open byeongjee opened 4 years ago

byeongjee commented 4 years ago

Currently, all signatures and public keys are just wrapper structs of bytes. For better performance and consistency, they should be wrappers of some types representing points in elliptic curves.

Verifying that a signature instance refers to a valid point in an elliptic curve should be done at the moment of decoding, not at the moment of signature verification.

For example, Schnorr signature defined as pub struct SchnorrSignature([u8; 64]); should be changed to pub struct SchnorrSignature(G1).

I believe we need a different way to handle decoding errors. i.e. decoding of signatures and public keys should fail if the RLP representation is correct, but the decoded bytes do not refer to a valid point in the elliptic curve.

Currently, RLP DecoderError does not support such invalidity. Do you have any nice idea to handle this error?

majecty commented 4 years ago

I like the point that parsing signature value and wrapping the curve point.

IMO, the Signature type should not implement RLP decodable. Checking value's validity is not role of the decoding function.

byeongjee commented 4 years ago

Then when should the signature type be verified? If the signature type is a wrapper of a curve point, it cannot be instantiated without verification.

Do you think there should be some intermediate type for RLP decoding? For example, we may obtain SignatureUnverified from RLP decoding of bytes, and SignatureVerified from another verification step. I think that would make everything clear, but double step decoding seems to be too complex. What do you think about it?

majecty commented 4 years ago

I think using an intermediate type like SignatureUnverified or [u8; 64] is better.

I guess that it will not be complex, but we need to check what will actually change when we apply it.

byeongjee commented 4 years ago

This is my conclusion. I'll fix my BLS code based on the following argument.