Skip to content

Commit

Permalink
do not mutate the reserve as part of the fee balancing process
Browse files Browse the repository at this point in the history
Everything in the past coin selection (without reserve) was easy to reason about. One could always look at the sum
of inputs and compare it with the sum of outputs and changes, and there were some nice properties about this. Having
mutable reserve makes it somewhat very confusing because, there's no trace of where the money comes from (turning
reserve into changes shouldn't make it disappear).
  • Loading branch information
KtorZ committed Jul 2, 2020
1 parent 80cc898 commit 73415e8
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 22 deletions.
16 changes: 1 addition & 15 deletions lib/core/src/Cardano/Wallet/Primitive/Fee.hs
Expand Up @@ -229,17 +229,6 @@ rebalanceSelection opts s
| φ_original == δ_original =
(s, Fee 0)

-- some fee left to pay, and the reserve is non-empty, use it first.
| φ_original > δ_original && reserve s > Just (Coin 0) =
let
-- Safe because of the above guard.
Just (Coin r) = reserve s
r' = if r > φ_original
then Coin (r - φ_original)
else Coin 0
in
rebalanceSelection opts (s { reserve = Just r' })

-- some fee left to pay, but we've depleted all change outputs
| φ_original > δ_original && null (change s) =
(s, Fee (φ_original - δ_original))
Expand Down Expand Up @@ -283,10 +272,7 @@ rebalanceSelection opts s
δ_dangling = φ_original -- by construction of the change output

extraChng = Coin (δ_original - φ_original)
sDangling = s
{ change = splitChange extraChng (change s)
, reserve = Coin 0 <$ reserve s
}
sDangling = s { change = splitChange extraChng (change s) }

-- | Reduce single change output by a given fee amount. If fees are too big for
-- a single coin, returns a `Coin 0`.
Expand Down
10 changes: 3 additions & 7 deletions lib/core/test/unit/Cardano/Wallet/Primitive/FeeSpec.hs
Expand Up @@ -52,8 +52,6 @@ import Data.Functor.Identity
( Identity (runIdentity) )
import Data.List.NonEmpty
( NonEmpty )
import Data.Maybe
( isNothing )
import Data.Word
( Word64 )
import Fmt
Expand Down Expand Up @@ -506,13 +504,11 @@ prop_rebalanceSelection sel onDangling = do
fee' /= Fee 0 || Fee (delta sel') == estimateFee opts sel'
SaveMoney ->
fee' /= Fee 0 || Fee (delta sel') >= estimateFee opts sel'
let reserveIsEmpty =
case reserve sel of
Nothing -> isNothing (reserve sel')
Just{} -> reserve sel' == Just (Coin 0)
let reserveIsUnchanged =
reserve sel == reserve sel'
conjoin
[ property selectionIsBalanced
, property reserveIsEmpty
, property reserveIsUnchanged
]
& counterexample (unlines
[ "selection (before):", pretty sel
Expand Down

0 comments on commit 73415e8

Please sign in to comment.