Skip to content

Commit 1a4cc3f

Browse files
edskoadinapoli-iohk
andcommitted
[CBR-226] Evaluate input selection policies
[CBR-226] Improve naming [CBR-226] Improve infra, eval 3:1 event stream [CBR-226] Avoid confusing median output [CBR-226] Split x-axis Splitting the x-axis allows us to plot all data without cramming stuff into a small corner and end up with mostly empty space. [CBR-226] Minor technical gnuplot fix [CBR-226] Remove gnuplot glitch [CBR-226] Minor cleanup [CBR-226] Improve infra, test more scenarios [CBR-226] Don't render each step, render more time [CBR-226] Try to get to the ideal value for change In the original version, the larger the UTxO, the more likely that we might be able to get close the minimum end of the range, which is also when we stopped, rather than closer and closer to the ideal value. Conversely, if the UTxO contains only large outputs, then once we reached the minimum, we might end up discarding a large number of outputs (as they would all exceed the upper end of the range), potentially walking across the entire UTxO before giving up and using the fallback. This resolves both of these problems. [CBR-226] Fix memory leaks in simulator [CBR-226] Basic profiling and minor perf fixes This was primarily a sanity check to see if the input selection wasn't doing anything crazy, performance-wise. Somewhat tentative result is that it's not; all the time is spent on the periphery (computing histograms, writing to disk, etc.). Made a few minor performance improvements along the way. This also disables some tests that weren't adding value. All in all this improves the running time of the simulator by about 12x. [CBR-226] Remove quadratic complexity in renderer We can now generate significantly longer time spans because the time it takes to render the simulations is now linear in the number of cycles, rather than quadratic. This wasn't the case before because we were writing the time series data at every single step -- but that's pointless because each next file just has one more data point than the previous. Instead we can just write it once at the end and instruct gnuplot to only plot some of them. Moreover, we also only selectively include data points in the time series (based on the sampling rate). Before 10,000 cycles took 2 mins, and 20,000 cycles took 5 minutes; we can now generate 30,000 cycles in 30 seconds, and it goes up linearly (except for the largest-first, which needs to sort each step; but I've limited those to 500 cycles, which already illustrates nicely what happens). [CBR-226] Reduce number of frames [CBR-226] Sample at end of slots, UTxO balance This changes the code to sample only at the end of each slot; we were attempting to do that already but not in a very good way; now this is more explicit, and we actually do less work. 10,000 cycles now renders in 4 seconds. This also adds an additional graph of the UTxO balance, superimposed on the UTxO size. [CBR-226] Sample only at end of slot, fix gnuplot In the previous commit we were only _rendering_ at the end of slots, but still sampling at every step. Now we really only sample at the end of slots, and only for those slots we want to render, for a somewhat less accurate but much faster simulation. This also fixes the gnuplot output (we weren't dealing correctly with singleton ranges). [CBR-226] Refactor: split Policy.hs into submodules [CBR-226] Abstract away from UTxO representation This allows us to pick a different representation for the largest-first policy, increasing its efficiency. This is important if we want to be able to simulate its effects over a large number of cycles. [CBR-226] Use SortedUTxO for largest-first policy We can now run this over longer periods in reasonable time (about the same efficiency as the random policy). [CBR-226] Improve how we split the x-axis [CBR-226] Introduce composable policies This means we can now evaluate what happens with largest first, then random. [CBR-226] Introduce largest-first fallback This required some refactoring of how the random policy is defined. [CBR-226] Introduce composite SlotNr This paves the way for splitting up statistics for different policies (not yet done). However, it seems the changes to teh random policy changed it in a way that I didn't anticipate, it seems to be growing more rapidly than it used to. [CBR-226] Reset overall statistics on new policy [CBR-240] Rework InputSelectionPolicy This commit separates more strictly the toplevel InputSelectionPolicy with the underlying implementation, which doesn't take anything but a list of outputs. This allows new-policies-writers to be guided by the types, in the sense that they should solely focus on picking inputs & generating change addresses, but nothing else. All the fee handling is done automatically outside the policy, as part of 'runPolicyT'. [CBR-2401 Implement first sketch of ReceiverPaysFee [CBR-240] Initial implementation of SenderPaysFee The idea behind this commit is that in case we need to cover the fees but we don't have enough stake to do so, we forge a new output using the treasury address and we rerun the input policy on the constrained Utxo and the only, new, singleton output. We iterate up until we either not fail or we succeed. [CBR-240] Extend epsilon to be parametric on the total output [CBR-240] Use approximated formula based on tx size This commit extends the evaluator by adapting some of the work Matt Noonan did on the fee estimation by transaction size. [CBR-240] Port input selection to real types This commit ports the input selection policies and types defined in the test code, to use real Cardano types. [CBR-240] Scaffolding test infrastructure This commit pave the way for running tests and properties targeting the coin selection policy. [CBR-240] Expose largestFirst as a policy for testing. [CBR-240] Initial sketch of an improved Utxo generator Rebase everything. [CBR-240] Refactor Spec.CoinSelection to be more DRY This commit partially refactors the Spec.CoinSelection module to be more dry, introducing workhorses-combinators to pay a single output or a batch of them. [CBR-240] Simplify tests to run only in the Gen monad This commit scraps the monadic nature of the CoinSelection specs and runs everything using plain 'Property', 'forAll' and the 'Gen' monad as the baseline. [CBR-240] Assert that ReceiverPaysFee amend the outputs This commit introduces an extra check for payments that has 'ReceiverPaysFee' as an expense regulation. We check that the final outputs have been amended and they haven't been left intact. [CBR-240] Generate unique change addresses in tests This commit allows the generators to generate unique change outputs. [CBR-240] Check if the sender payed the fee. Add new checks to assess if the sender payed the fee, and add better rendering if a test fails. [CBR-226] Make coin selection abstract The vast majority of all the coin selection evaluation infrastructure is now entirely polymorphic on some kind of coin selection domain. This means that the coin selection algorithms we evaluate can be the exact same as the ones that we will run in the end. [CBR-240] Port Alfredo's code to the generic dom The coin selection simulation and the "real" coin selection algorithm are now one and the same. In the simulation we instantiate it to the DSL, and in Alfredo's unit tests we run tests on it using the real Cardano types. [CBR-240] Deal with grouping Co-authored-by: Alfredo Di Napoli <[email protected]>
1 parent 961874f commit 1a4cc3f

40 files changed

+5016
-141
lines changed

.stylish-haskell.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ language_extensions:
222222
- DefaultSignatures
223223
- DeriveDataTypeable
224224
- DeriveGeneric
225+
- ExistentialQuantification
225226
- FlexibleContexts
226227
- FlexibleInstances
227228
- FunctionalDependencies

core/src/Pos/Core/Common/Coin.hs

+33-14
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@ module Pos.Core.Common.Coin
1818
, unsafeAddCoin
1919
, unsafeSubCoin
2020
, unsafeMulCoin
21+
, addCoin
2122
, subCoin
23+
, mulCoin
2224
, divCoin
2325
) where
2426

@@ -88,14 +90,22 @@ coinToInteger :: Coin -> Integer
8890
coinToInteger = toInteger . unsafeGetCoin
8991
{-# INLINE coinToInteger #-}
9092

91-
-- | Only use if you're sure there'll be no overflow.
92-
unsafeAddCoin :: Coin -> Coin -> Coin
93-
unsafeAddCoin (unsafeGetCoin -> a) (unsafeGetCoin -> b)
94-
| res >= a && res >= b && res <= unsafeGetCoin (maxBound @Coin) = Coin res
95-
| otherwise =
96-
error $ "unsafeAddCoin: overflow when summing " <> show a <> " + " <> show b
93+
-- Addition of coins. Returns 'Nothing' in case of overflow.
94+
addCoin :: Coin -> Coin -> Maybe Coin
95+
addCoin (unsafeGetCoin -> a) (unsafeGetCoin -> b)
96+
| res >= a && res >= b && res <= unsafeGetCoin (maxBound @Coin) = Just (Coin res)
97+
| otherwise = Nothing
9798
where
9899
res = a+b
100+
{-# INLINE addCoin #-}
101+
102+
-- | Only use if you're sure there'll be no overflow.
103+
unsafeAddCoin :: Coin -> Coin -> Coin
104+
unsafeAddCoin a b =
105+
case addCoin a b of
106+
Just r -> r
107+
Nothing ->
108+
error $ "unsafeAddCoin: overflow when summing " <> show a <> " + " <> show b
99109
{-# INLINE unsafeAddCoin #-}
100110

101111
-- | Subtraction of coins. Returns 'Nothing' when the subtrahend is bigger
@@ -110,17 +120,26 @@ unsafeSubCoin :: Coin -> Coin -> Coin
110120
unsafeSubCoin a b = fromMaybe (error "unsafeSubCoin: underflow") (subCoin a b)
111121
{-# INLINE unsafeSubCoin #-}
112122

113-
-- | Only use if you're sure there'll be no overflow.
114-
unsafeMulCoin :: Integral a => Coin -> a -> Coin
115-
unsafeMulCoin (unsafeGetCoin -> a) b
116-
| res <= coinToInteger (maxBound @Coin) = Coin (fromInteger res)
117-
| otherwise = error "unsafeMulCoin: overflow"
123+
-- | Multiplication between 'Coin's. Returns 'Nothing' in case of overflow.
124+
mulCoin :: Integral a => Coin -> a -> Maybe Coin
125+
mulCoin (unsafeGetCoin -> a) b
126+
| res <= coinToInteger (maxBound @Coin) = Just $ Coin (fromInteger res)
127+
| otherwise = Nothing
118128
where
119129
res = toInteger a * toInteger b
130+
{-# INLINE mulCoin #-}
120131

121-
divCoin :: Integral a => Coin -> a -> Coin
122-
divCoin (unsafeGetCoin -> a) b =
123-
Coin (fromInteger (toInteger a `div` toInteger b))
132+
-- | Only use if you're sure there'll be no overflow.
133+
unsafeMulCoin :: Integral a => Coin -> a -> Coin
134+
unsafeMulCoin a b =
135+
case mulCoin a b of
136+
Just r -> r
137+
Nothing -> error "unsafeMulCoin: overflow"
138+
{-# INLINE unsafeMulCoin #-}
139+
140+
divCoin :: Integral b => Coin -> b -> Coin
141+
divCoin (unsafeGetCoin -> a) b = Coin (a `div` fromIntegral b)
142+
{-# INLINE divCoin #-}
124143

125144
integerToCoin :: Integer -> Either Text Coin
126145
integerToCoin n

core/test/Test/Pos/Core/Arbitrary.hs

+1-1
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,7 @@ newtype CoinPairOverflowMul = TwoCoinsM
367367
instance Arbitrary CoinPairOverflowMul where
368368
arbitrary = do
369369
c1 <- arbitrary
370-
let integralC1 = coinToInteger c1
370+
let integralC1 = Types.getCoin c1
371371
lowerBound =
372372
1 + (coinToInteger $ (maxBound @Coin) `divCoin` integralC1)
373373
upperBound = coinToInteger (maxBound @Coin)

wallet-new/cardano-sl-wallet-new.cabal

+35-1
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ library
6262
Cardano.Wallet.API.V1.Wallets
6363
Cardano.Wallet.Kernel
6464
Cardano.Wallet.Kernel.Actions
65+
Cardano.Wallet.Kernel.CoinSelection
66+
Cardano.Wallet.Kernel.CoinSelection.FromGeneric
67+
Cardano.Wallet.Kernel.CoinSelection.Generic
68+
Cardano.Wallet.Kernel.CoinSelection.Generic.Fees
69+
Cardano.Wallet.Kernel.CoinSelection.Generic.Grouped
70+
Cardano.Wallet.Kernel.CoinSelection.Generic.LargestFirst
71+
Cardano.Wallet.Kernel.CoinSelection.Generic.Random
6572
Cardano.Wallet.Kernel.DB.AcidState
6673
Cardano.Wallet.Kernel.DB.BlockMeta
6774
Cardano.Wallet.Kernel.DB.HdWallet
@@ -86,6 +93,8 @@ library
8693
Cardano.Wallet.Kernel.Submission
8794
Cardano.Wallet.Kernel.Submission.Worker
8895
Cardano.Wallet.Kernel.Types
96+
Cardano.Wallet.Kernel.Util
97+
Cardano.Wallet.Kernel.Util.StrictStateT
8998
Cardano.Wallet.LegacyServer
9099
Cardano.Wallet.Orphans
91100
Cardano.Wallet.Orphans.Aeson
@@ -138,6 +147,7 @@ library
138147
, conduit
139148
, connection
140149
, containers
150+
, cryptonite
141151
, data-default
142152
, data-default-class
143153
, exceptions
@@ -357,8 +367,16 @@ test-suite wallet-unit-tests
357367
ghc-options: -Wall
358368
type: exitcode-stdio-1.0
359369
main-is: WalletUnitTest.hs
360-
other-modules: Test.Infrastructure.Generator
370+
other-modules: InputSelection.Evaluation
371+
InputSelection.Evaluation.Generic
372+
InputSelection.Evaluation.TimeSeries
373+
InputSelection.FromGeneric
374+
InputSelection.Generator
375+
InputSelection.SortedUtxo
376+
InputSelection.TxStats
377+
Test.Infrastructure.Generator
361378
Test.Infrastructure.Genesis
379+
Test.Spec.CoinSelection
362380
Test.Spec.Kernel
363381
Test.Spec.Models
364382
Test.Spec.Submission
@@ -369,6 +387,12 @@ test-suite wallet-unit-tests
369387
Util.Buildable
370388
Util.Buildable.Hspec
371389
Util.Buildable.QuickCheck
390+
Util.Distr
391+
Util.GenHash
392+
Util.Histogram
393+
Util.MultiSet
394+
Util.QuickCheck
395+
Util.Range
372396
Util.Validated
373397
UTxO.Bootstrap
374398
UTxO.Context
@@ -432,6 +456,7 @@ test-suite wallet-unit-tests
432456
, cardano-sl-wallet-new
433457
, constraints
434458
, containers
459+
, cryptonite
435460
, data-default
436461
, formatting
437462
, hspec
@@ -442,11 +467,20 @@ test-suite wallet-unit-tests
442467
, quickcheck-instances
443468
, safe-exceptions
444469
, serokell-util
470+
, tabl
445471
, text
446472
, text-format
447473
, universum
448474
, unordered-containers
449475
, vector
476+
-- needed only for input selection evaluation
477+
, bytestring
478+
, conduit
479+
, directory
480+
, filepath
481+
, normaldistribution
482+
, random
483+
, time
450484

451485
test-suite wallet-new-specs
452486
ghc-options: -Wall
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
module Cardano.Wallet.Kernel.CoinSelection (
2+
module Exports
3+
) where
4+
5+
import Cardano.Wallet.Kernel.CoinSelection.FromGeneric as Exports
6+
import Cardano.Wallet.Kernel.CoinSelection.Generic as Exports

0 commit comments

Comments
 (0)