Merge #2632

2632: Fix `prop_{create,extend}` in `SelectionSpec`. r=Anviking a=jonathanknowles

# Issue Number

#2630 / ADP-898

# Summary

This PR fixes the following flaky tests:
- `Migration.SelectionSpec.prop_create`
- `Migration.SelectionSpec.prop_extend`

These tests would both fail around 2% of the time, for the same underlying reason.

# Testing

This PR was tested by running the entire `SelectionSpec` **1000** times. No failures were observed.

# Analysis

Both `Selection.create` and `Selection.extend` are required to produce a balanced `Selection` where the fee (and fee excess) is minimized.

Both of these functions rely on `minimizeFee`: this function is required to be **idempotent**, so that:

>  `∀ s. minimizeFee (minimizeFee s) == minimizeFee s`.

To check that `create` and `extend` produce selections with minimal fees, it's sufficient to verify that the selections they return have fees that _cannot be minimized further_. To do this, our pre-existing property tests _already_ call `minimizeFee` and verify that the result does not change:
```hs
    verify (feeExcess selection == feeExcessExpected)
  where
    (feeExcessExpected, _) =
        minimizeFee constraints (feeExcess selection, outputs selection)  
```

However, in addition to this check, we were needlessly asserting that `balance` should be idempotent. The `balance` function is not actually designed for this: it's designed to always be called with outputs that are minimal in their ada quantities.

# Changes

This PR:

- removes the unnecessary and invalid assertion for the idempotency of `balance`.
- makes the pre-condition for `balance` explicit with a documentation comment.
- adds an explicit idempotency test for `minimizeFee`.
- improves error reporting in case of test failures, by displaying the actual fee excess.

Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>

lib/core/src/Cardano/Wallet/Primitive/Migration/Selection.hs

@@ -230,6 +230,8 @@ extend constraints selection (inputId, inputBundle) =
 -- The ada quantities of the outputs are maximized in order to minimize the fee
 -- excess.
 --
+-- Pre-condition: outputs have minimal ada quantities.
+--
 -- Guarantees the following property for a returned selection 's':
 --
 -- >>> verify s == SelectionCorrect

lib/core/test/unit/Cardano/Wallet/Primitive/Migration/SelectionSpec.hs

@@ -200,16 +200,15 @@ prop_create_inner mockConstraints inputs reward =
             $ verify
                 (correctness == SelectionCorrect)
                 "correctness == SelectionCorrect"
-            . verify
-                (Selection.balance constraints selection == Right selection)
-                "Rebalancing the selection leaves it unchanged"
             . verify
                 (feeExcess selection == feeExcessExpected)
                 "feeExcess selection == feeExcessExpected"
           where
             makeReports
                 = report correctness
                     "correctness"
+                . report (feeExcess selection)
+                    "feeExcess"
                 . report feeExcessExpected
                     "feeExcessExpected"
             correctness =
@@ -301,16 +300,15 @@ prop_extend_inner mockConstraints selectionOriginal input =
             $ verify
                 (correctness == SelectionCorrect)
                 "correctness == SelectionCorrect"
-            . verify
-                (Selection.balance constraints selection == Right selection)
-                "Rebalancing the selection leaves it unchanged"
             . verify
                 (feeExcess selection == feeExcessExpected)
                 "feeExcess selection == feeExcessExpected"
           where
             makeReports
                 = report correctness
                     "correctness"
+                . report (feeExcess selection)
+                    "feeExcess"
                 . report feeExcessExpected
                     "feeExcessExpected"
             correctness =
@@ -431,6 +429,9 @@ prop_minimizeFee_inner mockConstraints feeExcessBefore outputsBefore =
         . verify
             (length outputsAfter == length outputsBefore)
             "length outputsAfter == length outputsBefore"
+        . verify
+            (feeExcessAfter == feeExcessAfterSecondRun)
+            "feeExcessAfter == feeExcessAfterSecondRun (idempotency)"
     makeCoverage
         = cover 50 (feeExcessAfter == Coin 0)
             "feeExcessAfter == 0"
@@ -443,6 +444,8 @@ prop_minimizeFee_inner mockConstraints feeExcessBefore outputsBefore =
             "feeExcessBefore"
         . report feeExcessAfter
             "feeExcessAfter"
+        . report feeExcessAfterSecondRun
+            "feeExcessAfterSecondRun"
         . report feeExcessReduction
             "feeExcessReduction"
         . report feeExcessReductionExpected
@@ -456,6 +459,9 @@ prop_minimizeFee_inner mockConstraints feeExcessBefore outputsBefore =
 
     (feeExcessAfter, outputsAfter) =
         minimizeFee constraints (feeExcessBefore, outputsBefore)
+    (feeExcessAfterSecondRun, _) =
+        minimizeFee constraints (feeExcessAfter, outputsAfter)
+
     feeExcessReduction =
         Coin.distance feeExcessBefore feeExcessAfter
     feeExcessReductionExpected =