From f86e8685a25b4f5c0d6fa1dab7bb9dd31e4dc5e6 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 01:22:04 +0000 Subject: [PATCH 1/9] [FAIL] Extend the test blockchain with a `Tx` that fails script validation. This commit extends the 'ModelSpec' test blockchain (an excerpt of the real mainnet blockchain) with a transaction that is marked as having an invalid script. When processing this transaction, the wallet software should: - spend inputs within the 'collateralInputs' field (i.e., remove entries for "owned" inputs from the wallet's UTxO set) - create outputs within the 'collateralOutput' field (i.e., insert entries for "owned" outputs into the wallet's UTxO set) This commit causes the following test failure: ``` Cardano.Wallet.Primitive.Model Compare Wallet impl. with Specification applyBlock matches the basic model from the specification [x] Failures: test/unit/Cardano/Wallet/Primitive/ModelSpec.hs:221:9: 1) Cardano.Wallet.Primitive.Model, Compare Wallet impl. with Specification, applyBlock matches the basic model from the specification Falsified (after 3 tests and 1 shrink): WalletState { _ourAddresses = fromList [82d81858...aebb3709] , _discoveredAddresses = fromList [] } - input: 1st 39633666 output: address: 82d81858...aebb3709 coin: 3823755.953610 tokens: [] - input: 2nd 74782d63 output: address: 82d81858...aebb3709 coin: 19999.799999 tokens: [] /= [] ``` --- .../Cardano/Wallet/Primitive/ModelSpec.hs | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index f28272677a3..5da4eb48122 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -1926,6 +1926,58 @@ blockchain = ] , delegations = [] } + + -- After this point, all blocks and transactions are constructed by hand, + -- in order to simulate various interesting scenarios: + + , Block + { header = BlockHeader + { slotNo = slot 14 20 + , blockHeight = Quantity 302378 + , headerHash = Hash "unused" + , parentHeaderHash = Just $ Hash "unused" + } + , transactions = + -- This transaction is marked as having an invalid script. + -- It spends a single collateral input and creates a single + -- collateral output: + [ Tx + { txId = Hash "tx-create-collateral-output" + , fee = Just (Coin 1) + , resolvedInputs = + [ ( TxIn + { inputId = Hash "9c6fed8fef3b296d4dee6e62ca72b180bf0ed1c13eb5f0445099b2a146235e77" + , inputIx = 0 + } + , Coin 3823755953610 + ) + ] + , resolvedCollateralInputs = + [ ( TxIn + { inputId = Hash "9c6fed8fef3b296d4dee6e62ca72b180bf0ed1c13eb5f0445099b2a146235e77" + , inputIx = 1 + } + , Coin 19999800000 + ) + ] + , outputs = + [ TxOut + { address = Address "\130\216\CANXB\131X\FS\147\ACKn\246.n\DLE\233Y\166)\207c\v\248\183\235\212\EOTV\243h\192\190T\150'\196\161\SOHX\RSX\FS\202>U<\156c\197&\DC3S\235C\198\245\163\204=\214fa\201\t\205\248\204\226r%\NUL\SUB\174\187\&7\t" + , tokens = coinToBundle (3823755953610 - 1) + } + ] + , collateralOutput = Just + TxOut + { address = Address "\130\216\CANXB\131X\FS\147\ACKn\246.n\DLE\233Y\166)\207c\v\248\183\235\212\EOTV\243h\192\190T\150'\196\161\SOHX\RSX\FS\202>U<\156c\197&\DC3S\235C\198\245\163\204=\214fa\201\t\205\248\204\226r%\NUL\SUB\174\187\&7\t" + , tokens = coinToBundle (19999800000 - 1) + } + , withdrawals = mempty + , metadata = Nothing + , scriptValidity = Just TxScriptInvalid + } + ] + , delegations = [] + } ] where slot e s = SlotNo $ flatSlot (EpochLength 21600) (SlotId e s) From 4972b5d4caecd69547fae44bb59fe565bf43eaaa Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 01:16:12 +0000 Subject: [PATCH 2/9] [FAIL] Adjust `Types.Tx.txIns` to return all `TxIn` values of a `Tx`. Previously, the collateral inputs were omitted. This commit does not yet fix the test failure within `ModelSpec`. --- lib/core/src/Cardano/Wallet/Primitive/Types/Tx.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/core/src/Cardano/Wallet/Primitive/Types/Tx.hs b/lib/core/src/Cardano/Wallet/Primitive/Types/Tx.hs index 34f4e36abfa..fe66ca6c864 100644 --- a/lib/core/src/Cardano/Wallet/Primitive/Types/Tx.hs +++ b/lib/core/src/Cardano/Wallet/Primitive/Types/Tx.hs @@ -307,7 +307,7 @@ instance Buildable TxScriptValidity where build TxScriptInvalid = "invalid" txIns :: Set Tx -> Set TxIn -txIns = foldMap (Set.fromList . inputs) +txIns = foldMap (\tx -> Set.fromList (inputs tx <> collateralInputs tx)) inputs :: Tx -> [TxIn] inputs = map fst . resolvedInputs From 43a75edaee8503cd2379fb67268fe9971cdb99d7 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 01:38:48 +0000 Subject: [PATCH 3/9] [FAIL] Adjust `ModelSpec.updateUTxO` to account for script validation. We fix the testing function `updateUTxO` to spend the correct set of inputs, based on whether or not the transaction is marked as having failed script validation. On its own, this commit is not enough to fix `ModelSpec` tests that use the test blockchain. --- .../test/unit/Cardano/Wallet/Primitive/ModelSpec.hs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index 5da4eb48122..83849b0207f 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -986,7 +986,15 @@ updateUTxO !b utxo = do let txs = Set.fromList $ transactions b utxo' <- (foldMap utxoFromTx txs `restrictedTo`) . Set.map snd <$> state (txOutsOurs txs) - return $ (utxo <> utxo') `excluding` txIns txs + return $ + (utxo <> utxo') `excluding` foldMap (Set.fromList . inputsToSpend) txs + where + inputsToSpend :: Tx -> [TxIn] + inputsToSpend tx + | txScriptInvalid tx = + collateralInputs tx + | otherwise = + inputs tx -- | Return all transaction outputs that are ours. This plays well within a -- 'State' monad. From b0c641d5147f3b5a13158510d59ac230ea243005 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 01:44:32 +0000 Subject: [PATCH 4/9] [PASS] Adjust `ModelSpec.txOutsOurs` to account for script validation. We fix the testing function `txOutsOurs` to create the correct set of outputs, based on whether or not the transaction is marked as having failed script validation. This commit fixes `ModelSpec` tests that use the test blockchain. --- .../test/unit/Cardano/Wallet/Primitive/ModelSpec.hs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index 83849b0207f..ac2b3fd4e17 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -1011,8 +1011,8 @@ txOutsOurs -> s -> (Set (Tx, TxOut), s) txOutsOurs txs = - runState $ Set.fromList <$> - forMaybe (foldMap (\tx -> zip (repeat tx) (outputs tx)) txs) pick + runState $ Set.fromList <$> forMaybe + (foldMap (\tx -> zip (repeat tx) (outputsToCreate tx)) txs) pick where pick :: (Tx, TxOut) -> State s (Maybe (Tx, TxOut)) pick (tx, out) = do @@ -1023,6 +1023,13 @@ txOutsOurs txs = forMaybe :: Monad m => [a] -> (a -> m (Maybe b)) -> m [b] forMaybe xs = fmap catMaybes . for xs + outputsToCreate :: Tx -> [TxOut] + outputsToCreate tx + | txScriptInvalid tx = + F.toList (collateralOutput tx) + | otherwise = + outputs tx + {------------------------------------------------------------------------------- Test Data From de73b9d728eadab30208a11cfada293b8a728881 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 01:49:23 +0000 Subject: [PATCH 5/9] Extend the test blockchain with a `Tx` that spends a collateral output. This transaction is expected to spend a collateral ouput that was created by a previous transaction, where the previous transaction was marked as having failed script validation. --- .../Cardano/Wallet/Primitive/ModelSpec.hs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index ac2b3fd4e17..f62ea9e9ab8 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -1993,6 +1993,46 @@ blockchain = ] , delegations = [] } + + , Block + { header = BlockHeader + { slotNo = slot 14 21 + , blockHeight = Quantity 302379 + , headerHash = Hash "unused" + , parentHeaderHash = Just $ Hash "unused" + } + , transactions = + -- This transaction spends a single collateral output that was + -- created in the previous transaction: + [ Tx + { txId = Hash "tx-spend-collateral-output" + , fee = Just (Coin 1) + , resolvedInputs = + [ ( TxIn + { inputId = Hash "tx-create-collateral-output" + -- The previous transaction defined exactly one + -- ordinary output, so we use 1 as the index of + -- the collateral output: + , inputIx = 1 + } + , Coin (19999800000 - 1) + ) + ] + , resolvedCollateralInputs = [] + , outputs = + [ TxOut + { address = Address "\130\216\CANXB\131X\FS\147\ACKn\246.n\DLE\233Y\166)\207c\v\248\183\235\212\EOTV\243h\192\190T\150'\196\161\SOHX\RSX\FS\202>U<\156c\197&\DC3S\235C\198\245\163\204=\214fa\201\t\205\248\204\226r%\NUL\SUB\174\187\&7\t" + , tokens = coinToBundle (19999800000 - 2) + } + ] + , collateralOutput = Nothing + , withdrawals = mempty + , metadata = Nothing + , scriptValidity = Just TxScriptValid + } + ] + , delegations = [] + } ] where slot e s = SlotNo $ flatSlot (EpochLength 21600) (SlotId e s) From fcf7e01b208c5fd590fc06292db4281a27758a8b Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 04:01:06 +0000 Subject: [PATCH 6/9] Add test utility function `outputsCreatedByTx`. --- .../unit/Cardano/Wallet/Primitive/ModelSpec.hs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index f62ea9e9ab8..e74fc687ea1 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -2380,3 +2380,20 @@ instance Show (Address -> Bool) where instance Show (RewardAccount -> Bool) where show = const "(RewardAccount -> Bool)" + +-------------------------------------------------------------------------------- +-- Utility functions +-------------------------------------------------------------------------------- + +-- | Returns the outputs that a transaction should create, based on the +-- transaction's script validation status. +-- +-- Note that the indices are not returned. If it's important to obtain the +-- indices, then use function 'utxoFromTx'. +-- +outputsCreatedByTx :: Tx -> [TxOut] +outputsCreatedByTx tx + | txScriptInvalid tx = + F.toList (collateralOutput tx) + | otherwise = + outputs tx From 10037078401cb7365d3ee46ac7d62d1c8ea181f1 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 04:26:15 +0000 Subject: [PATCH 7/9] Add test utility function `inputsSpentByTx`. --- .../test/unit/Cardano/Wallet/Primitive/ModelSpec.hs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index e74fc687ea1..5fefbd9b49c 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -2385,6 +2385,16 @@ instance Show (RewardAccount -> Bool) where -- Utility functions -------------------------------------------------------------------------------- +-- | Returns the inputs that a transaction should spend, based on the +-- transaction's script validation status. +-- +inputsSpentByTx :: Tx -> Set TxIn +inputsSpentByTx tx + | txScriptInvalid tx = + Set.fromList (collateralInputs tx) + | otherwise = + Set.fromList (inputs tx) + -- | Returns the outputs that a transaction should create, based on the -- transaction's script validation status. -- From 52a6e068f99f374c57821c88ae1940b53cf3b8d1 Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 04:07:18 +0000 Subject: [PATCH 8/9] Use `outputsCreatedByTx` to remove duplication in `ModelSpec`. --- .../Cardano/Wallet/Primitive/ModelSpec.hs | 29 ++++--------------- 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index 5fefbd9b49c..91658e0b8e7 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -1012,7 +1012,7 @@ txOutsOurs -> (Set (Tx, TxOut), s) txOutsOurs txs = runState $ Set.fromList <$> forMaybe - (foldMap (\tx -> zip (repeat tx) (outputsToCreate tx)) txs) pick + (foldMap (\tx -> zip (repeat tx) (outputsCreatedByTx tx)) txs) pick where pick :: (Tx, TxOut) -> State s (Maybe (Tx, TxOut)) pick (tx, out) = do @@ -1023,13 +1023,6 @@ txOutsOurs txs = forMaybe :: Monad m => [a] -> (a -> m (Maybe b)) -> m [b] forMaybe xs = fmap catMaybes . for xs - outputsToCreate :: Tx -> [TxOut] - outputsToCreate tx - | txScriptInvalid tx = - F.toList (collateralOutput tx) - | otherwise = - outputs tx - {------------------------------------------------------------------------------- Test Data @@ -2122,10 +2115,7 @@ prop_filterByAddress_balance_applyTxToUTxO f tx = === expectedResult where - expectedResult = - if txScriptInvalid tx - then foldMap m (collateralOutput tx) - else foldMap m (outputs tx) + expectedResult = F.foldMap m (outputsCreatedByTx tx) where m output = if f (address output) @@ -2201,10 +2191,7 @@ prop_utxoFromTx_balance tx = cover 10 (not $ txScriptInvalid tx) "not $ txScriptInvalid tx)" $ - balance (utxoFromTx tx) === - if txScriptInvalid tx - then foldMap tokens (collateralOutput tx) - else foldMap tokens (outputs tx) + balance (utxoFromTx tx) === F.foldMap tokens (outputsCreatedByTx tx) prop_utxoFromTx_size :: Tx -> Property prop_utxoFromTx_size tx = @@ -2218,10 +2205,7 @@ prop_utxoFromTx_size tx = cover 10 (not $ txScriptInvalid tx) "not $ txScriptInvalid tx)" $ - UTxO.size (utxoFromTx tx) === - if txScriptInvalid tx - then F.length (collateralOutput tx) - else F.length (outputs tx) + UTxO.size (utxoFromTx tx) === F.length (outputsCreatedByTx tx) prop_utxoFromTx_values :: Tx -> Property prop_utxoFromTx_values tx = @@ -2235,10 +2219,7 @@ prop_utxoFromTx_values tx = cover 10 (not $ txScriptInvalid tx) "not $ txScriptInvalid tx)" $ - F.toList (unUTxO (utxoFromTx tx)) === - if txScriptInvalid tx - then F.toList (collateralOutput tx) - else F.toList (outputs tx) + F.toList (unUTxO (utxoFromTx tx)) === outputsCreatedByTx tx prop_utxoFromTx_disjoint :: Tx -> Property prop_utxoFromTx_disjoint tx = From 991063f7bd2b629ef615840e2c82044514a3175b Mon Sep 17 00:00:00 2001 From: Jonathan Knowles Date: Tue, 21 Jun 2022 04:32:34 +0000 Subject: [PATCH 9/9] Use `inputsSpentByTx` to remove duplication in `ModelSpec`. --- .../Cardano/Wallet/Primitive/ModelSpec.hs | 39 +++++-------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs index 91658e0b8e7..b591e02e133 100644 --- a/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs +++ b/lib/core/test/unit/Cardano/Wallet/Primitive/ModelSpec.hs @@ -987,14 +987,7 @@ updateUTxO !b utxo = do utxo' <- (foldMap utxoFromTx txs `restrictedTo`) . Set.map snd <$> state (txOutsOurs txs) return $ - (utxo <> utxo') `excluding` foldMap (Set.fromList . inputsToSpend) txs - where - inputsToSpend :: Tx -> [TxIn] - inputsToSpend tx - | txScriptInvalid tx = - collateralInputs tx - | otherwise = - inputs tx + (utxo <> utxo') `excluding` foldMap inputsSpentByTx txs -- | Return all transaction outputs that are ours. This plays well within a -- 'State' monad. @@ -2068,10 +2061,9 @@ prop_applyTxToUTxO_balance tx u = "not $ txScriptInvalid tx" $ balance (applyTxToUTxO tx u) === expectedBalance where - expectedBalance = balance (utxoFromTx tx) <> - if txScriptInvalid tx - then balance (u `excluding` Set.fromList (collateralInputs tx)) - else balance (u `excluding` Set.fromList (inputs tx)) + expectedBalance = + balance (utxoFromTx tx) <> + balance (u `excluding` inputsSpentByTx tx) prop_applyTxToUTxO_entries :: Tx -> UTxO -> Property prop_applyTxToUTxO_entries tx u = @@ -2090,10 +2082,7 @@ prop_applyTxToUTxO_entries tx u = "not $ txScriptInvalid tx" $ applyTxToUTxO tx u === expectedResult where - expectedResult = (<> utxoFromTx tx) $ - if txScriptInvalid tx - then u `excluding` Set.fromList (collateralInputs tx) - else u `excluding` Set.fromList (inputs tx) + expectedResult = (u `excluding` inputsSpentByTx tx) <> utxoFromTx tx prop_filterByAddress_balance_applyTxToUTxO :: (Address -> Bool) -> Tx -> Property @@ -2303,14 +2292,9 @@ prop_spendTx_balance tx u = lhs === rhs where lhs = balance (spendTx tx u) - rhs = TokenBundle.unsafeSubtract (balance u) toSubtract - where - toSubtract = - if txScriptInvalid tx - then balance - (u `UTxO.restrictedBy` Set.fromList (collateralInputs tx)) - else balance - (u `UTxO.restrictedBy` Set.fromList (inputs tx)) + rhs = TokenBundle.unsafeSubtract + (balance u) + (balance (u `UTxO.restrictedBy` inputsSpentByTx tx)) prop_spendTx :: Tx -> UTxO -> Property prop_spendTx tx u = @@ -2324,12 +2308,7 @@ prop_spendTx tx u = cover 10 (not $ txScriptInvalid tx) "not $ txScriptInvalid tx" $ - spendTx tx u === u `excluding` toExclude - where - toExclude = - if txScriptInvalid tx - then Set.fromList (collateralInputs tx) - else Set.fromList (inputs tx) + spendTx tx u === u `excluding` inputsSpentByTx tx prop_spendTx_utxoFromTx :: Tx -> UTxO -> Property prop_spendTx_utxoFromTx tx u =