Allow callers of selectAssets to specify seed for random selection.

[jonathanknowles]

Issue Number

ADP-1257

Summary

This PR makes it possible for callers of selectAssets to specify a seed for random selection.

Ultimately, it will be possible to specify a seed through the API, but this PR does not make any changes to the API. That part is left to future PRs.

Details

This PR adds the following field to SelectAssetParams:

data SelectAssetsParams s result = SelectAssetsParams
      { outputs :: [TxOut]
      , pendingTxs :: Set Tx
+     , randomSeed :: Maybe StdGenSeed
      , txContext :: TransactionCtx
      , utxoAvailableForCollateral :: UTxO
      , utxoAvailableForInputs :: UTxOSelection
      , wallet :: Wallet s
      }

Additionally, this PR:

• provides a new type StdGenSeed, which is interconvertible with StdGen, but provides a representation that is more convenient for construction and serialization.
• provides a JSON serialization for StdGenSeed.

[sevanspowell]

Looks good 🙂

I think my one niggle is that selectAssets is parameterised by the generator twice, once with the MonadRandom constraint, and again with the randomSeed in SelectAssetsParams . From the outside perspective it's unclear what generator is used in what circumstance.

i.e.
-- Given some function that runs in a MonadRandom context
someFn :: MonadRandom m => m Bool

-- I can run that function with a custom seed in the StdGen generator, IO monad:
bool <- evalRandT someFn (stdGenFromSeed someSeed)

-- Now, if we also add a seed argument to someFn
someFn' :: MonadRandom m => StdGenSeed -> m Bool

-- We can call the function with two different seeds:
bool <- evalRandT (someFn' someOtherSeed) (stdGenFromSeed someSeed)

-- It's unclear which seed influences what part of the randomness of someFn'

That is to say, selectAssets is already parameterised by a seed (actually a generator, but equivalent), and so there is no need to change the API to selectAssets.

We just need to get the seed in from the HTTP API to the call to selectAssets (where necessary):

- W.selectAssets W.SelectAssetsParams
                        { ...
                        , randomSeed = Just seed
                        , ...
                        }
+ flip evalRandT (stdGenFromSeed seed) $ W.selectAssets ...

(I'm simplifying the code a bit there)

[sevanspowell]

Looks good slightly_smiling_face

I think my one niggle is that selectAssets is parameterised by the generator twice, once with the MonadRandom constraint, and again with the randomSeed in SelectAssetsParams . From the outside perspective it's unclear what generator is used in what circumstance.

i.e.
-- Given some function that runs in a MonadRandom context
someFn :: MonadRandom m => m Bool

-- I can run that function with a custom seed in the StdGen generator, IO monad:
bool <- evalRandT someFn (stdGenFromSeed someSeed)

-- Now, if we also add a seed argument to someFn
someFn' :: MonadRandom m => StdGenSeed -> m Bool

-- We can call the function with two different seeds:
bool <- evalRandT (someFn' someOtherSeed) (stdGenFromSeed someSeed)

-- It's unclear which seed influences what part of the randomness of someFn'

That is to say, selectAssets is already parameterised by a seed (actually a generator, but equivalent), and so there is no need to change the API to selectAssets.

We just need to get the seed in from the HTTP API to the call to selectAssets (where necessary):

- W.selectAssets W.SelectAssetsParams
                        { ...
                        , randomSeed = Just seed
                        , ...
                        }
+ flip evalRandT (stdGenFromSeed seed) $ W.selectAssets ...

(I'm simplifying the code a bit there)

I've heavily investigated this option (my suggestion), but it was hampered by compiler errors caused by our prevalent use of typeclasses (in the form of "capabilities") and generic-lens.

[jonathanknowles]

That is to say, selectAssets is already parameterised by a seed (actually a generator, but equivalent), and so there is no need to change the API to selectAssets.

I've heavily investigated this option (my suggestion), but it was hampered by compiler errors caused by our prevalent use of typeclasses (in the form of "capabilities") and generic-lens.

Thanks for investigating this @sevanspowell.

I completely agree with your sentiments here.

Perhaps we can view this PR as a starting point, and merge it as-is, but continue to improve on this incrementally with future PRs, if/when we figure out a better way to achieve what we want.

To make things clearer for now, I've added some commentary to selectAssets, so that callers will know what to expect.