search

haskell-crypto / cryptonite

lowlevel set of cryptographic primitives for haskell
Other
226 stars 139 forks source link

instance (MonadRandom m, MonadTrans t, Monad (t m)) => MonadRandom (t m) #310

Closed frasertweedale closed 4 years ago

frasertweedale commented 4 years ago

It is common to use monad transformer stacks in applications, with IO at the heart. For applications using cryptonite, it is useful to have MonadRandom instances for the common transformers. Add the instance

instance (MonadRandom m, MonadTrans t, Monad (t m)) => MonadRandom (t m)

which achieves this with little code and one small additional dependency, 'transformers', with no new transitive dependencies and upon which many programs are likely to end up depending anyway. Although new dependencies are discouraged, this change is motivated by the usefulness of the instance and the desire to avoid orphan instances in applications.

Also drive-by remove (Functor m) from MonadRandom class constraints, because Functor is now a superclass of Monad and cryptonite no longer supports pre-AMP GHC versions.

Fixes: https://github.com/haskell-crypto/cryptonite/issues/304

ocheron commented 4 years ago

What if I want:

instance Monad m => MonadRandom (StateT ChaChaDRG m) where
    getRandomBytes n = StateT (return . randomBytesGenerate n)

Lifting is not the only valid implementation. And an instance for t m would overlap with almost everything.

ocheron commented 4 years ago

The goal of the typeclass is to abstract over different implementations. We can't force an instance defined for all parameterized types.

frasertweedale commented 4 years ago

@ocheron thanks for the consideration... and a justified rejection.

ocharles commented 4 years ago

What if I want:

instance Monad m => MonadRandom (StateT ChaChaDRG m) where
   getRandomBytes n = StateT (return . randomBytesGenerate n)

AFAICT, this pull request does not stop you doing that. You simply need to mark your instance as overlapping. (Though typically you mark the general instance as overlappable, but it's the same idea).

ocheron commented 4 years ago

Yes but it's not terribly good to rely on instance resolution when the impact of a change is so easily unnoticed.