Skip to content

Commit

Permalink
Merge #3204
Browse files Browse the repository at this point in the history
3204: Remove wallet types from coin selection test suite. r=jonathanknowles a=jonathanknowles

## Issue Numbers

Epic: ADP-1448 (_Coin Selection Type Generalization: Part 1_)

Tasks: ADP-1527, ADP-1528, ADP-1529, ADP-1530, ADP-1531.

## Summary

This PR adjusts the following modules:
- `CoinSelection.InternalSpec`
- `CoinSelection.Internal.BalanceSpec`
- `CoinSelection.Internal.CollateralSpec`
- `Primitive.Types.UTxOIndexSpec`
- `Primitive.Types.UTxOSelectionSpec`

Within these modules, we remove all usages of the following types:
- `Primitive.Types.Address`
- `Primitive.Types.TxIn`
- `Primitive.Types.TxOut`

To achieve this, we replace our dependency on `WalletSelectionContext` with a new `TestSelectionContext`:

```patch
- instance SelectionContext WalletSelectionContext where
-     type Address WalletSelectionContext = W.Address
-     type UTxO    WalletSelectionContext = W.WalletUTxO
+ instance SelectionContext TestSelectionContext where
+     type Address TestSelectionContext = TestAddress
+     type UTxO    TestSelectionContext = TestUTxO
```

From the point of view of the coin selection algorithm, the actual concrete types used for `Address ctx` and `UTxO ctx` are **_not important_**: it's **_only_** important that these types have `Ord`, `Show`, and `Buildable` instances. For the test suite, it's also important that these types have `Arbitrary` and `CoArbitrary` instances.

With that in mind, we define types `TestAddress` and `TestUTxO` in terms of the `Quid` (quasi-unique identifier) type, which provides support for **_deriving_** all these instances **_automatically_**:

```hs
newtype TestAddress = TestAddress (Hexadecimal Quid)
    deriving (Arbitrary, CoArbitrary) via Quid
    deriving Buildable via Pretty TestAddress
    deriving stock (Eq, Ord, Read, Show)

newtype TestUTxO = TestUTxO (Hexadecimal Quid)
    deriving (Arbitrary, CoArbitrary) via Quid
    deriving Buildable via Pretty TestUTxO
    deriving stock (Eq, Ord, Read, Show)
```

The `Arbitrary` instance for `Quid` generates values of a **_length_** that is governed by the QuickCheck size parameter. By adjusting the size parameter, we can control the level of "uniqueness" of generated `Quid` values, and the frequency of collisions:

- Choosing a **_lower value_** for the size parameter results in **_less uniqueness_,** and a **_higher rate_** of collisions.
- Choosing a **_higher value_** for the size parameter results in **_more uniqueness_,** and a **_lower rate_** of collisions.

For some properties, a certain level of collisions is desirable. In many cases we can satisfy our coverage checks just by adjusting the QC size parameter.

Co-authored-by: Jonathan Knowles <[email protected]>
Co-authored-by: Samuel Evans-Powell <[email protected]>
Co-authored-by: IOHK <[email protected]>
  • Loading branch information
4 people authored Mar 30, 2022
2 parents 52507fd + 50b87d0 commit 2bc2fee
Show file tree
Hide file tree
Showing 20 changed files with 689 additions and 690 deletions.
2 changes: 1 addition & 1 deletion cabal.project
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
--
--------------------------------------------------------------------------------

index-state: 2022-02-22T20:47:03Z
index-state: 2022-03-30T00:00:00Z
with-compiler: ghc-8.10.7

packages:
Expand Down
50 changes: 33 additions & 17 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions lib/core/cardano-wallet-core.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,7 @@ test-suite unit
, QuickCheck
, quickcheck-classes
, quickcheck-state-machine >= 0.6.0
, quickcheck-quid
, quiet
, random
, retry
Expand Down
Loading

0 comments on commit 2bc2fee

Please sign in to comment.