Merge #1865

1865: withdrawals as part of the coin selection r=KtorZ a=KtorZ

# Issue Number

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

#1861 

# Overview

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

- 12b28f7
  📍 **take withdrawals into account, one level earlier, during the coin selection**
  Still to be done:

    - Make sure it's correctly done in the largest-first algorithm
    - Add some test scenario that show the influence of the withdrawal

- 547b130
  📍 **unit-test the newly introduced 'proportionallyTo'**
  Getting this one wrong would be quite bad :s

- 975f726
  📍 **add unit tests showing how withdrawal impacts the random coin selection**


> 💡 NOTE
>
> I've chosen not to treat the withdrawal as a single _input_, but more as a "money pot" that is proportionally distributed amongst change output, so that it contributes to every output, based on their size. This is to avoid having a small output consuming the entire withdrawal for itself. Note sure if I'll keep the approach in the end, I'll have the night to think about it.

# Comments

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

- [ ] TODO: take into account the withdrawal when doing largest-first (and testing it)
- [ ] TODO: echo the error message change on "ErrInputsDepleted" to integration tests relying on that message.

<!-- 
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
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


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

lib/byron/test/integration/Test/Integration/Byron/Scenario/API/Transactions.hs

@@ -79,7 +79,6 @@ import Test.Integration.Framework.DSL
     , fixtureRandomWallet
     , fixtureRandomWalletAddrs
     , fixtureRandomWalletMws
-    , fixtureRandomWalletWith
     , getFromResponse
     , icarusAddresses
     , json
@@ -93,9 +92,7 @@ import Test.Integration.Framework.Request
     ( RequestException )
 import Test.Integration.Framework.TestData
     ( errMsg403Fee
-    , errMsg403InputsDepleted
     , errMsg403NotEnoughMoney_
-    , errMsg403UTxO
     , errMsg403WrongPass
     , errMsg404NoWallet
     )
@@ -137,12 +134,9 @@ spec = do
             , fixtureRandomWalletAddrs @n
             ]
 
-        scenario_TRANS_CREATE_02x @n
-
         -- TRANS_CREATE_03 requires actually being able to compute exact fees, which
         -- is not really possible w/ cardano-node. So, skipping.
 
-        scenario_TRANS_CREATE_04a @n
         scenario_TRANS_CREATE_04b @n
         scenario_TRANS_CREATE_04c @n
         scenario_TRANS_CREATE_04d @n
@@ -158,7 +152,6 @@ spec = do
             , icarusAddresses @n . entropyToMnemonic <$> genEntropy
             ]
 
-        scenario_TRANS_ESTIMATE_04a @n
         scenario_TRANS_ESTIMATE_04b @n
         scenario_TRANS_ESTIMATE_04c @n
 
@@ -294,50 +287,6 @@ scenario_TRANS_ESTIMATE_01_02 fixtureSource fixtures = it title $ \ctx -> do
   where
     title = "TRANS_ESTIMATE_01/02 - " ++ show (length fixtures) ++ " recipient(s)"
 
-scenario_TRANS_CREATE_02x
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        )
-    => SpecWith (Context t)
-scenario_TRANS_CREATE_02x = it title $ \ctx -> do
-    -- SETUP
-    (wSrc, payments) <- fixtureSingleUTxO @n ctx
-
-    -- ACTION
-    r <- postByronTransaction @n ctx wSrc payments fixturePassphrase
-
-    -- ASSERTIONS
-    verify r
-        [ expectResponseCode HTTP.status403
-        , expectErrorMessage errMsg403UTxO
-        ]
-  where
-    title = "TRANS_CREATE_02x - Multi-output failure w/ single UTxO"
-
-scenario_TRANS_CREATE_04a
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        )
-    => SpecWith (Context t)
-scenario_TRANS_CREATE_04a = it title $ \ctx -> do
-    -- SETUP
-    (wSrc, payments) <- fixtureErrInputsDepleted @n ctx
-
-    -- ACTION
-    r <- postByronTransaction @n ctx wSrc payments fixturePassphrase
-
-    -- ASSERTIONS
-    verify r
-        [ expectResponseCode HTTP.status403
-        , expectErrorMessage errMsg403InputsDepleted
-        ]
-  where
-    title = "TRANS_CREATE_04 - Error shown when ErrInputsDepleted encountered"
-
 scenario_TRANS_CREATE_04b
     :: forall (n :: NetworkDiscriminant) t.
         ( DecodeAddress n
@@ -404,28 +353,6 @@ scenario_TRANS_CREATE_04d = it title $ \ctx -> do
   where
     title = "TRANS_CREATE_04 - Wrong password"
 
-scenario_TRANS_ESTIMATE_04a
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        )
-    => SpecWith (Context t)
-scenario_TRANS_ESTIMATE_04a = it title $ \ctx -> do
-    -- SETUP
-    (wSrc, payments) <- fixtureErrInputsDepleted @n ctx
-
-    -- ACTION
-    r <- estimateByronTransaction ctx wSrc payments
-
-    -- ASSERTIONS
-    verify r
-        [ expectResponseCode HTTP.status403
-        , expectErrorMessage errMsg403InputsDepleted
-        ]
-  where
-    title = "TRANS_ESTIMATE_04 - Error shown when ErrInputsDepleted encountered"
-
 scenario_TRANS_ESTIMATE_04b
     :: forall (n :: NetworkDiscriminant) t.
         ( DecodeAddress n
@@ -814,45 +741,6 @@ scenario_TRANS_REG_1670 fixture = it title $ \ctx -> do
 -- More Elaborated Fixtures
 --
 
--- | Returns a source wallet and a list of payments.
---
--- NOTE: Random or Icarus wallets can be used interchangeably here.
-fixtureSingleUTxO
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        )
-    => Context t
-    -> IO (ApiByronWallet, [Aeson.Value])
-fixtureSingleUTxO ctx = do
-    wSrc  <- fixtureRandomWalletWith @n ctx [1_000_000]
-    addrs <- randomAddresses @n . entropyToMnemonic <$> genEntropy
-    let payments =
-            [ mkPayment @n (head addrs) 100_000
-            , mkPayment @n (head addrs) 100_000
-            ]
-    pure (wSrc, payments)
-
--- | Returns a source wallet and a list of payments. If submitted, the payments
--- should result in an error 403.
---
--- NOTE: Random or Icarus wallets can be used interchangeably here.
-fixtureErrInputsDepleted
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        )
-    => Context t
-    -> IO (ApiByronWallet, [Aeson.Value])
-fixtureErrInputsDepleted ctx = do
-    wSrc  <- fixtureRandomWalletWith @n ctx [12_000_000, 20_000_000, 17_000_000]
-    addrs <- randomAddresses @n . entropyToMnemonic <$> genEntropy
-    let amnts = [40_000_000, 22, 22] :: [Natural]
-    let payments = flip map (zip addrs amnts) $ uncurry (mkPayment @n)
-    pure (wSrc, payments)
-
 -- | Returns a source wallet and a list of payments.
 --
 -- NOTE: Random or Icarus wallets can be used interchangeably here.

lib/byron/test/integration/Test/Integration/Byron/Scenario/CLI/Transactions.hs

@@ -101,10 +101,8 @@ import Test.Integration.Framework.TestData
     ( cmdOk
     , errMsg400StartTimeLaterThanEndTime
     , errMsg403Fee
-    , errMsg403InputsDepleted
     , errMsg403NoPendingAnymore
     , errMsg403NotEnoughMoney_
-    , errMsg403UTxO
     , errMsg403WrongPass
     , errMsg404CannotFindTx
     , errMsg404NoWallet
@@ -146,12 +144,9 @@ spec = describe "BYRON_TXS_CLI" $ do
         , fixtureRandomWalletAddrs @n
         ]
 
-    scenario_TRANS_CREATE_02x @n
-
     -- TRANS_CREATE_03 requires actually being able to compute exact fees, which
     -- is not really possible w/ cardano-node. So, skipping.
 
-    scenario_TRANS_CREATE_04a @n
     scenario_TRANS_CREATE_04b @n
     scenario_TRANS_CREATE_04c @n
     scenario_TRANS_CREATE_04d @n
@@ -167,7 +162,6 @@ spec = describe "BYRON_TXS_CLI" $ do
         , icarusAddresses @n . entropyToMnemonic <$> genEntropy
         ]
 
-    scenario_TRANS_ESTIMATE_04a @n
     scenario_TRANS_ESTIMATE_04b @n
     scenario_TRANS_ESTIMATE_04c @n
 
@@ -613,53 +607,6 @@ scenario_TRANS_ESTIMATE_01_02 fixtureSource fixtures = it title $ \ctx -> do
   where
     title = "CLI_TRANS_ESTIMATE_01/02 - " ++ show (length fixtures) ++ " recipient(s)"
 
-scenario_TRANS_CREATE_02x
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        , KnownCommand t
-        )
-    => SpecWith (Context t)
-scenario_TRANS_CREATE_02x = it title $ \ctx -> do
-    -- SETUP
-    (wSrc, payments) <- fixtureSingleUTxO @n ctx
-
-    -- ACTION
-    let args = T.unpack <$> ((wSrc ^. walletId) : payments)
-    (c, out, err) <- postTransactionViaCLI @t ctx (T.unpack fixturePassphrase) args
-
-    -- ASSERTIONS
-    T.unpack err `shouldContain` errMsg403UTxO
-    c `shouldBe` ExitFailure 1
-    out `shouldBe` mempty
-
-  where
-    title = "CLI_TRANS_CREATE_02x - Multi-output failure w/ single UTxO"
-
-scenario_TRANS_CREATE_04a
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        , KnownCommand t
-        )
-    => SpecWith (Context t)
-scenario_TRANS_CREATE_04a = it title $ \ctx -> do
-    -- SETUP
-    (wSrc, payments) <- fixtureErrInputsDepleted @n ctx
-
-    -- ACTION
-    let args = T.unpack <$> ((wSrc ^. walletId) : payments)
-    (c, out, err) <- postTransactionViaCLI @t ctx (T.unpack fixturePassphrase) args
-
-    -- ASSERTIONS
-    T.unpack err `shouldContain` errMsg403InputsDepleted
-    c `shouldBe` ExitFailure 1
-    out `shouldBe` mempty
-  where
-    title = "CLI_TRANS_CREATE_04 - Error shown when ErrInputsDepleted encountered"
-
 scenario_TRANS_CREATE_04b
     :: forall (n :: NetworkDiscriminant) t.
         ( DecodeAddress n
@@ -729,29 +676,6 @@ scenario_TRANS_CREATE_04d = it title $ \ctx -> do
   where
     title = "CLI_TRANS_CREATE_04 - Wrong password"
 
-scenario_TRANS_ESTIMATE_04a
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        , KnownCommand t
-        )
-    => SpecWith (Context t)
-scenario_TRANS_ESTIMATE_04a = it title $ \ctx -> do
-    -- SETUP
-    (wSrc, payments) <- fixtureErrInputsDepleted @n ctx
-
-    -- ACTION
-    let args = T.unpack <$> ((wSrc ^. walletId) : payments)
-    (Exit c, Stdout out, Stderr err) <- postTransactionFeeViaCLI @t ctx args
-
-    -- ASSERTIONS
-    err `shouldContain` errMsg403InputsDepleted
-    c `shouldBe` ExitFailure 1
-    out `shouldBe` mempty
-  where
-    title = "CLI_TRANS_ESTIMATE_04 - Error shown when ErrInputsDepleted encountered"
-
 scenario_TRANS_ESTIMATE_04b
     :: forall (n :: NetworkDiscriminant) t.
         ( DecodeAddress n
@@ -835,47 +759,6 @@ scenario_TRANS_CREATE_07 = it title $ \ctx -> do
 -- More Elaborated Fixtures
 --
 
--- | Returns a source wallet and a list of payments.
---
--- NOTE: Random or Icarus wallets can be used interchangeably here.
-fixtureSingleUTxO
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        )
-    => Context t
-    -> IO (ApiByronWallet, [Text])
-fixtureSingleUTxO ctx = do
-    wSrc  <- fixtureRandomWalletWith @n ctx [1_000_000]
-    addrs <- randomAddresses @n . entropyToMnemonic <$> genEntropy
-    let addrStr = encodeAddress @n (head addrs)
-    let payments =
-            [ "--payment", "100000@" <> addrStr
-            , "--payment", "100000@" <> addrStr
-            ]
-    pure (wSrc, payments)
-
--- | Returns a source wallet and a list of payments. If submitted, the payments
--- should result in an error 403.
---
--- NOTE: Random or Icarus wallets can be used interchangeably here.
-fixtureErrInputsDepleted
-    :: forall (n :: NetworkDiscriminant) t.
-        ( DecodeAddress n
-        , EncodeAddress n
-        , PaymentAddress n ByronKey
-        )
-    => Context t
-    -> IO (ApiByronWallet, [Text])
-fixtureErrInputsDepleted ctx = do
-    wSrc  <- fixtureRandomWalletWith @n ctx [12_000_000, 20_000_000, 17_000_000]
-    addrs <- randomAddresses @n . entropyToMnemonic <$> genEntropy
-    -- let addrStrs = encodeAddress @n <$> (addrs)
-    let amnts = [40_000_000, 22, 22] :: [Natural]
-    let payments = flip map (zip addrs amnts) $ uncurry (mkPaymentCmd @n)
-    pure (wSrc, join payments)
-
 -- | Returns a source wallet and a list of payments.
 --
 -- NOTE: Random or Icarus wallets can be used interchangeably here.

lib/byron/test/unit/Cardano/Wallet/Byron/TransactionSpec.hs

@@ -390,7 +390,7 @@ genSelection = do
     genSelectionFor :: NonEmpty TxOut -> Gen CoinSelection
     genSelectionFor outs = do
         utxo <- vectorOf (NE.length outs * 3) genCoin >>= genUTxO @n @k
-        case runIdentity $ runExceptT $ largestFirst opts outs utxo of
+        case runIdentity $ runExceptT $ largestFirst opts outs (Quantity 0) utxo of
             Left _ -> genSelectionFor outs
             Right (s,_) -> return s
 

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

@@ -49,7 +49,6 @@ module Test.Integration.Framework.TestData
     , errMsg403NotAByronWallet
     , errMsg403NotEnoughMoney
     , errMsg403NotEnoughMoney_
-    , errMsg403UTxO
     , errMsg403WrongPass
     , errMsg403NoPendingAnymore
     , errMsg404NoSuchPool
@@ -285,8 +284,9 @@ errMsg403NotAByronWallet =
 
 errMsg403NotEnoughMoney_ :: String
 errMsg403NotEnoughMoney_ =
-    "I can't process this payment because there's \
-    \not enough UTxO available in the wallet."
+    "I cannot select enough UTxO from your wallet to construct an adequate \
+    \transaction. Try sending a smaller amount or increasing the number of \
+    \available UTxO."
 
 errMsg403NotEnoughMoney :: Int -> Int -> String
 errMsg403NotEnoughMoney has needs = "I can't process this payment because there's\
@@ -332,11 +332,6 @@ _errMsg403InpsOrOutsExceeded (maxNumInps, maxNumOuts) =
     \   more than " ++ show maxNumInps ++ " or the number of outputs\
     \ exceeds " ++ show maxNumOuts ++ "."
 
-errMsg403UTxO :: String
-errMsg403UTxO = "When creating new transactions, I'm not able to re-use the\
-    \ same UTxO for different outputs. Here, I only have 1\
-    \ available, but there are 2 outputs."
-
 errMsg403WrongPass :: String
 errMsg403WrongPass = "The given encryption passphrase doesn't match the one\
     \ I use to encrypt the root private key of the given wallet"