Repository: tahoe-lafs/zfec (language: python)

Issue:
"nix build" fails with a cabal error of some sort. stack trace attached
```
pmarreck  master @ c4b09f30 ?   lts-5.2 🟢 471ms
20230209123948 ~/ /zfec bash> nix build
error: builder for '/nix/store/3h88n4l57na51lfbywab56zdlhsvdhzr-fec-0.1.1.drv' failed with exit code 1;
       last 10 log lines:
       >   $, called at libraries/Cabal/Cabal/Distribution/Fields/ParseResult.hs:185:13 in Cabal-3.2.1.0:Distribution.Fields.ParseResult
       >   parseString, called at libraries/Cabal/Cabal/Distribution/Fields/ParseResult.hs:170:5 in Cabal-3.2.1.0:Distribution.Fields.ParseResult
       >   readAndParseFile, called at libraries/Cabal/Cabal/Distribution/PackageDescription/Parsec.hs:94:33 in Cabal-3.2.1.0:Distribution.PackageDescription.Parsec
       >   readGenericPackageDescription, called at libraries/Cabal/Cabal/Distribution/Simple.hs:248:19 in Cabal-3.2.1.0:Distribution.Simple
       >   confPkgDescr, called at libraries/Cabal/Cabal/Distribution/Simple.hs:216:33 in Cabal-3.2.1.0:Distribution.Simple
       >   configureAction, called at libraries/Cabal/Cabal/Distribution/Simple.hs:180:19 in Cabal-3.2.1.0:Distribution.Simple
       >   defaultMainHelper, called at libraries/Cabal/Cabal/Distribution/Simple.hs:116:27 in Cabal-3.2.1.0:Distribution.Simple
       >   defaultMain, called at Setup.lhs:3:10 in main:Main
       > Setup: Failed parsing "./fec.cabal".
       >
       For full logs, run 'nix log /nix/store/3h88n4l57na51lfbywab56zdlhsvdhzr-fec-0.1.1.drv'.

pmarreck  master @ c4b09f30 ?   lts-5.2 ❌ERROR 28s663ms
20230209124032 ~/ /zfec bash> pull
Already up to date.

pmarreck  master @ c4b09f30 ?   lts-5.2 🟢 545ms
20230209124038 ~/ /zfec bash> nix log /nix/store/3h88n4l57na51lfbywab56zdlhsvdhzr-fec-0.1.1.drv
@nix { "action": "setPhase", "phase": "setupCompilerEnvironmentPhase" }
setupCompilerEnvironmentPhase
Build with /nix/store/zaj69ac946lkj14g4ggrf6gz39hig72z-ghc-8.10.7.
@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking sources
unpacking source archive /nix/store/i3m8c9907bgqjsnmvnfhx3nyms1yqj0j-rhwcbxvj0kflk7q77w0xwmpdn9y3jjxg-source
source root is rhwcbxvj0kflk7q77w0xwmpdn9y3jjxg-source
@nix { "action": "setPhase", "phase": "patchPhase" }
patching sources
@nix { "action": "setPhase", "phase": "compileBuildDriverPhase" }
compileBuildDriverPhase
setupCompileFlags: -package-db=/build/tmp.AfluUAQBeQ/setup-package.conf.d -j16 +RTS -A64M -RTS -threaded -rtsopts
[1 of 1] Compiling Main             ( Setup.lhs, /build/tmp.AfluUAQBeQ/Main.o )
Linking Setup ...
@nix { "action": "setPhase", "phase": "configurePhase" }
configuring
configureFlags: --verbose --prefix=/nix/store/1cy7krvg4zb2ayyqfwvpvcbagbl5ysg1-fec-0.1.1 --libdir=$prefix/lib/$compiler --libsubdir=$abi/$libname --docdir=/nix/store/v03l5hbsxj8f5hlqagxrrfcd49mg4ka4-fec-0.1.1-doc/share/doc/fec-0.1.1 --with-gcc=gcc --package-db=/build/tmp.AfluUAQBeQ/package.conf.d --ghc-options=-j16 +RTS -A64M -RTS --disable-split-objs --enable-library-profiling --profiling-detail=exported-functions --disable-profiling --enable-shared --disable-coverage --enable-static --disable-executable-dynamic --enable-tests --disable-benchmarks --enable-library-vanilla --disable-library-for-ghci --ghc-option=-split-sections --extra-lib-dirs=/nix/store/1i5ah27gxx3a3fyjyydfwwzqq8ni33i8-ncurses-6.3-p20220507/lib --extra-lib-dirs=/nix/store/xghl0l2f6byzqvy06ana2y4isqwa7blw-libffi-3.4.4/lib --extra-lib-dirs=/nix/store/ijz81p08bp812q2bvv77lz9qpfzncibd-gmp-with-cxx-6.2.1/lib
Using Parsec parser
Warning: fec.cabal:0:0: Unsupported cabal-version 3.4. See
https://github.com/haskell/cabal/issues/4899.
Warning: fec.cabal:0:0: Unsupported cabal-version. See
https://github.com/haskell/cabal/issues/4899.
CallStack (from HasCallStack):
  $, called at libraries/Cabal/Cabal/Distribution/Fields/ParseResult.hs:185:13 in Cabal-3.2.1.0:Distribution.Fields.ParseResult
  parseString, called at libraries/Cabal/Cabal/Distribution/Fields/ParseResult.hs:170:5 in Cabal-3.2.1.0:Distribution.Fields.ParseResult
  readAndParseFile, called at libraries/Cabal/Cabal/Distribution/PackageDescription/Parsec.hs:94:33 in Cabal-3.2.1.0:Distribution.PackageDescription.Parsec
  readGenericPackageDescription, called at libraries/Cabal/Cabal/Distribution/Simple.hs:248:19 in Cabal-3.2.1.0:Distribution.Simple
  confPkgDescr, called at libraries/Cabal/Cabal/Distribution/Simple.hs:216:33 in Cabal-3.2.1.0:Distribution.Simple
  configureAction, called at libraries/Cabal/Cabal/Distribution/Simple.hs:180:19 in Cabal-3.2.1.0:Distribution.Simple
  defaultMainHelper, called at libraries/Cabal/Cabal/Distribution/Simple.hs:116:27 in Cabal-3.2.1.0:Distribution.Simple
  defaultMain, called at Setup.lhs:3:10 in main:Main
Setup: Failed parsing "./fec.cabal".


pmarreck  master @ c4b09f30 ?   lts-5.2 🟢 1s940ms
```


Discussion / hints:
I'm not familiar enough with Haskell to fix; was just trying to compile the commandline apps to play with `zfec`...
Thank you - confirmed.  I get the same build error.  We should fix this (I suppose by downgrading our cabal format version).

Meanwhile, if anyone needs a workaround, this should work:

```
nix develop
cabal build
```

The Python bindings should be unaffected (since the Nix packaging doesn't even try to account for them - you just have to use pip, etc).

One nice thing about Nix is that the original dev being able to reproduce a build failure is highly probable instead of highly improbable, which seems to be the case in literally every other non-trivial example I've seen, lol. Glad you're reproducing it, at least. Thanks for the workaround. You may just need to pin cabal to an earlier version (or make it work with the later version, assuming that isn't too difficult)
silly question, how do I just get a commandline version to play with?
The Python packaging comes with `zfec` and `zunfec` command line tools - does that help?  You should also get these if you install a pre-build package (ie `pip install zfec`).

I'm not sure there are any command line tools that use the Haskell bindings (at least, `fec.cabal` doesn't seem to define any).

I'm running on NixOS, we declaratively install things, so `pip install zfec` is not really an option unfortunately, is there a way to build the python bits within the project directory using whatever tooling the `flake.nix` file provides?
> I'm running on NixOS, we declaratively install things, so `pip install zfec` is not really an option unfortunately,

Ah, indeed.

> is there a way to build the python bits within the project directory using whatever tooling the `flake.nix` file provides?

Not currently, though this would be a nice addition to the flake, I think.

The Python bindings are actually in nixpkgs itself though, so if you install `legacyPackages.x86_64-linux.python3.withPackages (ps: [ps.zfec])` then I think you'll get something usable - though it's whatever  version is in your nixpkgs rather than master@HEAD.  However, not much has changed in zfec in a while so unless you wanted to explore some specific recent change, maybe this is good enough?

Although, this also seems to work, if you did want master@HEAD:

```
legacyPackages.x86_64-linux.python3.withPackages (ps: [(ps.zfec.overrideAttrs (old: { 
    src = /path/to/zfec/checkout; 
}))])
```

Yep, got it in that way, thanks!

Fix patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
new file mode 100644
index 00000000..98728bf0
--- /dev/null
+++ b/.circleci/config.yml
@@ -0,0 +1,33 @@
+version: "2.1"
+
+jobs:
+  nix-build:
+    docker:
+      - image: "nixos/nix:2.13.2"
+
+    environment:
+      # Let us use features marked "experimental".  For example, most/all of
+      # the `nix <subcommand>` forms.  Also, allow import from derivation
+      # because cabal2nix requires it.
+      NIX_CONFIG: |
+        experimental-features = nix-command flakes
+        allow-import-from-derivation = true
+
+    steps:
+      - "checkout"
+      - run:
+          name: "nix flake check"
+          command: |
+            nix flake show
+            nix flake check -v
+
+      - run:
+          name: "nix build"
+          no_output_timeout: "30m"
+          command: |
+            nix build -v
+
+workflows:
+  ci:
+    jobs:
+      - nix-build
diff --git a/Setup.lhs b/Setup.lhs
deleted file mode 100755
index 5bde0de9..00000000
--- a/Setup.lhs
+++ /dev/null
@@ -1,3 +0,0 @@
-#!/usr/bin/env runhaskell
-> import Distribution.Simple
-> main = defaultMain
diff --git a/fec.cabal b/fec.cabal
index bfbdb638..ae50f1bc 100644
--- a/fec.cabal
+++ b/fec.cabal
@@ -1,4 +1,4 @@
-cabal-version:      3.4
+cabal-version:      3.0
 name:               fec
 version:            0.1.1
 license:            GPL-2.0-or-later
diff --git a/haskell/Codec/FEC.hs b/haskell/Codec/FEC.hs
index ff16c26c..df92ac46 100644
--- a/haskell/Codec/FEC.hs
+++ b/haskell/Codec/FEC.hs
@@ -1,95 +1,120 @@
-{-# LANGUAGE ForeignFunctionInterface, EmptyDataDecls #-}
--- |
--- Module:    Codec.FEC
--- Copyright: Adam Langley
--- License:   GPLv2+|TGPPLv1+ (see README.rst for details)
---
--- Stability: experimental
---
--- The module provides k of n encoding - a way to generate (n - k) secondary
--- blocks of data from k primary blocks such that any k blocks (primary or
--- secondary) are sufficient to regenerate all blocks.
---
--- All blocks must be the same length and you need to keep track of which
--- blocks you have in order to tell decode. By convention, the blocks are
--- numbered 0..(n - 1) and blocks numbered < k are the primary blocks.
+{-# LANGUAGE EmptyDataDecls #-}
+{-# LANGUAGE ForeignFunctionInterface #-}
 
+{- |
+ Module:    Codec.FEC
+ Copyright: Adam Langley
+ License:   GPLv2+|TGPPLv1+ (see README.rst for details)
+
+ Stability: experimental
+
+ The module provides k of n encoding - a way to generate (n - k) secondary
+ blocks of data from k primary blocks such that any k blocks (primary or
+ secondary) are sufficient to regenerate all blocks.
+
+ All blocks must be the same length and you need to keep track of which
+ blocks you have in order to tell decode. By convention, the blocks are
+ numbered 0..(n - 1) and blocks numbered < k are the primary blocks.
+-}
 module Codec.FEC (
-    FECParams(paramK, paramN)
-  , fec
-  , encode
-  , decode
-
-  -- * Utility functions
-  , secureDivide
-  , secureCombine
-  , enFEC
-  , deFEC
-  ) where
+    FECParams (paramK, paramN),
+    fec,
+    encode,
+    decode,
 
+    -- * Utility functions
+    secureDivide,
+    secureCombine,
+    enFEC,
+    deFEC,
+) where
+
+import Data.Bits (xor)
 import qualified Data.ByteString as B
-import qualified Data.ByteString.Unsafe as BU
 import qualified Data.ByteString.Internal as BI
+import qualified Data.ByteString.Unsafe as BU
+import Data.List (nub, partition, sortBy, (\\))
 import Data.Word (Word8)
-import Data.Bits (xor)
-import Data.List (sortBy, partition, (\\), nub)
-import Foreign.Ptr
-import Foreign.Storable (sizeOf, poke)
-import Foreign.ForeignPtr
 import Foreign.C.Types
+import Foreign.ForeignPtr
 import Foreign.Marshal.Alloc
-import Foreign.Marshal.Array (withArray, advancePtr)
-import System.IO (withFile, IOMode(..))
+import Foreign.Marshal.Array (advancePtr, withArray)
+import Foreign.Ptr
+import Foreign.Storable (poke, sizeOf)
+import System.IO (IOMode (..), withFile)
 import System.IO.Unsafe (unsafePerformIO)
 
 data CFEC
 data FECParams = FECParams
-  { cfec   :: (ForeignPtr CFEC)
-  , paramK :: Int
-  , paramN :: Int
-  }
+    { cfec :: (ForeignPtr CFEC)
+    , paramK :: Int
+    , paramN :: Int
+    }
 
 instance Show FECParams where
-  show (FECParams _ k n) = "FEC (" ++ show k ++ ", " ++ show n ++ ")"
+    show (FECParams _ k n) = "FEC (" ++ show k ++ ", " ++ show n ++ ")"
 
-foreign import ccall unsafe "fec_new" _new :: CUInt  -- ^ k
-                                           -> CUInt  -- ^ n
-                                           -> IO (Ptr CFEC)
+foreign import ccall unsafe "fec_new"
+    _new ::
+        -- | k
+        CUInt ->
+        -- | n
+        CUInt ->
+        IO (Ptr CFEC)
 foreign import ccall unsafe "&fec_free" _free :: FunPtr (Ptr CFEC -> IO ())
-foreign import ccall unsafe "fec_encode" _encode :: Ptr CFEC
-                                                 -> Ptr (Ptr Word8)  -- ^ primary blocks
-                                                 -> Ptr (Ptr Word8)  -- ^ (output) secondary blocks
-                                                 -> Ptr CUInt  -- ^ array of secondary block ids
-                                                 -> CSize  -- ^ length of previous
-                                                 -> CSize  -- ^ block length
-                                                 -> IO ()
-foreign import ccall unsafe "fec_decode" _decode :: Ptr CFEC
-                                                 -> Ptr (Ptr Word8)  -- ^ input blocks
-                                                 -> Ptr (Ptr Word8)  -- ^ output blocks
-                                                 -> Ptr CUInt  -- ^ array of input indexes
-                                                 -> CSize  -- ^ block length
-                                                 -> IO ()
+foreign import ccall unsafe "fec_encode"
+    _encode ::
+        Ptr CFEC ->
+        -- | primary blocks
+        Ptr (Ptr Word8) ->
+        -- | (output) secondary blocks
+        Ptr (Ptr Word8) ->
+        -- | array of secondary block ids
+        Ptr CUInt ->
+        -- | length of previous
+        CSize ->
+        -- | block length
+        CSize ->
+        IO ()
+foreign import ccall unsafe "fec_decode"
+    _decode ::
+        Ptr CFEC ->
+        -- | input blocks
+        Ptr (Ptr Word8) ->
+        -- | output blocks
+        Ptr (Ptr Word8) ->
+        -- | array of input indexes
+        Ptr CUInt ->
+        -- | block length
+        CSize ->
+        IO ()
 
 -- | Return true if the given @k@ and @n@ values are valid
 isValidConfig :: Int -> Int -> Bool
 isValidConfig k n
-  | k > n = False
-  | k < 1 = False
-  | n < 1 = False
-  | n > 255 = False
-  | otherwise = True
+    | k > n = False
+    | k < 1 = False
+    | n < 1 = False
+    | n > 255 = False
+    | otherwise = True
 
 -- | Return a FEC with the given parameters.
-fec :: Int  -- ^ the number of primary blocks
-    -> Int  -- ^ the total number blocks, must be < 256
-    -> FECParams
+fec ::
+    -- | the number of primary blocks
+    Int ->
+    -- | the total number blocks, must be < 256
+    Int ->
+    FECParams
 fec k n =
-  if not (isValidConfig k n)
-     then error $ "Invalid FEC parameters: " ++ show k ++ " " ++ show n
-     else unsafePerformIO (do
-       cfec <- _new (fromIntegral k) (fromIntegral n)
-       params <- newForeignPtr _free cfec
-       return $ FECParams params k n)
+    if not (isValidConfig k n)
+        then error $ "Invalid FEC parameters: " ++ show k ++ " " ++ show n
+        else
+            unsafePerformIO
+                ( do
+                    cfec <- _new (fromIntegral k) (fromIntegral n)
+                    params <- newForeignPtr _free cfec
+                    return $ FECParams params k n
+                )
 
 -- | Create a C array of unsigned from an input array
 uintCArray :: [Int] -> ((Ptr CUInt) -> IO a) -> IO a
@@ -98,153 +123,235 @@ uintCArray xs f = withArray (map fromIntegral xs) f
 -- | Convert a list of ByteStrings to an array of pointers to their data
 byteStringsToArray :: [B.ByteString] -> ((Ptr (Ptr Word8)) -> IO a) -> IO a
 byteStringsToArray inputs f = do
-  let l = length inputs
-  allocaBytes (l * sizeOf (undefined :: Ptr Word8)) (\array -> do
-    let inner _ [] = f array
-        inner array' (bs : bss) = BU.unsafeUseAsCString bs (\ptr -> do
-          poke array' $ castPtr ptr
-          inner (advancePtr array' 1) bss)
-    inner array inputs)
+    let l = length inputs
+    allocaBytes
+        (l * sizeOf (undefined :: Ptr Word8))
+        ( \array -> do
+            let inner _ [] = f array
+                inner array' (bs : bss) =
+                    BU.unsafeUseAsCString
+                        bs
+                        ( \ptr -> do
+                            poke array' $ castPtr ptr
+                            inner (advancePtr array' 1) bss
+                        )
+            inner array inputs
+        )
 
 -- | Return True iff all the given ByteStrings are the same length
 allByteStringsSameLength :: [B.ByteString] -> Bool
 allByteStringsSameLength [] = True
 allByteStringsSameLength (bs : bss) = all ((==) (B.length bs)) $ map B.length bss
 
--- | Run the given function with a pointer to an array of @n@ pointers to
---   buffers of size @size@. Return these buffers as a list of ByteStrings
-createByteStringArray :: Int  -- ^ the number of buffers requested
-                      -> Int  -- ^ the size of each buffer
-                      -> ((Ptr (Ptr Word8)) -> IO ())
-                      -> IO [B.ByteString]
+{- | Run the given function with a pointer to an array of @n@ pointers to
+   buffers of size @size@. Return these buffers as a list of ByteStrings
+-}
+createByteStringArray ::
+    -- | the number of buffers requested
+    Int ->
+    -- | the size of each buffer
+    Int ->
+    ((Ptr (Ptr Word8)) -> IO ()) ->
+    IO [B.ByteString]
 createByteStringArray n size f = do
-  allocaBytes (n * sizeOf (undefined :: Ptr Word8)) (\array -> do
-    allocaBytes (n * size) (\ptr -> do
-      mapM_ (\i -> poke (advancePtr array i) (advancePtr ptr (size * i))) [0..(n - 1)]
-      f array
-      mapM (\i -> B.packCStringLen (castPtr $ advancePtr ptr (i * size), size)) [0..(n - 1)]))
-
--- | Generate the secondary blocks from a list of the primary blocks. The
---   primary blocks must be in order and all of the same size. There must be
---   @k@ primary blocks.
-encode :: FECParams
-       -> [B.ByteString]  -- ^ a list of @k@ input blocks
-       -> [B.ByteString]  -- ^ (n - k) output blocks
+    allocaBytes
+        (n * sizeOf (undefined :: Ptr Word8))
+        ( \array -> do
+            allocaBytes
+                (n * size)
+                ( \ptr -> do
+                    mapM_ (\i -> poke (advancePtr array i) (advancePtr ptr (size * i))) [0 .. (n - 1)]
+                    f array
+                    mapM (\i -> B.packCStringLen (castPtr $ advancePtr ptr (i * size), size)) [0 .. (n - 1)]
+                )
+        )
+
+{- | Generate the secondary blocks from a list of the primary blocks. The
+   primary blocks must be in order and all of the same size. There must be
+   @k@ primary blocks.
+-}
+encode ::
+    FECParams ->
+    -- | a list of @k@ input blocks
+    [B.ByteString] ->
+    -- | (n - k) output blocks
+    [B.ByteString]
 encode (FECParams params k n) inblocks
-  | length inblocks /= k = error "Wrong number of blocks to FEC encode"
-  | not (allByteStringsSameLength inblocks) = error "Not all inputs to FEC encode are the same length"
-  | otherwise = unsafePerformIO (do
-      let sz = B.length $ head inblocks
-      withForeignPtr params (\cfec -> do
-        byteStringsToArray inblocks (\src -> do
-          createByteStringArray (n - k) sz (\fecs -> do
-            uintCArray [k..(n - 1)] (\block_nums -> do
-              _encode cfec src fecs block_nums (fromIntegral (n - k)) $ fromIntegral sz)))))
+    | length inblocks /= k = error "Wrong number of blocks to FEC encode"
+    | not (allByteStringsSameLength inblocks) = error "Not all inputs to FEC encode are the same length"
+    | otherwise =
+        unsafePerformIO
+            ( do
+                let sz = B.length $ head inblocks
+                withForeignPtr
+                    params
+                    ( \cfec -> do
+                        byteStringsToArray
+                            inblocks
+                            ( \src -> do
+                                createByteStringArray
+                                    (n - k)
+                                    sz
+                                    ( \fecs -> do
+                                        uintCArray
+                                            [k .. (n - 1)]
+                                            ( \block_nums -> do
+                                                _encode cfec src fecs block_nums (fromIntegral (n - k)) $ fromIntegral sz
+                                            )
+                                    )
+                            )
+                    )
+            )
 
 -- | A sort function for tagged assoc lists
 sortTagged :: [(Int, a)] -> [(Int, a)]
 sortTagged = sortBy (\a b -> compare (fst a) (fst b))
 
--- | Reorder the given list so that elements with tag numbers < the first
---   argument have an index equal to their tag number (if possible)
+{- | Reorder the given list so that elements with tag numbers < the first
+   argument have an index equal to their tag number (if possible)
+-}
 reorderPrimaryBlocks :: Int -> [(Int, a)] -> [(Int, a)]
-reorderPrimaryBlocks n blocks = inner (sortTagged pBlocks) sBlocks [] where
-  (pBlocks, sBlocks) = partition (\(tag, _) -> tag < n) blocks
-  inner [] sBlocks acc = acc ++ sBlocks
-  inner pBlocks [] acc = acc ++ pBlocks
-  inner pBlocks@((tag, a) : ps) sBlocks@(s : ss) acc =
-    if length acc == tag
-       then inner ps sBlocks (acc ++ [(tag, a)])
-       else inner pBlocks ss (acc ++ [s])
-
--- | Recover the primary blocks from a list of @k@ blocks. Each block must be
---   tagged with its number (see the module comments about block numbering)
-decode :: FECParams
-       -> [(Int, B.ByteString)]  -- ^ a list of @k@ blocks and their index
-       -> [B.ByteString]  -- ^ a list the @k@ primary blocks
+reorderPrimaryBlocks n blocks = inner (sortTagged pBlocks) sBlocks []
+  where
+    (pBlocks, sBlocks) = partition (\(tag, _) -> tag < n) blocks
+    inner [] sBlocks acc = acc ++ sBlocks
+    inner pBlocks [] acc = acc ++ pBlocks
+    inner pBlocks@((tag, a) : ps) sBlocks@(s : ss) acc =
+        if length acc == tag
+            then inner ps sBlocks (acc ++ [(tag, a)])
+            else inner pBlocks ss (acc ++ [s])
+
+{- | Recover the primary blocks from a list of @k@ blocks. Each block must be
+   tagged with its number (see the module comments about block numbering)
+-}
+decode ::
+    FECParams ->
+    -- | a list of @k@ blocks and their index
+    [(Int, B.ByteString)] ->
+    -- | a list the @k@ primary blocks
+    [B.ByteString]
 decode (FECParams params k n) inblocks
-  | length (nub $ map fst inblocks) /= length (inblocks) = error "Duplicate input blocks in FEC decode"
-  | any (\f -> f < 0 || f >= n) $ map fst inblocks = error "Invalid block numbers in FEC decode"
-  | length inblocks /= k = error "Wrong number of blocks to FEC decode"
-  | not (allByteStringsSameLength $ map snd inblocks) = error "Not all inputs to FEC decode are same length"
-  | otherwise = unsafePerformIO (do
-      let sz = B.length $ snd $ head inblocks
-          inblocks' = reorderPrimaryBlocks k inblocks
-          presentBlocks = map fst inblocks'
-      withForeignPtr params (\cfec -> do
-        byteStringsToArray (map snd inblocks') (\src -> do
-          b <- createByteStringArray (n - k) sz (\out -> do
-                 uintCArray presentBlocks (\block_nums -> do
-                   _decode cfec src out block_nums $ fromIntegral sz))
-          let blocks = [0..(n - 1)] \\ presentBlocks
-              tagged = zip blocks b
-              allBlocks = sortTagged $ tagged ++ inblocks'
-          return $ take k $ map snd allBlocks)))
-
--- | Break a ByteString into @n@ parts, equal in length to the original, such
---   that all @n@ are required to reconstruct the original, but having less
---   than @n@ parts reveals no information about the orginal.
---
---   This code works in IO monad because it needs a source of random bytes,
---   which it gets from /dev/urandom. If this file doesn't exist an
---   exception results
---
---   Not terribly fast - probably best to do it with short inputs (e.g. an
---   encryption key)
-secureDivide :: Int  -- ^ the number of parts requested
-             -> B.ByteString  -- ^ the data to be split
-             -> IO [B.ByteString]
+    | length (nub $ map fst inblocks) /= length (inblocks) = error "Duplicate input blocks in FEC decode"
+    | any (\f -> f < 0 || f >= n) $ map fst inblocks = error "Invalid block numbers in FEC decode"
+    | length inblocks /= k = error "Wrong number of blocks to FEC decode"
+    | not (allByteStringsSameLength $ map snd inblocks) = error "Not all inputs to FEC decode are same length"
+    | otherwise =
+        unsafePerformIO
+            ( do
+                let sz = B.length $ snd $ head inblocks
+                    inblocks' = reorderPrimaryBlocks k inblocks
+                    presentBlocks = map fst inblocks'
+                withForeignPtr
+                    params
+                    ( \cfec -> do
+                        byteStringsToArray
+                            (map snd inblocks')
+                            ( \src -> do
+                                b <-
+                                    createByteStringArray
+                                        (n - k)
+                                        sz
+                                        ( \out -> do
+                                            uintCArray
+                                                presentBlocks
+                                                ( \block_nums -> do
+                                                    _decode cfec src out block_nums $ fromIntegral sz
+                                                )
+                                        )
+                                let blocks = [0 .. (n - 1)] \\ presentBlocks
+                                    tagged = zip blocks b
+                                    allBlocks = sortTagged $ tagged ++ inblocks'
+                                return $ take k $ map snd allBlocks
+                            )
+                    )
+            )
+
+{- | Break a ByteString into @n@ parts, equal in length to the original, such
+   that all @n@ are required to reconstruct the original, but having less
+   than @n@ parts reveals no information about the orginal.
+
+   This code works in IO monad because it needs a source of random bytes,
+   which it gets from /dev/urandom. If this file doesn't exist an
+   exception results
+
+   Not terribly fast - probably best to do it with short inputs (e.g. an
+   encryption key)
+-}
+secureDivide ::
+    -- | the number of parts requested
+    Int ->
+    -- | the data to be split
+    B.ByteString ->
+    IO [B.ByteString]
 secureDivide n input
-  | n < 0 = error "secureDivide called with negative number of parts"
-  | otherwise = withFile "/dev/urandom" ReadMode (\handle -> do
-      let inner 1 bs = return [bs]
-          inner n bs = do
-            mask <- B.hGet handle (B.length bs)
-            let masked = B.pack $ B.zipWith xor bs mask
-            rest <- inner (n - 1) masked
-            return (mask : rest)
-      inner n input)
-
--- | Reverse the operation of secureDivide. The order of the inputs doesn't
---   matter, but they must all be the same length
+    | n < 0 = error "secureDivide called with negative number of parts"
+    | otherwise =
+        withFile
+            "/dev/urandom"
+            ReadMode
+            ( \handle -> do
+                let inner 1 bs = return [bs]
+                    inner n bs = do
+                        mask <- B.hGet handle (B.length bs)
+                        let masked = B.pack $ B.zipWith xor bs mask
+                        rest <- inner (n - 1) masked
+                        return (mask : rest)
+                inner n input
+            )
+
+{- | Reverse the operation of secureDivide. The order of the inputs doesn't
+   matter, but they must all be the same length
+-}
 secureCombine :: [B.ByteString] -> B.ByteString
 secureCombine [] = error "Passed empty list of inputs to secureCombine"
 secureCombine [a] = a
 secureCombine [a, b] = B.pack $ B.zipWith xor a b
 secureCombine (a : rest) = B.pack $ B.zipWith xor a $ secureCombine rest
 
--- | A utility function which takes an arbitary input and FEC encodes it into a
---   number of blocks. The order the resulting blocks doesn't matter so long
---   as you have enough to present to @deFEC@.
-enFEC :: Int  -- ^ the number of blocks required to reconstruct
-      -> Int  -- ^ the total number of blocks
-      -> B.ByteString  -- ^ the data to divide
-      -> [B.ByteString]  -- ^ the resulting blocks
-enFEC k n input = taggedPrimaryBlocks ++ taggedSecondaryBlocks where
-  taggedPrimaryBlocks = map (uncurry B.cons) $ zip [0..] primaryBlocks
-  taggedSecondaryBlocks = map (uncurry B.cons) $ zip [(fromIntegral k)..] secondaryBlocks
-  remainder = B.length input `mod` k
-  paddingLength = if remainder >= 1 then (k - remainder) else k
-  paddingBytes = (B.replicate (paddingLength - 1) 0) `B.append` (B.singleton $ fromIntegral paddingLength)
-  divide a bs
-    | B.null bs = []
-    | otherwise = (B.take a bs) : (divide a $ B.drop a bs)
-  input' = input `B.append` paddingBytes
-  blockSize = B.length input' `div` k
-  primaryBlocks = divide blockSize input'
-  secondaryBlocks = encode params primaryBlocks
-  params = fec k n
+{- | A utility function which takes an arbitary input and FEC encodes it into a
+   number of blocks. The order the resulting blocks doesn't matter so long
+   as you have enough to present to @deFEC@.
+-}
+enFEC ::
+    -- | the number of blocks required to reconstruct
+    Int ->
+    -- | the total number of blocks
+    Int ->
+    -- | the data to divide
+    B.ByteString ->
+    -- | the resulting blocks
+    [B.ByteString]
+enFEC k n input = taggedPrimaryBlocks ++ taggedSecondaryBlocks
+  where
+    taggedPrimaryBlocks = map (uncurry B.cons) $ zip [0 ..] primaryBlocks
+    taggedSecondaryBlocks = map (uncurry B.cons) $ zip [(fromIntegral k) ..] secondaryBlocks
+    remainder = B.length input `mod` k
+    paddingLength = if remainder >= 1 then (k - remainder) else k
+    paddingBytes = (B.replicate (paddingLength - 1) 0) `B.append` (B.singleton $ fromIntegral paddingLength)
+    divide a bs
+        | B.null bs = []
+        | otherwise = (B.take a bs) : (divide a $ B.drop a bs)
+    input' = input `B.append` paddingBytes
+    blockSize = B.length input' `div` k
+    primaryBlocks = divide blockSize input'
+    secondaryBlocks = encode params primaryBlocks
+    params = fec k n
 
 -- | Reverses the operation of @enFEC@.
-deFEC :: Int  -- ^ the number of blocks required (matches call to @enFEC@)
-      -> Int  -- ^ the total number of blocks (matches call to @enFEC@)
-      -> [B.ByteString]  -- ^ a list of k, or more, blocks from @enFEC@
-      -> B.ByteString
+deFEC ::
+    -- | the number of blocks required (matches call to @enFEC@)
+    Int ->
+    -- | the total number of blocks (matches call to @enFEC@)
+    Int ->
+    -- | a list of k, or more, blocks from @enFEC@
+    [B.ByteString] ->
+    B.ByteString
 deFEC k n inputs
-  | length inputs < k = error "Too few inputs to deFEC"
-  | otherwise = B.take (B.length fecOutput - paddingLength) fecOutput where
-      paddingLength = fromIntegral $ B.last fecOutput
-      inputs' = take k inputs
-      taggedInputs = map (\bs -> (fromIntegral $ B.head bs, B.tail bs)) inputs'
-      fecOutput = B.concat $ decode params taggedInputs
-      params = fec k n
+    | length inputs < k = error "Too few inputs to deFEC"
+    | otherwise = B.take (B.length fecOutput - paddingLength) fecOutput
+  where
+    paddingLength = fromIntegral $ B.last fecOutput
+    inputs' = take k inputs
+    taggedInputs = map (\bs -> (fromIntegral $ B.head bs, B.tail bs)) inputs'
+    fecOutput = B.concat $ decode params taggedInputs
+    params = fec k n


Test patch:
diff --git a/haskell/test/FECTest.hs b/haskell/test/FECTest.hs
index a1b3da22..0e22606f 100644
--- a/haskell/test/FECTest.hs
+++ b/haskell/test/FECTest.hs
@@ -36,8 +36,8 @@ instance Arbitrary Params where
 
 instance Arbitrary FEC.FECParams where
     arbitrary = do
-      (Params required total) <- arbitrary :: Gen Params
-      return $ FEC.fec required total
+        (Params required total) <- arbitrary :: Gen Params
+        return $ FEC.fec required total
 
 randomTake :: Int -> Int -> [a] -> [a]
 randomTake seed n values = map snd $ take n sortedValues
@@ -48,21 +48,21 @@ randomTake seed n values = map snd $ take n sortedValues
     rnds = randoms gen
     gen = mkStdGen seed
 
--- | Any combination of the inputs blocks and the output blocks from
--- @FEC.encode@, as long as there are at least @k@ of them, can be recombined
--- using @FEC.decode@ to produce the original input blocks.
---
-testFEC
-  :: FEC.FECParams
-  -- ^ The FEC parameters to exercise.
-  -> Word16
-  -- ^ The length of the blocks to exercise.
-  -> Int
-  -- ^ A random seed to use to be able to vary the choice of which blocks to
-  -- try to decode.
-  -> Bool
-  -- ^ True if the encoded input was reconstructed by decoding, False
-  -- otherwise.
+{- | Any combination of the inputs blocks and the output blocks from
+ @FEC.encode@, as long as there are at least @k@ of them, can be recombined
+ using @FEC.decode@ to produce the original input blocks.
+-}
+testFEC ::
+    -- | The FEC parameters to exercise.
+    FEC.FECParams ->
+    -- | The length of the blocks to exercise.
+    Word16 ->
+    -- | A random seed to use to be able to vary the choice of which blocks to
+    -- try to decode.
+    Int ->
+    -- | True if the encoded input was reconstructed by decoding, False
+    -- otherwise.
+    Bool
 testFEC fec len seed = FEC.decode fec someTaggedBlocks == origBlocks
   where
     -- Construct some blocks.  Each will just be the byte corresponding to the
@@ -84,9 +84,9 @@ testFEC fec len seed = FEC.decode fec someTaggedBlocks == origBlocks
 -- | @FEC.secureDivide@ is the inverse of @FEC.secureCombine@.
 prop_divide :: Word16 -> Word8 -> Word8 -> Property
 prop_divide size byte divisor = monadicIO $ do
-  let input = B.replicate (fromIntegral size + 1) byte
-  parts <- run $ FEC.secureDivide (fromIntegral divisor) input
-  assert (FEC.secureCombine parts == input)
+    let input = B.replicate (fromIntegral size + 1) byte
+    parts <- run $ FEC.secureDivide (fromIntegral divisor) input
+    assert (FEC.secureCombine parts == input)
 
 -- | @FEC.encode@ is the inverse of @FEC.decode@.
 prop_decode :: FEC.FECParams -> Word16 -> Int -> Property
@@ -95,7 +95,7 @@ prop_decode fec len seed = property $ testFEC fec len seed
 -- | @FEC.enFEC@ is the inverse of @FEC.deFEC@.
 prop_deFEC :: Params -> B.ByteString -> Property
 prop_deFEC (Params required total) testdata =
-  FEC.deFEC required total minimalShares === testdata
+    FEC.deFEC required total minimalShares === testdata
   where
     allShares = FEC.enFEC required total testdata
     minimalShares = take required allShares