[CDEC-464] Switch uses of pipes to conduit

[mhuesch]

Description

The Cardano SL codebase currently depends on the pipes package in addition to depending on conduit.

Since used of conduit is far more prevalent we should replace all usage of pipes with conduit.

Linked issue

https://iohk.myjetbrains.com/youtrack/issue/CDEC-464

Type of change

• [~] 🐞 Bug fix (non-breaking change which fixes an issue)
• [~] 🛠 New feature (non-breaking change which adds functionality)
• [~] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
• 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
• [~] 🔨 New or improved tests for existing code
• [~] ⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

• I have read the style guide document, and my code follows the code style of this project.
• If my code deals with exceptions, it follows the guidelines.
• I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.

Testing checklist

• [~] I have added tests to cover my changes.
• All new and existing tests passed.
  ^ no new tests added - existing ones should cover these changes.

[mhuesch]

I marked two areas that need attention.

I did not replace uses of pipes in the post-mortem executable from cardano-sl-tools, since it does not build at present.

Once this PR is up I will look at bumping dependencies to get post-mortem buildable, and then will change uses of pipes there.

[mhuesch]

❗️ The await from conduit returns a (Maybe a). The await from pipes returns an a.

The conduit docs say here:

Wait for a single input value from upstream. If no data is available, returns Nothing. Once await returns Nothing, subsequent calls will also return Nothing.

It seems like if/when we hit the Nothing case, we will loop forever. However in the previous pipes code, I believe the Nothing values would just be dropped, so I think the behavior should be the same.

[erikd]

With conduit, await returning a Nothing means there is no more data to be got from the source.

I therefore think the we should not loop there. Instead, the Nothing case should just do pure () or pure someReturnValue depending on what is required.

[mhuesch]

This chunk is in IO (), so pure () seems appropriate.

[mhuesch]

❗️transPipe comes with a warning in the docs:

Note that the monad transforming function will be run multiple times, resulting in unintuitive behavior in some cases. For a fuller treatment, please see:

https://github.com/snoyberg/conduit/wiki/Dealing-with-monad-transformers

From reading the github post I think we're ok, because the only use of hoistLogic calls elimRealMode to unwrap a ReaderT, which shouldn't involve the state-carry-over effects mentioned as potential problems.

[erikd]

Yep, that seems legit!

[k0001]

Another way of saying this is that the passed in function foo :: (forall x. m x -> n x) must be a monad morphism and not just any natural transformation. What this means, more or less, is that foo a >> foo b should behave the same as foo (a >> b). If we set foo = flip evalState 1, it's easy to see how these two examples will behave differently. I would recommend improving the documentation here, saying that the given function is expected to be a monad morphism.

[mhuesch]

@k0001 Thanks, that is a good recommendation. I've updated the comment here.

[erikd]

The handling of the Nothing case from the return of Counduit.await is not correct and needs fixing.

Conduit.await issue addressed

[mhuesch]

@erikd I believe that I've confirmed correct behavior between the latest revision on this branch, the release/1.3.0 revision, and the latest develop revision.

As such, this should be safe to merge.