Depend on crc32c instead of digest #106
Conversation
There was a problem hiding this comment.
What's the motivation for this change? I'm not opposed, just wondering
BTW, this is definitely a breaking change, no? So we'd need a changelog entry to that effect. And could changing checksums unexpectedly break things in ouroboros-consensus? Does it break the ImmutableDB (IIRC that's where checksums are used)?
| , containers ^>=0.5 || ^>=0.6 || ^>=0.7 | ||
| , deepseq ^>=1.4 || ^>=1.5 | ||
| , digest ^>=0.0 | ||
| , crc32c |
There was a problem hiding this comment.
This should have a strict upper and lower bound (or caret operator)
| instance CRC32C [Word8] where | ||
| crc32cUpdate n = (crc32cUpdate n) . BL.pack |
There was a problem hiding this comment.
Is this instance used anywhere?
| instance CRC32C BSS.ShortByteString where | ||
| crc32c = crc32c . BSS.fromShort | ||
|
|
||
| computeCRC :: forall a. Digest.CRC32 a => a -> CRC | ||
| computeCRC = coerce (Digest.crc32 :: a -> Word32) | ||
| crc32cUpdate cks = crc32cUpdate cks . BSS.fromShort |
There was a problem hiding this comment.
This can be optimised to work on the short bytestring directly
| computeCRC, | ||
| initCRC, | ||
| updateCRC, |
There was a problem hiding this comment.
updateCRC and computeCRC reference the new CRC32C class but the class isn't exposed. Do we really need a new class? I think we only really need to compute crc's on bytestrings, and those computations can be internal
2 participants
No description provided.