simplify implementation of 'estimateMaxNumberOfInputs' & increase cov…

…erage - I've added scenarios for Byron and Icarus keys - I've added a 'within' clause to some scenarios in order to make sure that the properties are executed within a reasonable time.
cardano-foundation · Sep 4, 2020 · 8df8b86 · 8df8b86
1 parent d528050
commit 8df8b86
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 55 deletions.
diff --git a/lib/shelley/src/Cardano/Wallet/Shelley/Transaction.hs b/lib/shelley/src/Cardano/Wallet/Shelley/Transaction.hs
@@ -1,5 +1,4 @@
 {-# LANGUAGE AllowAmbiguousTypes #-}
-{-# LANGUAGE BangPatterns #-}
 {-# LANGUAGE DataKinds #-}
 {-# LANGUAGE DerivingStrategies #-}
 {-# LANGUAGE FlexibleContexts #-}
@@ -312,28 +311,18 @@ _estimateMaxNumberOfInputs
     -- ^ Number of outputs in transaction
     -> Word8
 _estimateMaxNumberOfInputs networkId (Quantity maxSize) md nOuts =
-      fromIntegral $ bisect (lowerBound, upperBound)
+    findMax minBound
   where
-    bisect (!inf, !sup)
-        | middle == inf && isTooBig sup = inf
-        | middle == inf                 = sup
-        | isTooBig middle               = bisect (inf, middle)
-        | otherwise                     = bisect (middle, sup)
-      where
-        middle = inf + ((sup - inf) `div` 2)
-
-    growingFactor = 2
-
-    lowerBound = upperBound `div` growingFactor
-    upperBound = upperBound_ 1
-      where
-        upperBound_ !n | isTooBig n = n
-                       | otherwise  = upperBound_ (n*growingFactor)
+    findMax :: Word8 -> Word8
+    findMax inf
+        | inf == maxBound    = 0
+        | isTooBig (inf + 1) = inf
+        | otherwise          = findMax (inf + 1)
 
     isTooBig nInps = size > fromIntegral maxSize
       where
         size = computeTxSize networkId (txWitnessTagFor @k) md Nothing sel
-        sel  = dummyCoinSel nInps (fromIntegral nOuts)
+        sel  = dummyCoinSel (fromIntegral nInps) (fromIntegral nOuts)
 
 dummyCoinSel :: Int -> Int -> CoinSelection
 dummyCoinSel nInps nOuts = mempty

diff --git a/lib/shelley/test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs b/lib/shelley/test/unit/Cardano/Wallet/Shelley/TransactionSpec.hs
@@ -1,3 +1,4 @@
+{-# LANGUAGE AllowAmbiguousTypes #-}
 {-# LANGUAGE DataKinds #-}
 {-# LANGUAGE DuplicateRecordFields #-}
 {-# LANGUAGE FlexibleInstances #-}
@@ -32,10 +33,14 @@ import Cardano.Wallet.Primitive.AddressDerivation
     , PassphraseScheme (..)
     , preparePassphrase
     )
+import Cardano.Wallet.Primitive.AddressDerivation.Byron
+    ( ByronKey )
+import Cardano.Wallet.Primitive.AddressDerivation.Icarus
+    ( IcarusKey )
 import Cardano.Wallet.Primitive.AddressDerivation.Shelley
     ( ShelleyKey )
 import Cardano.Wallet.Primitive.CoinSelection
-    ( CoinSelection (withdrawal), CoinSelectionOptions )
+    ( CoinSelection, CoinSelectionOptions )
 import Cardano.Wallet.Primitive.Fee
     ( Fee (..), FeeOptions (..), FeePolicy (..), adjustForFee )
 import Cardano.Wallet.Primitive.Types
@@ -51,7 +56,8 @@ import Cardano.Wallet.Primitive.Types
 import Cardano.Wallet.Shelley.Compatibility
     ( Shelley, sealShelleyTx )
 import Cardano.Wallet.Shelley.Transaction
-    ( mkByronWitness
+    ( TxWitnessTagFor
+    , mkByronWitness
     , mkShelleyWitness
     , mkUnsignedTx
     , newTransactionLayer
@@ -61,7 +67,7 @@ import Cardano.Wallet.Shelley.Transaction
 import Cardano.Wallet.Transaction
     ( TransactionLayer (..) )
 import Control.Monad
-    ( replicateM )
+    ( forM_, replicateM )
 import Control.Monad.Trans.Except
     ( catchE, runExceptT, withExceptT )
 import Data.Function
@@ -70,6 +76,8 @@ import Data.Proxy
     ( Proxy (..) )
 import Data.Quantity
     ( Quantity (..) )
+import Data.Typeable
+    ( Typeable, typeRep )
 import Data.Word
     ( Word16, Word8 )
 import Ouroboros.Network.Block
@@ -91,6 +99,7 @@ import Test.QuickCheck
     , scale
     , vectorOf
     , withMaxSuccess
+    , within
     , (===)
     , (==>)
     )
@@ -113,8 +122,20 @@ spec = do
         prop "roundtrip for Shelley witnesses" prop_decodeSignedShelleyTxRoundtrip
         prop "roundtrip for Byron witnesses" prop_decodeSignedByronTxRoundtrip
 
-    estimateMaxInputsTests Cardano.Mainnet
-    estimateMaxInputsTests (Cardano.Testnet (Cardano.NetworkMagic 0))
+    estimateMaxInputsTests @ShelleyKey Cardano.Mainnet
+        [(1,23),(10,16),(20,9),(30,2)]
+    estimateMaxInputsTests @ShelleyKey (Cardano.Testnet (Cardano.NetworkMagic 0))
+        [(1,23),(10,16),(20,9),(30,2)]
+
+    estimateMaxInputsTests @ByronKey Cardano.Mainnet
+        [(1,19),(10,12),(20,6),(30,0)]
+    estimateMaxInputsTests @ByronKey (Cardano.Testnet (Cardano.NetworkMagic 0))
+        [(1,18),(10,12),(20,5),(30,0)]
+
+    estimateMaxInputsTests @IcarusKey Cardano.Mainnet
+        [(1,19),(10,14),(20,9),(30,4)]
+    estimateMaxInputsTests @IcarusKey (Cardano.Testnet (Cardano.NetworkMagic 0))
+        [(1,18),(10,13),(20,8),(30,2)]
 
     describe "cost of withdrawal" $ do
         it "one can measure the cost of withdrawal" $ property $ \withdrawal ->
@@ -126,7 +147,7 @@ spec = do
                 minFee = fromIntegral . getFee . minimumFee tl policy Nothing Nothing
                   where tl = testTxLayer
 
-                costWith = minFee (mempty { withdrawal })
+                costWith = minFee (mempty { CS.withdrawal = withdrawal })
                 costWithout = minFee mempty
 
                 costOfWithdrawal :: Integer
@@ -162,23 +183,25 @@ spec = do
         res `shouldBe` Right (FeeEstimation 165413 165413)
 
 estimateMaxInputsTests
-    :: Cardano.NetworkId
+    :: forall k. (TxWitnessTagFor k, Typeable k)
+    => Cardano.NetworkId
+    -> [(Word8, Word8)]
     -> SpecWith ()
-estimateMaxInputsTests net =
-    describe ("estimateMaxNumberOfInputs for networkId="<> show net) $ do
-
-        it "order of magnitude, nOuts = 1" $
-            _estimateMaxNumberOfInputs @ShelleyKey net (Quantity 4096) Nothing 1 `shouldBe` 23
-        it "order of magnitude, nOuts = 10" $
-            _estimateMaxNumberOfInputs @ShelleyKey net (Quantity 4096) Nothing 10 `shouldBe` 16
-        it "order of magnitude, nOuts = 20" $
-            _estimateMaxNumberOfInputs @ShelleyKey net (Quantity 4096) Nothing 20 `shouldBe` 9
-        it "order of magnitude, nOuts = 30" $
-            _estimateMaxNumberOfInputs @ShelleyKey net (Quantity 4096) Nothing 30 `shouldBe` 2
-
-        prop "more outputs ==> less inputs" (prop_moreOutputsMeansLessInputs net)
-        prop "less outputs ==> more inputs" (prop_lessOutputsMeansMoreInputs net)
-        prop "bigger size  ==> more inputs" (prop_biggerMaxSizeMeansMoreInputs net)
+estimateMaxInputsTests net cases = do
+    let k = show $ typeRep (Proxy @k)
+    describe ("estimateMaxNumberOfInputs for "<>k<>" on "<>show net) $ do
+        forM_ cases $ \(nOuts, nInps) -> do
+            let (o,i) = (show nOuts, show nInps)
+            it ("order of magnitude, nOuts = " <> o <> " → nInps = " <> i) $
+                _estimateMaxNumberOfInputs @k net (Quantity 4096) Nothing nOuts
+                    `shouldBe` nInps
+
+        prop "more outputs ==> less inputs"
+            (prop_moreOutputsMeansLessInputs @k net)
+        prop "less outputs ==> more inputs"
+            (prop_lessOutputsMeansMoreInputs @k net)
+        prop "bigger size  ==> more inputs"
+            (prop_biggerMaxSizeMeansMoreInputs @k net)
 
 prop_decodeSignedShelleyTxRoundtrip
     :: DecodeShelleySetup
@@ -211,39 +234,48 @@ prop_decodeSignedByronTxRoundtrip (DecodeByronSetup utxo outs slotNo network pai
 
 -- | Increasing the number of outputs reduces the number of inputs.
 prop_moreOutputsMeansLessInputs
-    :: Cardano.NetworkId
+    :: forall k. TxWitnessTagFor k
+    => Cardano.NetworkId
     -> Quantity "byte" Word16
     -> Word8
     -> Property
-prop_moreOutputsMeansLessInputs net size nOuts = withMaxSuccess 1000 $
-    nOuts < maxBound ==>
-        _estimateMaxNumberOfInputs @ShelleyKey net size Nothing nOuts
+prop_moreOutputsMeansLessInputs net size nOuts
+    = withMaxSuccess 1000
+    $ within 100000
+    $ nOuts < maxBound ==>
+        _estimateMaxNumberOfInputs @k net size Nothing nOuts
         >=
-        _estimateMaxNumberOfInputs @ShelleyKey net size Nothing (nOuts + 1)
+        _estimateMaxNumberOfInputs @k net size Nothing (nOuts + 1)
 
 -- | Reducing the number of outputs increases the number of inputs.
 prop_lessOutputsMeansMoreInputs
-    :: Cardano.NetworkId
+    :: forall k. TxWitnessTagFor k
+    => Cardano.NetworkId
     -> Quantity "byte" Word16
     -> Word8
     -> Property
-prop_lessOutputsMeansMoreInputs net size nOuts = withMaxSuccess 1000 $
-    nOuts > minBound ==>
-        _estimateMaxNumberOfInputs @ShelleyKey net size Nothing (nOuts - 1)
+prop_lessOutputsMeansMoreInputs net size nOuts
+    = withMaxSuccess 1000
+    $ within 100000
+    $ nOuts > minBound ==>
+        _estimateMaxNumberOfInputs @k net size Nothing (nOuts - 1)
         >=
-        _estimateMaxNumberOfInputs @ShelleyKey net size Nothing nOuts
+        _estimateMaxNumberOfInputs @k net size Nothing nOuts
 
 -- | Increasing the max size automatically increased the number of inputs
 prop_biggerMaxSizeMeansMoreInputs
-    :: Cardano.NetworkId
+    :: forall k. TxWitnessTagFor k
+    => Cardano.NetworkId
     -> Quantity "byte" Word16
     -> Word8
     -> Property
-prop_biggerMaxSizeMeansMoreInputs net (Quantity size) nOuts = withMaxSuccess 1000 $
-    size < maxBound `div` 2 ==>
-        _estimateMaxNumberOfInputs @ShelleyKey net (Quantity size) Nothing nOuts
+prop_biggerMaxSizeMeansMoreInputs net (Quantity size) nOuts
+    = withMaxSuccess 1000
+    $ within 100000
+    $ size < maxBound `div` 2 ==>
+        _estimateMaxNumberOfInputs @k net (Quantity size) Nothing nOuts
         <=
-        _estimateMaxNumberOfInputs @ShelleyKey net (Quantity (size * 2)) Nothing nOuts
+        _estimateMaxNumberOfInputs @k net (Quantity (size * 2)) Nothing nOuts
 
 testCoinSelOpts :: CoinSelectionOptions ()
 testCoinSelOpts = coinSelOpts testTxLayer (Quantity 4096) Nothing