Merge #966

966: More tests for byron wallets migration, in particular migrating from big wallets r=piotr-iohk a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#779 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have added an extra integration scenario to test migrating a big wallet with a lot of dust
- [x] I have revised byron migrate response behavior when trying to migrate an empty wallet to return a 403. Two reasons for this: this is more semantically correct. It prevents some extra code needed to handle possible errors that could have occurred if we indeed tried migrating (like, an invalid passphrase).
- [x] enable a currently pending test in BYRON_TX_LIST which required faucet
- [x] Added extra negative scenario to check migrating with a wrong passphrase 
- [x] Revised endpoints status in swagger.yaml + added missing error code in migration response

# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <matthias.benkort@gmail.com>

lib/core-integration/src/Test/Integration/Framework/TestData.hs

@@ -53,6 +53,7 @@ module Test.Integration.Framework.TestData
     , errMsg403UTxO
     , errMsg403WrongPass
     , errMsg403NoPendingAnymore
+    , errMsg403NothingToMigrate
     , errMsg404NoEndpoint
     , errMsg404CannotFindTx
     , errMsg404NoRootKey
@@ -341,6 +342,12 @@ errMsg403WrongPass :: String
 errMsg403WrongPass = "The given encryption passphrase doesn't match the one\
     \ I use to encrypt the root private key of the given wallet"
 
+errMsg403NothingToMigrate :: Text -> String
+errMsg403NothingToMigrate wid =
+    "I can't migrate the wallet with the given id: " ++ unpack wid ++
+    ", because it's either empty or full of small coins which wouldn't be \
+    \worth migrating."
+
 errMsg404NoEndpoint :: String
 errMsg404NoEndpoint = "I couldn't find the requested endpoint. If the endpoint\
     \ contains path parameters, please ensure they are well-formed, otherwise I\

lib/core-integration/src/Test/Integration/Scenario/API/ByronTransactions.hs

@@ -22,7 +22,7 @@ import Control.Monad
 import Data.Generics.Internal.VL.Lens
     ( (^.) )
 import Test.Hspec
-    ( SpecWith, describe, it, pendingWith )
+    ( SpecWith, describe, it )
 import Test.Integration.Framework.DSL
     ( Context
     , Headers (..)
@@ -33,6 +33,7 @@ import Test.Integration.Framework.DSL
     , expectErrorMessage
     , expectListSizeEqual
     , expectResponseCode
+    , fixtureByronWallet
     , listByronTxEp
     , request
     , toQueryString
@@ -70,14 +71,12 @@ spec = do
             ]
 
     it "BYRON_TX_LIST_01 - Can list transactions on Byron Wallet" $ \ctx -> do
-        pendingWith "Blocked by #849"
-        -- TODO make it fixtureByronWallet after #849 and adjust expectations
-        w <- emptyByronWallet ctx
+        w <- fixtureByronWallet ctx
         r <- request @([ApiTransaction n]) ctx (listByronTxEp w mempty)
             Default Empty
         verify r
             [ expectResponseCode @IO HTTP.status200
-            , expectListSizeEqual 0
+            , expectListSizeEqual 10
             ]
 
     describe "BYRON_TX_LIST_01 - Faulty start, end, order values" $ do

lib/core-integration/src/Test/Integration/Scenario/API/ByronWallets.hs

@@ -16,6 +16,7 @@ import Cardano.Wallet.Api.Types
     ( ApiByronWallet
     , ApiByronWalletMigrationInfo (..)
     , ApiTransaction
+    , ApiUtxoStatistics
     , ApiWallet
     )
 import Cardano.Wallet.Primitive.AddressDerivation
@@ -70,6 +71,7 @@ import Test.Integration.Framework.DSL
     , getByronWalletEp
     , getFromResponse
     , getWalletEp
+    , getWalletUtxoEp
     , json
     , listByronWalletsEp
     , listWalletsEp
@@ -78,12 +80,15 @@ import Test.Integration.Framework.DSL
     , postByronWalletEp
     , request
     , state
+    , unsafeRequest
     , verify
     , walletId
     , walletName
     )
 import Test.Integration.Framework.TestData
     ( arabicWalletName
+    , errMsg403NothingToMigrate
+    , errMsg403WrongPass
     , errMsg404NoEndpoint
     , errMsg404NoWallet
     , errMsg405
@@ -112,6 +117,7 @@ import Test.Integration.Framework.TestData
     )
 
 import qualified Cardano.Wallet.Api.Types as ApiTypes
+import qualified Data.Map.Strict as Map
 import qualified Data.Text as T
 import qualified Network.HTTP.Types.Status as HTTP
 
@@ -145,16 +151,17 @@ spec = do
                 ]
 
     it "BYRON_MIGRATE_02 - \
-        \migrating an empty wallet should not generate transactions."
+        \migrating an empty wallet should fail."
         $ \ctx -> do
             sourceWallet <- emptyByronWallet ctx
             targetWallet <- emptyWallet ctx
-            let payload = Json [json|{"passphrase": #{fixturePassphrase}}|]
+            let payload = Json [json|{"passphrase": "Secure Passphrase"}|]
             let ep = migrateByronWalletEp sourceWallet targetWallet
             r <- request @[ApiTransaction n] ctx ep Default payload
+            let srcId = sourceWallet ^. walletId
             verify r
-                [ expectResponseCode @IO HTTP.status202
-                , expectFieldSatisfy id null
+                [ expectResponseCode @IO HTTP.status403
+                , expectErrorMessage (errMsg403NothingToMigrate srcId)
                 ]
 
     it "BYRON_MIGRATE_03 - \
@@ -265,6 +272,91 @@ spec = do
             expectResponseCode @IO HTTP.status404 rg
             expectErrorMessage errMsg404NoEndpoint rg
 
+    it "BYRON_MIGRATE_07 - \
+        \ migrate a big wallet requiring more than one tx" $ \ctx -> do
+        -- NOTE
+        -- Special mnemonic for which 500 legacy funds are attached to in the
+        -- genesis file.
+        --
+        -- Out of these 500 coins, 100 of them are of 1 Lovelace and are
+        -- expected to be treated as dust. The rest are all worth: 10000000000
+        -- lovelace.
+        let mnemonics =
+                ["collect", "fold", "file", "clown"
+                , "injury", "sun", "brass", "diet"
+                , "exist", "spike", "behave", "clip"
+                ] :: [Text]
+        let payloadRestore = Json [json| {
+                "name": "Big Byron Wallet",
+                "mnemonic_sentence": #{mnemonics},
+                "passphrase": #{fixturePassphrase}
+                } |]
+        (_, wOld) <- unsafeRequest @ApiByronWallet ctx
+            postByronWalletEp payloadRestore
+        eventually $ do
+            request @ApiByronWallet ctx
+                (getByronWalletEp wOld)
+                Default
+                Empty >>= flip verify
+                [ expectFieldSatisfy balanceAvailable (> 0)
+                ]
+        let originalBalance = view balanceAvailable wOld
+
+        -- Calculate the expected migration fee:
+        rFee <- request @ApiByronWalletMigrationInfo ctx
+            (calculateByronMigrationCostEp wOld)
+            Default
+            Empty
+        verify rFee
+            [ expectResponseCode @IO HTTP.status200
+            , expectFieldSatisfy amount (> 0)
+            ]
+        let expectedFee = getFromResponse amount rFee
+
+        -- Migrate to a new empty wallet
+        wNew <- emptyWallet ctx
+        let payloadMigrate = Json [json|{"passphrase": #{fixturePassphrase}}|]
+        request @[ApiTransaction n] ctx
+            (migrateByronWalletEp wOld wNew)
+            Default
+            payloadMigrate >>= flip verify
+            [ expectResponseCode @IO HTTP.status202
+            , expectFieldSatisfy id ((== 12). length)
+            ]
+
+        -- Check that funds become available in the target wallet:
+        let expectedBalance = originalBalance - expectedFee
+        eventually $ do
+            request @ApiWallet ctx
+                (getWalletEp wNew)
+                Default
+                Empty >>= flip verify
+                [ expectFieldEqual balanceAvailable expectedBalance
+                , expectFieldEqual balanceTotal     expectedBalance
+                ]
+
+        -- Analyze the target wallet UTxO distribution
+        request @ApiUtxoStatistics ctx (getWalletUtxoEp wNew)
+            Default
+            Empty >>= flip verify
+            [ expectFieldSatisfy #distribution ((== (Just 400)). Map.lookup 10000000000)
+            ]
+
+    it "BYRON_MIGRATE_08 - fails with a wrong passphrase" $ \ctx -> do
+        -- Restore a Byron wallet with funds, to act as a source wallet:
+        sourceWallet <- fixtureByronWallet ctx
+
+        -- Perform a migration from the source wallet to a target wallet:
+        targetWallet <- emptyWallet ctx
+        r0 <- request @[ApiTransaction n] ctx
+            (migrateByronWalletEp sourceWallet targetWallet )
+            Default
+            (Json [json|{"passphrase": "not-the-right-passphrase"}|])
+        verify r0
+            [ expectResponseCode @IO HTTP.status403
+            , expectErrorMessage errMsg403WrongPass
+            ]
+
     it "BYRON_GET_02 - Byron ep does not show Shelley wallet" $ \ctx -> do
         w <- emptyWallet ctx
         let wid = w ^. walletId

lib/core/src/Cardano/Wallet/Api/Server.hs

@@ -189,7 +189,7 @@ import Control.Exception
     , tryJust
     )
 import Control.Monad
-    ( forM, forM_, void )
+    ( forM, forM_, void, when )
 import Control.Monad.IO.Class
     ( MonadIO, liftIO )
 import Control.Monad.Trans.Except
@@ -921,6 +921,8 @@ migrateByronWallet rndCtx seqCtx (ApiT rndWid) (ApiT seqWid) migrateData = do
             cs <- W.createMigrationSourceData @_ @_ @t rndWrk rndWid
             W.assignMigrationTargetAddresses seqWrk seqWid () cs
 
+    when (null cs) $ liftHandler $ throwE $ ErrMigratingEmptyWallet rndWid
+
     forM cs $ \selection -> do
         (tx, meta, time, wit) <- liftHandler
             $ withWorkerCtx rndCtx rndWid (throwE . ErrSignTxNoSuchWallet)
@@ -1270,10 +1272,25 @@ data ErrCreateWallet
         -- ^ Somehow, we couldn't create a worker or open a db connection
     deriving (Eq, Show)
 
+newtype ErrMigrateWallet
+    = ErrMigratingEmptyWallet WalletId
+        -- ^ User attempted to migrate an empty wallet
+    deriving (Eq, Show)
+
 -- | Small helper to easy show things to Text
 showT :: Show a => a -> Text
 showT = T.pack . show
 
+instance LiftHandler ErrMigrateWallet where
+    handler = \case
+        ErrMigratingEmptyWallet wid ->
+            apiError err403 NothingToMigrate $ mconcat
+                [ "I can't migrate the wallet with the given id: "
+                , toText wid
+                , ", because it's either empty or full of small coins "
+                , "which wouldn't be worth migrating."
+                ]
+
 instance LiftHandler ErrNoSuchWallet where
     handler = \case
         ErrNoSuchWallet wid ->

lib/core/src/Cardano/Wallet/Api/Types.hs

@@ -331,6 +331,7 @@ data ApiErrorCode
     | UnsupportedMediaType
     | UnexpectedError
     | NotSynced
+    | NothingToMigrate
     deriving (Eq, Generic, Show)
 
 -- | Defines a point in time that can be formatted as and parsed from an