Remove hard-coded dependency on txOutMax{Coin,TokenQuantity} within coin selection modules

[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.

  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:

 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.

[Anviking]

👍

[Anviking]

Actually, wouldn't it be better to hard-code these inside coin-selection?

As long as coin-selection is specific to the Cardano blockchain (and as long they the two fields are not added to the PParams), there would be no need for any kind of user to provide any different value, right?

[Anviking]

Ah, it seems you want to be able to vary these for testing, which seems like a good reason. But when we create the main interface for the haskell+cardano-api-dependent coin-selection library, I think we should ensure these two fields are not part of it. (This might trivially end up being the case, if we take a Cardano.ProtocolParameters as an argument rather than SelectionConstraints)

[jonathanknowles]

bors r+

[iohk-bors]

Build succeeded:

• buildkite/cardano-wallet
• ci/hydra-build:required

[jonathanknowles]

Ah, it seems you want to be able to vary these for testing, which seems like a good reason. But when we create the main interface for the haskell+cardano-api-dependent coin-selection library, I think we should ensure these two fields are not part of it.

Definitely, that was also my thinking. 👍🏻

This PR is just so that the internal coin selection modules:

• don't need to depend on hard-coded constants imported from elsewhere
• can have tests that vary these constants (if necessary) to provide better coverage

For example, it might be nice to generate smaller values to provoke the splitting of token bundles more frequently, to check that properties still hold.