[CO-354] Fix SafeCopy instance for AddrAttributes

From @dcoutts:
> The uncheckedLookAhead is actually a bit dangerous
> and rather misleading. It only does what you expect
> if all the input is supplied in one chunk, since it
> only looks up to the end of the chunk. If acid-state
> uses the incremental interface to deserialise in e.g.
> 4k chunks then this can fail at a chunk boundary.

I switch to using `Alternative` to run the non-legacy
parser first, and if it fails, to run the legacy parser.
This seems to leverage Cereal's `fail` mechanism the way
it is intended - when the first parser fails, no input is
consumed, and the second parser is run.

Author: Michael Hueschen

lib/src/Pos/SafeCopy.hs

@@ -12,7 +12,6 @@ import qualified Cardano.Crypto.Wallet as CC
 import qualified Cardano.Crypto.Wallet.Encrypted as CC
 import qualified Crypto.Math.Edwards25519 as ED25519
 import qualified Crypto.Sign.Ed25519 as EDS25519
-import qualified Data.ByteString.Lazy as BSL
 import           Data.SafeCopy (Contained, SafeCopy (..), base, contain, deriveSafeCopySimple,
                                 safeGet, safePut)
 import qualified Data.Serialize as Cereal
@@ -147,31 +146,21 @@ instance SafeCopy AddrAttributes where
         let bs = Bi.serialize (mkAttributes aa)
         safePut bs
 
-    getCopy = contain $ do
-        let label = Cereal.label "Pos.Core.Common.AddrAttributes.AddrAttributes:"
-
-        let getLegacy =
-                (\apdp asd -> AddrAttributes apdp asd NMNothing)
-                    <$> safeGet
-                    <*> safeGet
-
-        -- ByteStrings are prefixed with a Int64 length. We cheat here and read the length as
-        -- thought it were a safePut-encoded Int64, so we know how long the ByteString will be.
+    -- Try decoding as a BSL.ByteString containing the new format, but if that
+    -- fails go for the legacy format.
+    getCopy = contain $ label $ getNonLegacy <|> getLegacy
+      where
+        label = Cereal.label "Pos.Core.Common.AddrAttributes.AddrAttributes:"
+        --
+        getNonLegacy = do
+            eAAA <- Bi.decodeFull <$> safeGet
+            case eAAA of
+                Left  err -> fail (show err)
+                Right aaa -> pure (attrData aaa)
         --
-        -- 4[version] + 8[Int64] + <bytesLen>
-        -- bytesLen == length of AddrAttributes bytestring
-        bytesLen <- Cereal.lookAhead safeGet
-        bytes <- BSL.fromStrict <$> Cereal.uncheckedLookAhead (fromIntegral bytesLen + 12)
-        let _aaaVersionBytes  = BSL.take 4 bytes
-            attrAddrAttrBytes = BSL.drop 12 bytes
-        label $ if BSL.length attrAddrAttrBytes /= bytesLen
-                   then getLegacy
-                   else case Bi.decodeFull attrAddrAttrBytes of
-                            Left _              -> getLegacy
-                            Right attrAddrAttrs -> do
-                                -- seek ahead since we passed our bytes
-                                Cereal.uncheckedSkip (12 + fromIntegral bytesLen)
-                                pure (attrData attrAddrAttrs)
+        getLegacy = AddrAttributes <$> safeGet
+                                   <*> safeGet
+                                   <*> pure NMNothing
 
 deriveSafeCopySimple 0 'base ''Address'
 deriveSafeCopySimple 0 'base ''Address