Minimize the responsibility of makeChangeForNonUserSpecifiedOutputs.

This change moves the following responsibilities out of the
`makeChangeForNonUserSpecifiedOutputs` function:

    - the removing of burned values
    - the addition of minted values

This reduces the complexity of property and unit tests that test this
function.

The following functions are still checked by relevant properties:

    - `addMintValuesToChangeMaps`
    - `removeBurnValuesFromChangeMaps`

Further changes will add unit tests to act as documentation and sanity
checks for the above functions.

lib/core/src/Cardano/Wallet/Primitive/CoinSelection/MA/RoundRobin.hs

@@ -1186,11 +1186,13 @@ makeChange criteria
  makeChangeForNonUserSpecifiedAssets
  outputMaps
  nonUserSpecifiedAssetQuantities
+ & addMintValuesToChangeMaps
  (removeAssetIds userSpecifiedAssetIds assetsToMint)
+ & removeBurnValuesFromChangeMaps
  (removeAssetIds userSpecifiedAssetIds assetsToBurn)
-
- removeAssetIds :: Set AssetId -> TokenMap -> TokenMap
- removeAssetIds as = TokenMap.filter (not . (`Set.member` as))
+ where
+  removeAssetIds :: Set AssetId -> TokenMap -> TokenMap
+  removeAssetIds as = TokenMap.filter (not . (`Set.member` as))
 
  totalInputValueInsufficient = error
  "makeChange: not (totalOutputValue <= totalInputValue)"
@@ -1450,30 +1452,18 @@ makeChangeForNonUserSpecifiedAsset n (asset, quantities) =
 -- The resultant list is sorted into ascending order when maps are compared
 -- with the `leq` function.
 --
--- This function operates on the precondition that the assets to burn are
--- `leq` than the (asset quantities to distribute + assets to mint).
 makeChangeForNonUserSpecifiedAssets
  :: NonEmpty a
  -- ^ Determines the number of change maps to create.
  -> Map AssetId (NonEmpty TokenQuantity)
  -- ^ A map of asset quantities to distribute.
- -> TokenMap
- -- ^ A map of non-user-specified assets to mint.
- -> TokenMap
- -- ^ A map of non-user-specified assets to burn.
  -> NonEmpty TokenMap
  -- ^ The resultant change maps.
-makeChangeForNonUserSpecifiedAssets
- n
- nonUserSpecifiedAssetQuantities
- nonUserSpecifiedMintQuantities
- nonUserSpecifiedBurnQuantities =
+makeChangeForNonUserSpecifiedAssets n nonUserSpecifiedAssetQuantities =
  F.foldr
  (NE.zipWith (<>) . makeChangeForNonUserSpecifiedAsset n)
  (TokenMap.empty <$ n)
  (Map.toList nonUserSpecifiedAssetQuantities)
- & addMintValuesToChangeMaps nonUserSpecifiedMintQuantities
- & removeBurnValuesFromChangeMaps nonUserSpecifiedBurnQuantities
 
 -- | Constructs a list of ada change outputs based on the given distribution.
 --

lib/core/test/unit/Cardano/Wallet/Primitive/CoinSelection/MA/RoundRobinSpec.hs

@@ -2558,10 +2558,8 @@ checkCoverageFor_makeChangeForNonUserSpecifiedAssets n assetQuantityMap prop =
 prop_makeChangeForNonUserSpecifiedAssets_length
  :: NonEmpty ()
  -> NonEmpty (AssetId, NonEmpty TokenQuantity)
- -> TokenMap
- -> TokenMap
  -> Property
-prop_makeChangeForNonUserSpecifiedAssets_length n assetQuantities mint burn =
+prop_makeChangeForNonUserSpecifiedAssets_length n assetQuantities =
  checkCoverageFor_makeChangeForNonUserSpecifiedAssets n assetQuantityMap $
  lengthActual === lengthExpected
  where
@@ -2570,34 +2568,30 @@ prop_makeChangeForNonUserSpecifiedAssets_length n assetQuantities mint burn =
 
  lengthActual :: Int
  lengthActual = length
- (makeChangeForNonUserSpecifiedAssets n assetQuantityMap mint burn)
+ (makeChangeForNonUserSpecifiedAssets n assetQuantityMap)
 
  lengthExpected :: Int
  lengthExpected = length n
 
 prop_makeChangeForNonUserSpecifiedAssets_order
  :: NonEmpty ()
  -> NonEmpty (AssetId, NonEmpty TokenQuantity)
- -> TokenMap
- -> TokenMap
  -> Property
-prop_makeChangeForNonUserSpecifiedAssets_order n assetQuantities mint burn =
+prop_makeChangeForNonUserSpecifiedAssets_order n assetQuantities =
  checkCoverageFor_makeChangeForNonUserSpecifiedAssets n assetQuantityMap $
  property $ inAscendingPartialOrder result
  where
  assetQuantityMap :: Map AssetId (NonEmpty TokenQuantity)
  assetQuantityMap = Map.fromList (F.toList assetQuantities)
 
  result :: NonEmpty TokenMap
- result = makeChangeForNonUserSpecifiedAssets n assetQuantityMap mint burn
+ result = makeChangeForNonUserSpecifiedAssets n assetQuantityMap
 
 prop_makeChangeForNonUserSpecifiedAssets_sum
  :: NonEmpty ()
  -> NonEmpty (AssetId, NonEmpty TokenQuantity)
- -> TokenMap
- -> TokenMap
  -> Property
-prop_makeChangeForNonUserSpecifiedAssets_sum n assetQuantities mint burn =
+prop_makeChangeForNonUserSpecifiedAssets_sum n assetQuantities =
  checkCoverageFor_makeChangeForNonUserSpecifiedAssets n assetQuantityMap $
  sumActual === sumExpected
  where
@@ -2606,31 +2600,18 @@ prop_makeChangeForNonUserSpecifiedAssets_sum n assetQuantities mint burn =
 
  sumActual :: TokenMap
  sumActual =
- F.fold
- $ makeChangeForNonUserSpecifiedAssets n assetQuantityMap mint burn
+ F.fold $ makeChangeForNonUserSpecifiedAssets n assetQuantityMap
 
  sumExpected :: TokenMap
  sumExpected =
  TokenMap.fromFlatList (Map.toList $ F.fold <$> assetQuantityMap)
- `TokenMap.add` mint
- -- makeChangeForNonUserSpecifiedAssets presumes we've performed a valid
- -- selection, and that the burns are `leq` (assetQuantities + mints).
- -- Because these values are randomly generated, this is not guaranteed
- -- when called here. When burns > (assetQuantities + mints), the
- -- function just clamps the result to zero, which we do here too (with
- -- TokenMap.difference).
- `TokenMap.difference` burn
 
 data TestDataForMakeChangeForNonUserSpecifiedAssets =
  TestDataForMakeChangeForNonUserSpecifiedAssets
  { changeMapCount
  :: NonEmpty ()
  , nonUserSpecifiedAssetQuantities
  :: Map AssetId (NonEmpty TokenQuantity)
- , assetsToMint
- :: TokenMap
- , assetsToBurn
- :: TokenMap
  , expectedResult
  :: NonEmpty TokenMap
  }
@@ -2644,8 +2625,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  makeChangeForNonUserSpecifiedAssets
  (view #changeMapCount test)
  (view #nonUserSpecifiedAssetQuantities test)
- (view #assetsToMint test)
- (view #assetsToBurn test)
  ===
  (view #expectedResult test)
  where
@@ -2674,9 +2653,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  , test6
  , test7
  , test8
- , test9
- , test10
- , test11
  ]
 
  test1 = TestDataForMakeChangeForNonUserSpecifiedAssets
@@ -2686,8 +2662,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [1])
  , ("B", [3, 2, 1])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [("A", 1), ("B", 6)] ]
  }
@@ -2699,8 +2673,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [1])
  , ("B", [3, 2, 1])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [ ("B", 3)]
  , [("A", 1), ("B", 3)]
@@ -2714,8 +2686,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [1])
  , ("B", [3, 2, 1])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [ ("B", 1)]
  , [ ("B", 2)]
@@ -2730,8 +2700,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [1])
  , ("B", [3, 2, 1])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [ ]
  , [ ("B", 1)]
@@ -2747,8 +2715,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [4, 1, 3, 2])
  , ("B", [9, 1, 8, 2, 7, 3, 6, 4, 5])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [("A", 10), ("B", 45)] ]
  }
@@ -2760,8 +2726,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [4, 1, 3, 2])
  , ("B", [9, 1, 8, 2, 7, 3, 6, 4, 5])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [("A", 4), ("B", 18)]
  , [("A", 6), ("B", 27)]
@@ -2775,8 +2739,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [4, 1, 3, 2])
  , ("B", [9, 1, 8, 2, 7, 3, 6, 4, 5])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [("A", 1), ("B", 9)]
  , [("A", 2), ("B", 9)]
@@ -2792,8 +2754,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  [ ("A", [4, 1, 3, 2])
  , ("B", [9, 1, 8, 2, 7, 3, 6, 4, 5])
  ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.empty
  , expectedResult = mkExpectedResult
  [ [ ("B", 1)]
  , [ ("B", 2)]
@@ -2807,73 +2767,6 @@ unit_makeChangeForNonUserSpecifiedAssets =
  ]
  }
 
- -- Burn all change, still preserves number of output bundles
- test9 = TestDataForMakeChangeForNonUserSpecifiedAssets
- { changeMapCount = mkChangeMapCount
- 9
- , nonUserSpecifiedAssetQuantities = mkNonUserSpecifiedAssetQuantities
- [ ("A", [4, 1, 3, 2])
- , ("B", [9, 1, 8, 2, 7, 3, 6, 4, 5])
- ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.fromFlatList
- [ mockAssetQuantity "A" 10
- , mockAssetQuantity "B" 45
- ]
- , expectedResult = mkExpectedResult
- [ []
- , []
- , []
- , []
- , []
- , []
- , []
- , []
- , []
- ]
- }
-
- -- Mints are added to the largest bundle
- test10 = TestDataForMakeChangeForNonUserSpecifiedAssets
- { changeMapCount = mkChangeMapCount
- 3
- , nonUserSpecifiedAssetQuantities = mkNonUserSpecifiedAssetQuantities
- [ ("A", [1, 3, 2])
- , ("B", [1, 2, 3])
- ]
- , assetsToMint = TokenMap.fromFlatList
- [ mockAssetQuantity "B" 2
- , mockAssetQuantity "C" 10
- ]
- , assetsToBurn = TokenMap.empty
- , expectedResult = mkExpectedResult
- [ [("A", 1), ("B", 1) ]
- , [("A", 2), ("B", 2) ]
- , [("A", 3), ("B", 5), ("C", 10)]
- ]
- }
-
- -- Burns removed from smallest bundles first
- test11 = TestDataForMakeChangeForNonUserSpecifiedAssets
- { changeMapCount = mkChangeMapCount
- 4
- , nonUserSpecifiedAssetQuantities = mkNonUserSpecifiedAssetQuantities
- [ ("A", [2, 3, 2])
- , ("B", [1, 2, 3, 4])
- ]
- , assetsToMint = TokenMap.empty
- , assetsToBurn = TokenMap.fromFlatList
- [ mockAssetQuantity "A" 1
- , mockAssetQuantity "B" 4
- ]
- , expectedResult = mkExpectedResult
- [ [ ]
- , [("A", 1) ]
- , [("A", 2), ("B", 2)]
- , [("A", 3), ("B", 4)]
- ]
- }
-
 --------------------------------------------------------------------------------
 -- Making change for known assets
 --------------------------------------------------------------------------------