Add FixedSizeBytes, lift VRF sizes to the type level#665
Conversation
91ae16a to
f5bc0ae
Compare
9304d9a to
4956ff8
Compare
4956ff8 to
23c6496
Compare
neilmayhew
left a comment
There was a problem hiding this comment.
LGTM. My comments are just suggestions.
d62deb2 to
52b93c6
Compare
lehins
left a comment
There was a problem hiding this comment.
I am blocking this PR because I want to give my review before it gets merged
lehins
left a comment
There was a problem hiding this comment.
This is a very nice direction! Still needs a bit of love though to make ideal.
5bb498c to
fe2e04d
Compare
1523a46 to
19ee900
Compare
Make rawDecodeFixedSized MonadFail-returning and add mkRawFixedSizedDecoder, which supplies a human-readable noun while deriving the type name and expected size from the type. DSIGN/KES/VRF ToCBOR/FromCBOR instances and the Signed/Certified codecs now use encodeFixedSized/decodeFixedSized; the per-type encode*/decode* functions are deprecated. Deserialise error messages now use the type-constructor name (e.g. "VerKeyDSIGN:") instead of the decoder function name; test expectations updated accordingly. Rename prop_cbor_with -> prop_cbor_fixed_sized and prop_cbor_direct_vs_class -> prop_cbor_fixed_sized_vs_class, dropping their encoder/decoder args.
f093a6b to
5e28649
Compare
lehins
left a comment
There was a problem hiding this comment.
We need to reduce duplication if we introducing this new interface, which requires inverting the approach of rawEncodeFixedSized/rawDecodeFixedSized using rawSerialise*/rawDeserialise*
| -- | Build a raw fixed-size decoder, failing with an informative message when | ||
| -- the underlying decoder returns 'Nothing'. The first argument should be a | ||
| -- noun describing the type (e.g. "key", "signature") | ||
| mkRawFixedSizedDecoder :: | ||
| forall a m. | ||
| (Typeable a, KnownNat (FixedSize a), MonadFail m) => | ||
| String -> (BS.ByteString -> Maybe a) -> BS.ByteString -> m a | ||
| mkRawFixedSizedDecoder name rawDecode bs = | ||
| maybe (failFixedSized name (Proxy @a) bs) pure (rawDecode bs) | ||
| {-# INLINE mkRawFixedSizedDecoder #-} |
There was a problem hiding this comment.
I think you missed the point of my suggestion about MonadFail
The goal is to:
- avoid duplicate logic of checking the size
- improve failure messages
- and later deprecation or extraction into standalone functions of individual
rawDeserialise*functions.
Which means we need an instance of FixedSizeCodec for each algorithm, eg. instance FixedSizeCodec (VerKeyDSIGN Ed25519DSIGN) and all of the raw(Des|S)erialise* can get this implementation rawDeserialise* = rawDecodeFixedSized
| -- | Build a raw fixed-size decoder, failing with an informative message when | |
| -- the underlying decoder returns 'Nothing'. The first argument should be a | |
| -- noun describing the type (e.g. "key", "signature") | |
| mkRawFixedSizedDecoder :: | |
| forall a m. | |
| (Typeable a, KnownNat (FixedSize a), MonadFail m) => | |
| String -> (BS.ByteString -> Maybe a) -> BS.ByteString -> m a | |
| mkRawFixedSizedDecoder name rawDecode bs = | |
| maybe (failFixedSized name (Proxy @a) bs) pure (rawDecode bs) | |
| {-# INLINE mkRawFixedSizedDecoder #-} |
Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
8ad5a6a to
592a5c3
Compare
592a5c3 to
bffd13f
Compare
a23cc29 to
24cc783
Compare
03cb8bd to
a6753eb
Compare
Description
This PR lifts the sizes of
VerKeyVRF,SignKeyVRFandCertVRFto the type level, which is similar to what we already do forDSIGNandKES. I also added theFixedSizeBytestypeclass, which gives a uniform interface to all therawDeserialise*andencode*functions forDSIGN/KES/VRFtypes.Checklist
CHANGELOG.mdfor the affected packages.New section is never added with the code changes. (See RELEASING.md)
.cabalandCHANGELOG.mdfiles according to theversioning process.
.cabalfiles for all affected packages are updated.If you change the bounds in a cabal file, that package itself must have a version increase. (See RELEASING.md)