Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend hard-coded test blockchain to test collateral inputs and outputs. #3346

Merged
merged 9 commits into from
Jun 21, 2022

Conversation

jonathanknowles
Copy link
Member

@jonathanknowles jonathanknowles commented Jun 21, 2022

Issue Number

ADP-1814

Summary

This PR extends the hard-coded test blockchain in ModelSpec (an excerpt of the mainnet blockchain).

It appends a pair of transactions to the end: (and ensures that all tests pass)

  • Transaction t0:

    • marked as failing script validation
    • specifies a single collateral input cinp_0
    • specifies a single collateral output cout_0
  • Transaction t1:

    • marked as passing script validation
    • specifies a single ordinary input that refers to cout_0 of t0

Details

The first commit in this PR is marked with [FAIL], as it causes a test failure in "applyBlock matches the basic model from the specification".

The cause of this failure is that the following test functions were not updated to account for script validation:

  • ModelSpec.updateUTxO
  • ModelSpec.txOutsOurs

Subsequent commits adjust these functions to correctly account for script validation. The first commit to result in a passing test suite is marked with [PASS].

@sevanspowell
Copy link
Contributor

Great level of explanation 👍

My only comment is that there seems to be a lot of places where we need to update this logic of utxo 'excluding' (collateral OR input). Because these are tests I don't mind repeating ourselves though. Just food for thought.

@sevanspowell sevanspowell self-requested a review June 21, 2022 02:54
@jonathanknowles
Copy link
Member Author

jonathanknowles commented Jun 21, 2022

My only comment is that there seems to be a lot of places where we need to update this logic of utxo 'excluding' (collateral OR input). Because these are tests I don't mind repeating ourselves though. Just food for thought.

I do agree. I'm planning to make a subsequent PR that removes the duplication, at least within the test suite.

@jonathanknowles
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Jun 21, 2022
3346: Extend hard-coded test blockchain to test collateral inputs and outputs. r=jonathanknowles a=jonathanknowles

## Issue Number

ADP-1814

## Summary

This PR extends the hard-coded test blockchain in `ModelSpec` (an excerpt of the mainnet blockchain).

It appends a pair of transactions to the end: (and ensures that all tests pass)

- Transaction `t0`:
  - marked as **_failing_** script validation
  - specifies a single collateral input `cinp_0`
  - specifies a single collateral output `cout_0`
  
- Transaction `t1`:
  - marked as **_passing_** script validation
  - specifies a single ordinary input that refers to `cout_0` of `t0`

## Details

The first commit in this PR is marked with `[FAIL]`, as it causes a test failure in "_applyBlock matches the basic model from the specification_".

The cause of this failure is that the following **_test_** functions were not updated to account for script validation:
- `ModelSpec.updateUTxO`
- `ModelSpec.txOutsOurs`

Subsequent commits adjust these functions to correctly account for script validation. The first commit to result in a passing test suite is marked with `[PASS]`.

Co-authored-by: Jonathan Knowles <[email protected]>
@jonathanknowles
Copy link
Member Author

bors cancel

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 21, 2022

Canceled.

@jonathanknowles
Copy link
Member Author

@sevanspowell I've added some commits that reduce the duplication within ModelSpec.

@jonathanknowles
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Jun 21, 2022
3346: Extend hard-coded test blockchain to test collateral inputs and outputs. r=jonathanknowles a=jonathanknowles

## Issue Number

ADP-1814

## Summary

This PR extends the hard-coded test blockchain in `ModelSpec` (an excerpt of the mainnet blockchain).

It appends a pair of transactions to the end: (and ensures that all tests pass)

- Transaction `t0`:
  - marked as **_failing_** script validation
  - specifies a single collateral input `cinp_0`
  - specifies a single collateral output `cout_0`
  
- Transaction `t1`:
  - marked as **_passing_** script validation
  - specifies a single ordinary input that refers to `cout_0` of `t0`

## Details

The first commit in this PR is marked with `[FAIL]`, as it causes a test failure in "_applyBlock matches the basic model from the specification_".

The cause of this failure is that the following **_test_** functions were not updated to account for script validation:
- `ModelSpec.updateUTxO`
- `ModelSpec.txOutsOurs`

Subsequent commits adjust these functions to correctly account for script validation. The first commit to result in a passing test suite is marked with `[PASS]`.

Co-authored-by: Jonathan Knowles <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 21, 2022

Build failed:

Failure log:

[ 88 of 128] Compiling Cardano.Pool.DB.Sqlite.TH ( src/Cardano/Pool/DB/Sqlite/TH.hs, dist/build/Cardano/Pool/DB/Sqlite/TH.o, dist/build/Cardano/Pool/DB/Sqlite/TH.dyn_o )
[ 89 of 128] Compiling Cardano.Wallet.DB.Sqlite.Schema ( src/Cardano/Wallet/DB/Sqlite/Schema.hs, dist/build/Cardano/Wallet/DB/Sqlite/Schema.o, dist/build/Cardano/Wallet/DB/Sqlite/Schema.dyn_o )
[ 90 of 128] Compiling Cardano.Wallet.DB.Pure.Implementation ( src/Cardano/Wallet/DB/Pure/Implementation.hs, dist/build/Cardano/Wallet/DB/Pure/Implementation.o, dist/build/Cardano/Wallet/DB/Pure/Implementation.dyn_o )
[ 91 of 128] Compiling Cardano.Wallet.DB ( src/Cardano/Wallet/DB.hs, dist/build/Cardano/Wallet/DB.o, dist/build/Cardano/Wallet/DB.dyn_o )
[ 92 of 128] Compiling Cardano.Wallet.DB.Pure.Layer ( src/Cardano/Wallet/DB/Pure/Layer.hs, dist/build/Cardano/Wallet/DB/Pure/Layer.o, dist/build/Cardano/Wallet/DB/Pure/Layer.dyn_o )
[ 93 of 128] Compiling Cardano.Wallet   ( src/Cardano/Wallet.hs, dist/build/Cardano/Wallet.o, dist/build/Cardano/Wallet.dyn_o )
[ 94 of 128] Compiling Cardano.Wallet.Registry ( src/Cardano/Wallet/Registry.hs, dist/build/Cardano/Wallet/Registry.o, dist/build/Cardano/Wallet/Registry.dyn_o )
[ 95 of 128] Compiling Cardano.Pool.Rank.Likelihood ( src/Cardano/Pool/Rank/Likelihood.hs, dist/build/Cardano/Pool/Rank/Likelihood.o, dist/build/Cardano/Pool/Rank/Likelihood.dyn_o )
[ 96 of 128] Compiling Cardano.Pool.Rank ( src/Cardano/Pool/Rank.hs, dist/build/Cardano/Pool/Rank.o, dist/build/Cardano/Pool/Rank.dyn_o )
[ 97 of 128] Compiling Cardano.Pool.DB  ( src/Cardano/Pool/DB.hs, dist/build/Cardano/Pool/DB.o, dist/build/Cardano/Pool/DB.dyn_o )
[ 98 of 128] Compiling Cardano.Pool.DB.Model ( src/Cardano/Pool/DB/Model.hs, dist/build/Cardano/Pool/DB/Model.o, dist/build/Cardano/Pool/DB/Model.dyn_o )
[ 99 of 128] Compiling Cardano.Pool.DB.MVar ( src/Cardano/Pool/DB/MVar.hs, dist/build/Cardano/Pool/DB/MVar.o, dist/build/Cardano/Pool/DB/MVar.dyn_o )
[100 of 128] Compiling Data.Time.Text   ( src/Data/Time/Text.hs, dist/build/Data/Time/Text.o, dist/build/Data/Time/Text.dyn_o )
[101 of 128] Compiling Cardano.Wallet.Api.Types ( src/Cardano/Wallet/Api/Types.hs, dist/build/Cardano/Wallet/Api/Types.o, dist/build/Cardano/Wallet/Api/Types.dyn_o )
exprType TYPE: Text
exprType TYPE: Text
/nix/store/7zlg27zqcq9a4ks15zyr84w24lgs57x8-stdenv-linux/setup: line 1356:   111 Killed                  $SETUP_HS build lib:cardano-wallet-core -j$(($NIX_BUILD_CORES > 4 ? 4 : $NIX_BUILD_CORES))

…ation.

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: []
          /= []
```
Previously, the collateral inputs were omitted.

This commit does not yet fix the test failure within `ModelSpec`.
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.
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.
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.
@jonathanknowles jonathanknowles force-pushed the jonathanknowles/ADP-1814 branch from 231177b to 991063f Compare June 21, 2022 05:39
@jonathanknowles
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Jun 21, 2022
3346: Extend hard-coded test blockchain to test collateral inputs and outputs. r=jonathanknowles a=jonathanknowles

## Issue Number

ADP-1814

## Summary

This PR extends the hard-coded test blockchain in `ModelSpec` (an excerpt of the mainnet blockchain).

It appends a pair of transactions to the end: (and ensures that all tests pass)

- Transaction `t0`:
  - marked as **_failing_** script validation
  - specifies a single collateral input `cinp_0`
  - specifies a single collateral output `cout_0`
  
- Transaction `t1`:
  - marked as **_passing_** script validation
  - specifies a single ordinary input that refers to `cout_0` of `t0`

## Details

The first commit in this PR is marked with `[FAIL]`, as it causes a test failure in "_applyBlock matches the basic model from the specification_".

The cause of this failure is that the following **_test_** functions were not updated to account for script validation:
- `ModelSpec.updateUTxO`
- `ModelSpec.txOutsOurs`

Subsequent commits adjust these functions to correctly account for script validation. The first commit to result in a passing test suite is marked with `[PASS]`.

Co-authored-by: Jonathan Knowles <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 21, 2022

Build failed:

Lots of cached failures with the same error as #3346 (comment).

I'm fairly sure that this PR isn't the cause, because cardano-wallet-core:test:unit passes with:

Finished in 1793.1921 seconds, used 2739.2300 seconds of CPU time
4444 examples, 0 failures, 7 pending

@jonathanknowles
Copy link
Member Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 21, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 7845658 into master Jun 21, 2022
@iohk-bors iohk-bors bot deleted the jonathanknowles/ADP-1814 branch June 21, 2022 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants