foundry icon indicating copy to clipboard operation
foundry copied to clipboard

Add new error types for signatures and public keys

Open byeongjee opened this issue 5 years ago • 4 comments

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?

byeongjee avatar Feb 24 '20 06:02 byeongjee

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.

majecty avatar Feb 24 '20 07:02 majecty

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?

byeongjee avatar Feb 24 '20 07:02 byeongjee

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.

majecty avatar Feb 24 '20 08:02 majecty

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

  • Signature should be stored in bytes form
    • because each signature will be verified only once
  • We have two types
    • BlsPublicUnverified type has serde, rlp traits
    • BlsPublic type has all verification-related methods
  • At the self-nomination step, convert BlsPublicUnverified into BlsPublic
    • every Candidate and Validator must hold BlsPublic, not BlsPublicUnverified
  • Conversion from BlsPublicUnverified to BlsPublic occurs at the self-nomination step
  • When decoding the genesis state from the initial configuration file, it should directly decode BlsPublic from a string. An error at this step should result in panic.

byeongjee avatar Feb 26 '20 07:02 byeongjee