Skip to content

Commit

Permalink
Merge #3197
Browse files Browse the repository at this point in the history
3197: Remove hard-coded dependency on `txOutMax{Coin,TokenQuantity}` within coin selection modules r=jonathanknowles a=jonathanknowles

## Issue Number

ADP-1481

## Summary

This PR adjusts the **_internal_** coin selection modules so that they no longer depend on **_hard-coded_** maximum token quantity constants imported from the wallet.

Instead, these constants are now **_passed in_** to coin selection as part of `SelectionConstraints`.

```patch
  data SelectionConstraints ctx = SelectionConstraints
      { ...
+     , outputMaximumAdaQuantity
+        :: Coin
+     , outputMaximumTokenQuantity
+        :: TokenQuantity
      , ...
      }
 ```
 
There is no impact on the wallet itself, since these dependencies are injected in automatically by the `Cardano.Wallet.CoinSelection` module, which provides a wallet-friendly interface to coin selection.

## Future Work

We intend to eventually adjust coin selection so that it **_does not discriminate_** between **_ada_** and **_non-ada_** quantities. (See https://input-output.atlassian.net/browse/ADP-1449). When that moment comes, we may want to replace these two constants with a function similar to:

```patch
 data SelectionConstraints ctx = SelectionConstraints
     { ...
-    , outputMaximumAdaQuantity
-       :: Coin
-    , outputMaximumTokenQuantity
-       :: TokenQuantity
+    , outputMaximumTokenQuantity
+        :: Asset ctx -> TokenQuantity 
     , ...
     }
 ```

Then, callers of coin selection can provide a function that meets their requirements. If the requirement is still to have different limits for ada and non-ada quantities, then this can be encoded in the function.

Or, if we can get away with a single limit for all asset types (including ada), then we can reduce this to a single constant.

Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
  • Loading branch information
iohk-bors[bot] and jonathanknowles authored Mar 23, 2022
2 parents a72ca89 + b7c1fc7 commit ace5058
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 8 deletions.
11 changes: 10 additions & 1 deletion lib/core/src/Cardano/Wallet/CoinSelection.hs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,12 @@ import Cardano.Wallet.Primitive.Types.TokenBundle
import Cardano.Wallet.Primitive.Types.TokenMap
( AssetId, TokenMap )
import Cardano.Wallet.Primitive.Types.Tx
( TokenBundleSizeAssessment, TxIn, TxOut (..) )
( TokenBundleSizeAssessment
, TxIn
, TxOut (..)
, txOutMaxCoin
, txOutMaxTokenQuantity
)
import Cardano.Wallet.Primitive.Types.UTxO
( UTxO (..) )
import Cardano.Wallet.Primitive.Types.UTxOSelection
Expand Down Expand Up @@ -248,6 +253,10 @@ toInternalSelectionConstraints SelectionConstraints {..} =
computeMinimumCost . toExternalSelectionSkeleton
, computeSelectionLimit =
computeSelectionLimit . fmap (uncurry TxOut)
, maximumOutputAdaQuantity =
txOutMaxCoin
, maximumOutputTokenQuantity =
txOutMaxTokenQuantity
, ..
}

Expand Down
12 changes: 12 additions & 0 deletions lib/core/src/Cardano/Wallet/CoinSelection/Internal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,14 @@ data SelectionConstraints ctx = SelectionConstraints
:: Natural
-- ^ Specifies the minimum required amount of collateral as a
-- percentage of the total transaction fee.
, maximumOutputAdaQuantity
:: Coin
-- ^ Specifies the largest ada quantity that can appear in the token
-- bundle of an output.
, maximumOutputTokenQuantity
:: TokenQuantity
-- ^ Specifies the largest non-ada quantity that can appear in the
-- token bundle of an output.
}
deriving Generic

Expand Down Expand Up @@ -402,6 +410,10 @@ toBalanceConstraintsParams (constraints, params) =
& adjustComputeSelectionLimit
, assessTokenBundleSize =
view #assessTokenBundleSize constraints
, maximumOutputAdaQuantity =
view #maximumOutputAdaQuantity constraints
, maximumOutputTokenQuantity =
view #maximumOutputTokenQuantity constraints
}
where
adjustComputeMinimumCost
Expand Down
34 changes: 27 additions & 7 deletions lib/core/src/Cardano/Wallet/CoinSelection/Internal/Balance.hs
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,7 @@ import Cardano.Wallet.Primitive.Types.TokenMap
import Cardano.Wallet.Primitive.Types.TokenQuantity
( TokenQuantity (..) )
import Cardano.Wallet.Primitive.Types.Tx
( TokenBundleSizeAssessment (..)
, TokenBundleSizeAssessor (..)
, txOutMaxCoin
, txOutMaxTokenQuantity
)
( TokenBundleSizeAssessment (..), TokenBundleSizeAssessor (..) )
import Cardano.Wallet.Primitive.Types.UTxOIndex
( SelectionFilter (..), UTxOIndex (..) )
import Cardano.Wallet.Primitive.Types.UTxOSelection
Expand Down Expand Up @@ -231,6 +227,14 @@ data SelectionConstraints ctx = SelectionConstraints
:: [(Address ctx, TokenBundle)] -> SelectionLimit
-- ^ Computes an upper bound for the number of ordinary inputs to
-- select, given a current set of outputs.
, maximumOutputAdaQuantity
:: Coin
-- ^ Specifies the largest ada quantity that can appear in the token
-- bundle of an output.
, maximumOutputTokenQuantity
:: TokenQuantity
-- ^ Specifies the largest non-ada quantity that can appear in the
-- token bundle of an output.
}
deriving Generic

Expand Down Expand Up @@ -905,6 +909,8 @@ performSelectionNonEmpty constraints params
, computeMinimumAdaQuantity
, computeMinimumCost
, computeSelectionLimit
, maximumOutputAdaQuantity
, maximumOutputTokenQuantity
} = constraints
SelectionParams
{ outputsToCover
Expand Down Expand Up @@ -998,6 +1004,8 @@ performSelectionNonEmpty constraints params
, outputBundles
, assetsToMint
, assetsToBurn
, maximumOutputAdaQuantity
, maximumOutputTokenQuantity
}
)
where
Expand Down Expand Up @@ -1079,6 +1087,8 @@ performSelectionNonEmpty constraints params
, outputBundles = snd <$> outputsToCover
, assetsToMint
, assetsToBurn
, maximumOutputAdaQuantity
, maximumOutputTokenQuantity
}

mkSelectionResult :: [TokenBundle] -> SelectionResultOf NonEmpty ctx
Expand Down Expand Up @@ -1390,6 +1400,14 @@ data MakeChangeCriteria minCoinFor bundleSizeAssessor = MakeChangeCriteria
-- ^ Assets to mint: these provide input value to a transaction.
, assetsToBurn :: TokenMap
-- ^ Assets to burn: these consume output value from a transaction.
, maximumOutputAdaQuantity
:: Coin
-- ^ Specifies the largest ada quantity that can appear in the token
-- bundle of an output.
, maximumOutputTokenQuantity
:: TokenQuantity
-- ^ Specifies the largest non-ada quantity that can appear in the
-- token bundle of an output.
} deriving (Eq, Generic, Show)

-- | Indicates 'True' if and only if a token bundle exceeds the maximum size
Expand Down Expand Up @@ -1450,6 +1468,8 @@ makeChange criteria
, outputBundles
, assetsToMint
, assetsToBurn
, maximumOutputAdaQuantity
, maximumOutputTokenQuantity
} = criteria

-- The following subtraction is safe, as we have already checked
Expand Down Expand Up @@ -1503,7 +1523,7 @@ makeChange criteria
& flip splitBundlesWithExcessiveAssetCounts
(tokenBundleSizeExceedsLimit assessBundleSizeWithMaxCoin)
& flip splitBundlesWithExcessiveTokenQuantities
txOutMaxTokenQuantity
maximumOutputTokenQuantity

-- When assessing the size of a change map to determine if it is
-- excessively large, we don't yet know how large the associated
Expand All @@ -1524,7 +1544,7 @@ makeChange criteria
assessBundleSizeWithMaxCoin :: TokenBundleSizeAssessor
assessBundleSizeWithMaxCoin = TokenBundleSizeAssessor
$ view #assessTokenBundleSize bundleSizeAssessor
. flip TokenBundle.setCoin txOutMaxCoin
. flip TokenBundle.setCoin maximumOutputAdaQuantity

-- Change for user-specified assets: assets that were present in the
-- original set of user-specified outputs ('outputsToCover').
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ import Data.Generics.Internal.VL.Lens
( view )
import Data.Generics.Labels
()
import Data.IntCast
( intCast )
import Data.List.NonEmpty
( NonEmpty (..) )
import Data.Map.Strict
Expand Down Expand Up @@ -1847,6 +1849,8 @@ mkBoundaryTestExpectation (BoundaryTestData params expectedResult) = do
, assessTokenBundleSize = unMockAssessTokenBundleSize $
boundaryTestBundleSizeAssessor params
, computeSelectionLimit = const NoLimit
, maximumOutputAdaQuantity = testMaximumOutputAdaQuantity
, maximumOutputTokenQuantity = testMaximumOutputTokenQuantity
}

encodeBoundaryTestCriteria
Expand Down Expand Up @@ -2474,8 +2478,30 @@ unMockSelectionConstraints m = SelectionConstraints
unMockComputeMinimumCost $ view #computeMinimumCost m
, computeSelectionLimit =
unMockComputeSelectionLimit $ view #computeSelectionLimit m
, maximumOutputAdaQuantity =
testMaximumOutputAdaQuantity
, maximumOutputTokenQuantity =
testMaximumOutputTokenQuantity
}

-- | Specifies the largest ada quantity that can appear in the token bundle
-- of an output.
--
-- For the moment, we use the same constant that is used in the wallet. In
-- future, we can improve our test coverage by allowing this value to vary.
--
testMaximumOutputAdaQuantity :: Coin
testMaximumOutputAdaQuantity = Coin 45_000_000_000_000_000

-- | Specifies the largest non-ada quantity that can appear in the token bundle
-- of an output.
--
-- For the moment, we use the same constant that is used in the wallet. In
-- future, we can improve our test coverage by allowing this value to vary.
--
testMaximumOutputTokenQuantity :: TokenQuantity
testMaximumOutputTokenQuantity = TokenQuantity $ intCast $ maxBound @Word64

--------------------------------------------------------------------------------
-- Computing minimum ada quantities
--------------------------------------------------------------------------------
Expand Down Expand Up @@ -2674,6 +2700,8 @@ genMakeChangeData = flip suchThat isValidMakeChangeData $ do
<*> genTokenBundles outputBundleCount
<*> genAssetsToMint
<*> genAssetsToBurn
<*> genMaximumOutputAdaQuantity
<*> genMaximumOutputTokenQuantity
where
genAssetsToMint :: Gen TokenMap
genAssetsToMint = genTokenMapSmallRange
Expand All @@ -2695,6 +2723,12 @@ genMakeChangeData = flip suchThat isValidMakeChangeData $ do
<$> genTokenBundleSmallRangePositive
<*> replicateM count genTokenBundleSmallRangePositive

genMaximumOutputAdaQuantity :: Gen Coin
genMaximumOutputAdaQuantity = pure testMaximumOutputAdaQuantity

genMaximumOutputTokenQuantity :: Gen TokenQuantity
genMaximumOutputTokenQuantity = pure testMaximumOutputTokenQuantity

makeChangeWith
:: MakeChangeData
-> Either UnableToConstructChangeError [TokenBundle]
Expand All @@ -2720,6 +2754,8 @@ prop_makeChange_identity bundles = (===)
, outputBundles = bundles
, assetsToMint = TokenMap.empty
, assetsToBurn = TokenMap.empty
, maximumOutputAdaQuantity = testMaximumOutputAdaQuantity
, maximumOutputTokenQuantity = testMaximumOutputTokenQuantity
}

-- | Tests that 'makeChange' generates the correct number of change bundles.
Expand Down Expand Up @@ -3080,6 +3116,8 @@ unit_makeChange =
, outputBundles = o
, assetsToMint = TokenMap.empty
, assetsToBurn = TokenMap.empty
, maximumOutputAdaQuantity = testMaximumOutputAdaQuantity
, maximumOutputTokenQuantity = testMaximumOutputTokenQuantity
}
]
where
Expand Down
52 changes: 52 additions & 0 deletions lib/core/test/unit/Cardano/Wallet/CoinSelection/InternalSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
{-# LANGUAGE GADTs #-}
{-# LANGUAGE LambdaCase #-}
{-# LANGUAGE NamedFieldPuns #-}
{-# LANGUAGE NumericUnderscores #-}
{-# LANGUAGE OverloadedLabels #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeApplications #-}
{-# OPTIONS_GHC -fno-warn-orphans #-}
{- HLINT ignore "Use camelCase" -}

Expand Down Expand Up @@ -100,8 +102,12 @@ import Data.Functor
( (<&>) )
import Data.Generics.Internal.VL.Lens
( over, view, (^.) )
import Data.IntCast
( intCast )
import Data.Map.Strict
( Map )
import Data.Word
( Word64 )
import Generics.SOP
( NP (..) )
import GHC.Generics
Expand Down Expand Up @@ -542,6 +548,10 @@ data MockSelectionConstraints = MockSelectionConstraints
:: Int
, minimumCollateralPercentage
:: Natural
, maximumOutputAdaQuantity
:: Coin
, maximumOutputTokenQuantity
:: TokenQuantity
}
deriving (Eq, Generic, Show)

Expand All @@ -554,6 +564,8 @@ genMockSelectionConstraints = MockSelectionConstraints
<*> genMockComputeSelectionLimit
<*> genMaximumCollateralInputCount
<*> genMinimumCollateralPercentage
<*> genMaximumOutputAdaQuantity
<*> genMaximumOutputTokenQuantity

shrinkMockSelectionConstraints
:: MockSelectionConstraints -> [MockSelectionConstraints]
Expand All @@ -565,6 +577,8 @@ shrinkMockSelectionConstraints = genericRoundRobinShrink
<:> shrinkMockComputeSelectionLimit
<:> shrinkMaximumCollateralInputCount
<:> shrinkMinimumCollateralPercentage
<:> shrinkMaximumOutputAdaQuantity
<:> shrinkMaximumOutputTokenQuantity
<:> Nil

unMockSelectionConstraints
Expand All @@ -584,6 +598,10 @@ unMockSelectionConstraints m = SelectionConstraints
view #maximumCollateralInputCount m
, minimumCollateralPercentage =
view #minimumCollateralPercentage m
, maximumOutputAdaQuantity =
view #maximumOutputAdaQuantity m
, maximumOutputTokenQuantity =
view #maximumOutputTokenQuantity m
}

--------------------------------------------------------------------------------
Expand Down Expand Up @@ -616,6 +634,40 @@ genMinimumCollateralPercentage = chooseNatural (0, 1000)
shrinkMinimumCollateralPercentage :: Natural -> [Natural]
shrinkMinimumCollateralPercentage = shrinkNatural

--------------------------------------------------------------------------------
-- Maximum token quantities
--------------------------------------------------------------------------------

genMaximumOutputAdaQuantity :: Gen Coin
genMaximumOutputAdaQuantity = pure testMaximumOutputAdaQuantity

genMaximumOutputTokenQuantity :: Gen TokenQuantity
genMaximumOutputTokenQuantity = pure testMaximumOutputTokenQuantity

shrinkMaximumOutputAdaQuantity :: Coin -> [Coin]
shrinkMaximumOutputAdaQuantity = const []

shrinkMaximumOutputTokenQuantity :: TokenQuantity -> [TokenQuantity]
shrinkMaximumOutputTokenQuantity = const []

-- | Specifies the largest ada quantity that can appear in the token bundle
-- of an output.
--
-- For the moment, we use the same constant that is used in the wallet. In
-- future, we can improve our test coverage by allowing this value to vary.
--
testMaximumOutputAdaQuantity :: Coin
testMaximumOutputAdaQuantity = Coin 45_000_000_000_000_000

-- | Specifies the largest non-ada quantity that can appear in the token bundle
-- of an output.
--
-- For the moment, we use the same constant that is used in the wallet. In
-- future, we can improve our test coverage by allowing this value to vary.
--
testMaximumOutputTokenQuantity :: TokenQuantity
testMaximumOutputTokenQuantity = TokenQuantity $ intCast $ maxBound @Word64

--------------------------------------------------------------------------------
-- Selection parameters
--------------------------------------------------------------------------------
Expand Down

0 comments on commit ace5058

Please sign in to comment.