Merge #3416

3416: Reduce overestimation of minimum UTxO values for Shelley-era addresses. r=jonathanknowles a=jonathanknowles ### Issue Number ADP-2039 ### Summary This PR reduces our overestimation of minimum UTxO values for outputs with Shelley-era addresses. ### Details - When computing minimum ada quantities for _**user-specified outputs**_, coin selection now uses the _actual addresses specified by the user_. - When computing minimum ada quantities for _**auto-generated change outputs**_, coin selection now uses a _maximum length change address_ that is specific to the current wallet key type. - This allows us to use a smaller overestimation in the case of Shelley-era addresses, as the longest Shelley-era address is shorter than the longest Byron-era address. Co-authored-by: Jonathan Knowles <[email protected]> Co-authored-by: Johannes Lund <[email protected]>
cardano-foundation · Aug 3, 2022 · dd20f94 · dd20f94
2 parents 8dba4af + cbc2c20
commit dd20f94
Show file tree

Hide file tree

Showing 38 changed files with 814 additions and 441 deletions.
diff --git a/lib/core-integration/src/Test/Integration/Framework/DSL.hs b/lib/core-integration/src/Test/Integration/Framework/DSL.hs
@@ -674,9 +674,20 @@ walletId =
 -- | Min UTxO parameter for the test cluster.
 minUTxOValue :: ApiEra -> Natural
 minUTxOValue e
-    | e >= ApiBabbage = 1_107_670 -- needs to be overestimated for the sake of
-    -- long byron addresses
-    | e >= ApiAlonzo = 999_978 -- From 34482 lovelace per word
+    | e >= ApiBabbage = 995_610
+        -- This value is a slight overestimation for outputs with Shelley
+        -- addresses and no tokens.
+        --
+        -- However, it would be incorrect for outputs with Byron addresses,
+        -- where the lower bound would be greater by the following amount:
+        --
+        -- 4310 lovelace/byte * (86 - 57) byte ≈ 0.125 ada
+        --
+        -- However, this value appears to be fine for the purposes of
+        -- integration tests.
+        --
+    | e >= ApiAlonzo = 999_978
+        -- From 34482 lovelace/word.
     | otherwise   = 1_000_000
 
 -- | Parameter in test cluster shelley genesis.

diff --git a/lib/core-integration/src/Test/Integration/Framework/TestData.hs b/lib/core-integration/src/Test/Integration/Framework/TestData.hs
@@ -316,11 +316,13 @@ errMsg403InvalidConstructTx =
      \Include at least one of them."
 
 errMsg403MinUTxOValue :: String
-errMsg403MinUTxOValue =
-    "Some outputs have ada values that are too small. There's a \
-    \minimum ada value specified by the protocol that each output must satisfy. \
-    \I'll handle that minimum value myself when you do not explicitly specify \
-    \an ada value for an output. Otherwise, you must specify enough ada."
+errMsg403MinUTxOValue = unwords
+    [ "One of the outputs you've specified has an ada quantity that is"
+    , "below the minimum required. Either increase the ada quantity to"
+    , "at least the minimum, or specify an ada quantity of zero, in"
+    , "which case the wallet will automatically assign the correct"
+    , "minimum ada quantity to the output."
+    ]
 
 errMsg409WalletExists :: String -> String
 errMsg409WalletExists walId = "This operation would yield a wallet with the following\

diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Byron/Transactions.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Byron/Transactions.hs
@@ -106,6 +106,7 @@ import Test.Integration.Framework.Request
     ( RequestException )
 import Test.Integration.Framework.TestData
     ( errMsg400StartTimeLaterThanEndTime
+    , errMsg403MinUTxOValue
     , errMsg404NoAsset
     , errMsg404NoWallet
     , steveToken
@@ -186,7 +187,7 @@ spec = describe "BYRON_TRANSACTIONS" $ do
         rtx <- request @(ApiTransaction n) ctx
             (Link.createTransactionOld @'Byron wSrc) Default payload
         expectResponseCode HTTP.status403 rtx
-        expectErrorMessage "Some outputs have ada values that are too small." rtx
+        expectErrorMessage errMsg403MinUTxOValue rtx
 
     describe "BYRON_TRANS_ASSETS_CREATE_02a - Multi-asset transaction with no ADA" $
         forM_ [ (fixtureMultiAssetRandomWallet @n, "Byron wallet")

diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Byron/TransactionsNew.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Byron/TransactionsNew.hs
@@ -78,7 +78,11 @@ import Test.Integration.Framework.DSL
     , verify
     )
 import Test.Integration.Framework.TestData
-    ( errMsg403Fee, errMsg403InvalidConstructTx, errMsg403NotEnoughMoney )
+    ( errMsg403Fee
+    , errMsg403InvalidConstructTx
+    , errMsg403MinUTxOValue
+    , errMsg403NotEnoughMoney
+    )
 
 import qualified Cardano.Wallet.Api.Link as Link
 import qualified Network.HTTP.Types.Status as HTTP
@@ -324,7 +328,7 @@ spec = describe "NEW_BYRON_TRANSACTIONS" $ do
                 (Link.createUnsignedTransaction @'Byron wa) Default payload
             verify rTx
                 [ expectResponseCode HTTP.status403
-                , expectErrorMessage "Some outputs have ada values that are too small."
+                , expectErrorMessage errMsg403MinUTxOValue
                 ]
 
     describe "BYRON_TRANS_NEW_ASSETS_CREATE_01c - Multi-asset tx without Ada" $

diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Network.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Network.hs
@@ -21,7 +21,7 @@ import Cardano.Wallet.Api.Types
     )
 import Cardano.Wallet.Primitive.SyncProgress
     ( SyncProgress (..) )
-import Cardano.Wallet.Primitive.Types
+import Cardano.Wallet.Primitive.Types.ProtocolMagic
     ( getProtocolMagic, mainnetMagic )
 import Control.Monad
     ( when )

diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/StakePools.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/StakePools.hs
@@ -1433,8 +1433,8 @@ spec = describe "SHELLEY_STAKE_POOLS" $ do
     costOfJoining :: Context -> Natural
     costOfJoining ctx =
         if _mainEra ctx >= ApiBabbage
-        then costOf (\coeff cst -> 458 * coeff + cst) ctx
-        else costOf (\coeff cst -> 454 * coeff + cst) ctx
+        then costOf (\coeff cst -> 454 * coeff + cst) ctx
+        else costOf (\coeff cst -> 450 * coeff + cst) ctx
 
     costOfQuitting :: Context -> Natural
     costOfQuitting ctx =

diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Transactions.hs
@@ -683,7 +683,7 @@ spec = describe "SHELLEY_TRANSACTIONS" $ do
             (Link.createTransactionOld @'Shelley wSrc) Default payload
         -- It should fail with InsufficientMinCoinValueError
         expectResponseCode HTTP.status403 rtx
-        expectErrorMessage "Some outputs have ada values that are too small." rtx
+        expectErrorMessage errMsg403MinUTxOValue rtx
 
     it "TRANS_ASSETS_CREATE_02a - Multi-asset transaction without Ada" $ \ctx -> runResourceT $ do
         wSrc <- fixtureMultiAssetWallet ctx

diff --git a/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/TransactionsNew.hs b/lib/core-integration/src/Test/Integration/Scenario/API/Shelley/TransactionsNew.hs
@@ -1051,7 +1051,7 @@ spec = describe "NEW_SHELLEY_TRANSACTIONS" $ do
             (Link.createUnsignedTransaction @'Shelley wa) Default payload
         verify rTx
             [ expectResponseCode HTTP.status403
-            , expectErrorMessage "Some outputs have ada values that are too small."
+            , expectErrorMessage errMsg403MinUTxOValue
             ]
 
     it "TRANS_NEW_ASSETS_CREATE_01c - Multi-asset tx without Ada" $ \ctx -> runResourceT $ do

diff --git a/lib/core/cardano-wallet-core.cabal b/lib/core/cardano-wallet-core.cabal
@@ -252,10 +252,12 @@ library
       Cardano.Wallet.Primitive.Passphrase.Types
       Cardano.Wallet.Primitive.Types
       Cardano.Wallet.Primitive.Types.Address
+      Cardano.Wallet.Primitive.Types.Address.Constants
       Cardano.Wallet.Primitive.Types.Coin
       Cardano.Wallet.Primitive.Types.Hash
       Cardano.Wallet.Primitive.Types.MinimumUTxO
       Cardano.Wallet.Primitive.Types.MinimumUTxO.Gen
+      Cardano.Wallet.Primitive.Types.ProtocolMagic
       Cardano.Wallet.Primitive.Types.Redeemer
       Cardano.Wallet.Primitive.Types.RewardAccount
       Cardano.Wallet.Primitive.Types.TokenBundle

diff --git a/lib/core/src/Cardano/Byron/Codec/Cbor.hs b/lib/core/src/Cardano/Byron/Codec/Cbor.hs
@@ -49,12 +49,12 @@ import Cardano.Wallet.Primitive.AddressDerivation
     ( Depth (..), DerivationType (..), Index (..) )
 import Cardano.Wallet.Primitive.Passphrase
     ( Passphrase (..) )
-import Cardano.Wallet.Primitive.Types
-    ( ProtocolMagic (..) )
 import Cardano.Wallet.Primitive.Types.Address
     ( Address (..) )
 import Cardano.Wallet.Primitive.Types.Hash
     ( Hash (..) )
+import Cardano.Wallet.Primitive.Types.ProtocolMagic
+    ( ProtocolMagic (..) )
 import Cardano.Wallet.Primitive.Types.Tx
     ( TxIn (..), TxOut (..), unsafeCoinToTxOutCoinValue )
 import Control.Monad

diff --git a/lib/core/src/Cardano/Wallet.hs b/lib/core/src/Cardano/Wallet.hs
@@ -290,7 +290,8 @@ import Cardano.Wallet.Network
     , NetworkLayer (..)
     )
 import Cardano.Wallet.Primitive.AddressDerivation
-    ( DelegationAddress (..)
+    ( BoundedAddressLength (..)
+    , DelegationAddress (..)
     , Depth (..)
     , DerivationIndex (..)
     , DerivationPrefix (..)
@@ -421,8 +422,6 @@ import Cardano.Wallet.Primitive.Types.RewardAccount
     ( RewardAccount (..) )
 import Cardano.Wallet.Primitive.Types.TokenBundle
     ( TokenBundle (..) )
-import Cardano.Wallet.Primitive.Types.TokenMap
-    ( TokenMap )
 import Cardano.Wallet.Primitive.Types.TokenPolicy
     ( TokenName (UnsafeTokenName), TokenPolicyId (UnsafeTokenPolicyId) )
 import Cardano.Wallet.Primitive.Types.TokenQuantity
@@ -473,7 +472,7 @@ import Cardano.Wallet.Transaction
 import Control.Applicative
     ( (<|>) )
 import Control.Arrow
-    ( left )
+    ( first, left )
 import Control.DeepSeq
     ( NFData )
 import Control.Monad
@@ -546,7 +545,7 @@ import Data.Map.Strict
 import Data.Maybe
     ( fromMaybe, isJust, mapMaybe )
 import Data.Proxy
-    ( Proxy )
+    ( Proxy (..) )
 import Data.Quantity
     ( Quantity (..) )
 import Data.Set
@@ -922,26 +921,28 @@ getWalletUtxoSnapshot ctx wid = do
     (wallet, _, pending) <- withExceptT id (readWallet @ctx @s @k ctx wid)
     pp <- liftIO $ currentProtocolParameters nl
     era <- liftIO $ currentNodeEra nl
-    let bundles = availableUTxO @s pending wallet
+    let txOuts = availableUTxO @s pending wallet
             & unUTxO
             & F.toList
-            & fmap (view #tokens)
-    pure $ pairBundleWithMinAdaQuantity era pp <$> bundles
+    pure $ first (view #tokens) . pairTxOutWithMinAdaQuantity era pp <$> txOuts
   where
     nl = ctx ^. networkLayer
     tl = ctx ^. transactionLayer @k
 
-    pairBundleWithMinAdaQuantity
+    pairTxOutWithMinAdaQuantity
         :: Cardano.AnyCardanoEra
         -> ProtocolParameters
-        -> TokenBundle
-        -> (TokenBundle, Coin)
-    pairBundleWithMinAdaQuantity era pp bundle =
-        (bundle, computeMinAdaQuantity $ view #tokens bundle)
+        -> TxOut
+        -> (TxOut, Coin)
+    pairTxOutWithMinAdaQuantity era pp out =
+        (out, computeMinAdaQuantity out)
       where
-        computeMinAdaQuantity :: TokenMap -> Coin
-        computeMinAdaQuantity =
-            view #txOutputMinimumAdaQuantity (constraints tl era pp)
+        computeMinAdaQuantity :: TxOut -> Coin
+        computeMinAdaQuantity (TxOut addr bundle) =
+            view #txOutputMinimumAdaQuantity
+                (constraints tl era pp)
+                (addr)
+                (view #tokens bundle)
 
 -- | List the wallet's UTxO statistics.
 listUtxoStatistics
@@ -1568,6 +1569,7 @@ balanceTransaction
         , MonadRandom m
         , HasLogger m WalletWorkerLog ctx
         , Cardano.IsShelleyBasedEra era
+        , BoundedAddressLength k
         )
     => ctx
     -> ArgGenChange s
@@ -1591,6 +1593,7 @@ balanceTransactionWithSelectionStrategy
     :: forall era m s k ctx.
         ( HasTransactionLayer k ctx
         , GenChange s
+        , BoundedAddressLength k
         , MonadRandom m
         , HasLogger m WalletWorkerLog ctx
         , Cardano.IsShelleyBasedEra era
@@ -1985,6 +1988,8 @@ balanceTransactionWithSelectionStrategy
                     intCast @Word16 @Int $ view #maximumCollateralInputCount pp
                 , minimumCollateralPercentage =
                     view #minimumCollateralPercentage pp
+                , maximumLengthChangeAddress =
+                    maxLengthAddressFor $ Proxy @k
                 }
 
             selectionParams = SelectionParams
@@ -2171,8 +2176,8 @@ calcMinimumCoinValues
 calcMinimumCoinValues ctx era outs = do
     pp <- currentProtocolParameters nl
     pure
-        $ view #txOutputMinimumAdaQuantity (constraints tl era pp)
-        . view (#tokens . #tokens) <$> outs
+        $ uncurry (view #txOutputMinimumAdaQuantity (constraints tl era pp))
+        . (\o -> (view #address o, view (#tokens . #tokens) o)) <$> outs
   where
     nl = ctx ^. networkLayer
     tl = ctx ^. transactionLayer @k
@@ -2213,7 +2218,8 @@ data SelectAssetsParams s result = SelectAssetsParams
 --
 selectAssets
     :: forall ctx m s k result.
-        ( HasTransactionLayer k ctx
+        ( BoundedAddressLength k
+        , HasTransactionLayer k ctx
         , HasLogger m WalletWorkerLog ctx
         , MonadRandom m
         )
@@ -2247,6 +2253,8 @@ selectAssets ctx era pp params transform = do
                 intCast @Word16 @Int $ view #maximumCollateralInputCount pp
             , minimumCollateralPercentage =
                 view #minimumCollateralPercentage pp
+            , maximumLengthChangeAddress =
+                maxLengthAddressFor $ Proxy @k
             }
     let selectionParams = SelectionParams
             { assetsToMint =