Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize UTxO identifier type within UTxOIndex. #3154

Conversation

jonathanknowles
Copy link
Contributor

@jonathanknowles jonathanknowles commented Feb 23, 2022

Issue Number

ADP-1418

Summary

This PR adds the type parameter u to the UTxOIndex type, to represent unique UTxO identifiers. The parameter u can be instantiated to any type for which there is an Ord instance.

- data UTxOIndex   = UTxOIndex
+ data UTxOIndex u = UTxOIndex
      { assetsAll
-         :: !(Map AssetId (NonEmptySet InputId ))
+         :: !(Map AssetId (NonEmptySet u       ))
          -- An index of all entries that contain at least one non-ada asset.
      , assetsSingleton
-         :: !(Map AssetId (NonEmptySet InputId))
+         :: !(Map AssetId (NonEmptySet u      ))
          -- An index of all entries that contain exactly one non-ada asset.
      , coins
-         :: !(Set InputId)
+         :: !(Set u      )
          -- An index of all entries that contain no non-ada assets.
      , balance
          :: !TokenBundle
          -- The total balance of all entries.
-     , utxo
+     , universe
-         :: !(Map InputId TokenBundle)
+         :: !(Map u       TokenBundle)
          -- The complete set of all entries.
      }

Additional Work

This PR also:

  • Revises comments within UTxOIndex module to reflect the UTxOIndex type's generality.
  • Revises names for identifiers within UTxOIndex according to the following scheme:
    - u represents a unique identifier for an individual UTxO (unspent transaction output).
    - b represents a TokenBundle value.
    - i represents an index of type UTxOIndex.
  • Renames functions UTxOIndex.{to,from}UTxO to UTxOIndex.{to,from}Map, since these functions really do produce and consume values of type Map.
  • Renames internal field utxo of type UTxOIndex to universe, to avoid overloading the term UTxO, which refers to a single unspent transaction output in this module.

@jonathanknowles jonathanknowles self-assigned this Feb 23, 2022
@jonathanknowles jonathanknowles changed the title Use type parameter for UTxO identifiers in UTxOIndex Generalize UTxO identifier type within UTxOIndex. Feb 23, 2022
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/use-type-parameter-for-utxo-identifier-in-utxo-index branch 2 times, most recently from de16df5 to 3e8aa22 Compare February 23, 2022 04:56
@jonathanknowles jonathanknowles marked this pull request as ready for review February 23, 2022 05:01
@@ -505,7 +508,7 @@ prop_selectRandom_one_withAdaOnly u = checkCoverage $ monadicIO $ do
-- This should only succeed if there is at least one element with a non-zero
-- quantity of the asset.
--
prop_selectRandom_one_withAsset :: UTxOIndex -> AssetId -> Property
prop_selectRandom_one_withAsset :: UTxOIndex InputId -> AssetId -> Property
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these properties would work with any Ord i => UTxOIndex i right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these properties would work with any Ord i => UTxOIndex i right?

Yes, precisely!

I haven't converted the test suites yet, but I'm thinking of making a simple TestUTxOKey type with generators and shrinkers for it.

I haven't done this yet, because I predict some effort (though not too much) with be required to maintain the coverage conditions.

@@ -234,7 +234,7 @@ toList = fold (\ubs u b -> (u, b) : ubs) []
-- Consider using 'fold' if your goal is to consume all entries in the output.
--
toUTxO :: UTxOIndex u -> Map u TokenBundle
toUTxO = utxo
toUTxO = universe
Copy link
Member

@Anviking Anviking Feb 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utxos? allUTxOs? rawUTxO? Maybe universe is a bit too "dressed up"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utxos? allUTxOs? rawUTxO? Maybe universe is a bit too dressed up

I take your point. 😄

How about everything?

@jonathanknowles jonathanknowles force-pushed the jonathanknowles/coin-selection-compound-input-id branch from 59c9fbe to a269694 Compare February 23, 2022 23:06
We use the following convention for identifier names:

  - `u` represents a unique identifier for an individual UTxO (unspent
        transaction output).
  - `b` represents a `TokenBundle` value.
  - `i` represents an index of type `UTxOIndex`.
The former field name was confusing because it overloaded the term "UTxO"
(used elsewhere in the module to refer to a single UTxO) with the complete
set of UTxOs in the index.
These names better reflect the types involved.
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/use-type-parameter-for-utxo-identifier-in-utxo-index branch from 3e8aa22 to 6d8a198 Compare February 23, 2022 23:12
@jonathanknowles jonathanknowles merged commit 82c8e38 into jonathanknowles/coin-selection-compound-input-id Feb 23, 2022
@jonathanknowles jonathanknowles deleted the jonathanknowles/use-type-parameter-for-utxo-identifier-in-utxo-index branch February 23, 2022 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants