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

Add integration test for Plutus.Contracts.Currency #3036

Merged
merged 4 commits into from
Nov 30, 2021

Conversation

HeinrichApfelmus
Copy link
Contributor

@HeinrichApfelmus HeinrichApfelmus commented Nov 24, 2021

This pull request adds an integration test for the smart contract Plutus.Contract.Currency from the plutus-use-cases repository.

This test essentially constructs a transaction in binary format (CBOR) that is suitable for the transactions-balance endpoint. The Plutus script is included in pre-complied form. This approach is somewhat low-level, but follows the style of the integration tests that came before me.

Comments

  • For completeness, the code required to obtain the pre-compiled contract from the Plutus source code is recorded in the file lib/core-integration/aux/Plutus/CurrencyContract.hs. I have not made any attempt to include the necessary Plutus dependencies in the core-integration package. One should be able to run it any REPL that loads the plutus-use-cases package.
  • The pre-compiled Plutus contract needs to be edited in order to include a specific TxOutRef. However, it's not as simple as replacing a few bytes, because the binary encoding of Plutus contract is a bit-wise encoding, defined in the flat package. For reference, I have reimplemented the flat encoding of integers in the file lib/core-integration/aux/Plutus/FlatInteger.hs.
  • To make the binary format (CBOR) of the transaction easier to understand and learn, I have relied on the Haskell representation Codec.CBOR.Term.Term to construct the transaction. The format itself is defined in alonzo.cddl.

Issue Number

ADP-1221, ADP-1234

@HeinrichApfelmus HeinrichApfelmus self-assigned this Nov 24, 2021
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1234 branch from 26d42b6 to e389c36 Compare November 25, 2021 12:19
@piotr-iohk
Copy link
Contributor

piotr-iohk commented Nov 25, 2021

Got this running locally:

Failures:

  src/Test/Integration/Plutus.hs:423:29: 
  1) API Specifications, NEW_SHELLEY_TRANSACTIONS, Plutus scenarios, currency
       uncaught exception: ErrorCall
       Integration test for currency contract can only handle TxInput indices 0-31. Sorry.
       CallStack (from HasCallStack):
         error, called at src/Test/Integration/Plutus.hs:423:29 in cardano-wallet-core-integration-2021.11.11-GK96ACHnYb8G230fohhqiN:Test.Integration.Plutus

  To rerun use: --match "/API Specifications/NEW_SHELLEY_TRANSACTIONS/Plutus scenarios/currency/"

Randomized with seed 350012249

@HeinrichApfelmus
Copy link
Contributor Author

  1. API Specifications, NEW_SHELLEY_TRANSACTIONS, Plutus scenarios, currency
    uncaught exception: ErrorCall
    Integration test for currency contract can only handle TxInput indices 0-31. Sorry.

🙈 Ouch. I was hoping that in practice, unspent transaction outputs would be limited to have an index in the range 0–31. 😅 The reason for this restriction is that I have to bake the index into the Plutus contract, but this is not as simple as replacing a couple of hex bytes with new bytes — the index is stored in individual bits, and might even be variable-length encoded. 🙈 I'll try to increase the range.

=> ApiWalletInput n
-- ^ UTxO that is hard-wired into the smart contract.
-> Aeson.Value
currencyTx input = Aeson.object
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shows all the bits of the transaction very nicely!

pure $ head . view #inputs <$> result
txOutRef <- fromEither =<< getFreshUTxO
let mint = PlutusScenario.currencyTx txOutRef
pure (mint, [])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curretly all Plutus tx tests are wired to the same "Plutus scenarios" sub-suite which is neat but doesn't give opportunity for more sophisticated assertions. Like, for Currency test, we could check that "apfel" and "banana" are really minted and on wallet balance after the transaction. What do you think?

I am trying to add more assertions though when adjusting contract for e2e tests (e.g. here) so maybe that's enough.

Copy link
Contributor Author

@HeinrichApfelmus HeinrichApfelmus Nov 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like, for Currency test, we could check that "apfel" and "banana" are really minted and on wallet balance after the transaction. What do you think?

I was wondering the same thing. To be honest, I think that your e2e tests look a lot cleaner than the Haskell version. 😅 I'm not sure exactly why, but I believe the reason is that the Haskell version has a hard time constructing and deconstructing JSON values.

If you ask me, I would rather spend the time on improving the integration test DSL as a whole rather than trying to improve this individual test.

In this case, the smart contract checks that the assets are minted; thus, as long as the smart contract is touched by the node, we can be certain that these values are minted, as the node would reject the transaction otherwise.

@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1234 branch 4 times, most recently from 7380bd6 to 42b5044 Compare November 29, 2021 15:53
Copy link
Contributor

@piotr-iohk piotr-iohk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@HeinrichApfelmus
Copy link
Contributor Author

bors merge

iohk-bors bot added a commit that referenced this pull request Nov 30, 2021
3036: Add integration test for Plutus.Contracts.Currency r=HeinrichApfelmus a=HeinrichApfelmus

This pull request adds an integration test for the smart contract [Plutus.Contract.Currency](https://github.com/input-output-hk/plutus-apps/blob/main/plutus-use-cases/src/Plutus/Contracts/Currency.hs) from the [plutus-use-cases](https://github.com/input-output-hk/plutus-apps/tree/main/plutus-use-cases) repository.

This test essentially constructs a transaction in binary format (CBOR) that is suitable for the `transactions-balance` endpoint. The Plutus script is included in pre-complied form. This approach is somewhat low-level, but follows the style of the integration tests that came before me.

### Comments

* For completeness, the code required to obtain the pre-compiled contract from the Plutus source code is recorded in the file `lib/core-integration/aux/Plutus/CurrencyContract.hs`. I have not made any attempt to include the necessary Plutus dependencies in the `core-integration` package. One should be able to run it any REPL that loads the `plutus-use-cases` package.
* The pre-compiled Plutus contract needs to be edited in order to include a specific TxOutRef. However, it's not as simple as replacing a few bytes, because the binary encoding of Plutus contract is a *bit*-wise encoding, defined in the [flat][] package. For reference, I have reimplemented the flat encoding of integers in the file `lib/core-integration/aux/Plutus/FlatInteger.hs`.
* To make the binary format (CBOR) of the transaction easier to understand and learn, I have relied on the Haskell representation `Codec.CBOR.Term.Term` to construct the transaction. The format itself is defined in [alonzo.cddl](https://github.com/input-output-hk/cardano-ledger/blob/master/eras/alonzo/test-suite/cddl-files/alonzo.cddl).

  [flat]: https://hackage.haskell.org/package/flat

### Issue Number

ADP-1221, ADP-1234


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

iohk-bors bot commented Nov 30, 2021

Build failed:

Mac-mini

#2472

Uses a pre-compiled (and -serialised) contract.
Perhaps we should change the endpoint to no longer require this field, as one can always put an empty array as a value for this field.
Fixes the issue where transaction outputs with large indices would make the test fail.
… to facilitate integration testing in environments where the `flat` package is not available.
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1234 branch from 42b5044 to 97e5c0e Compare November 30, 2021 13:23
@HeinrichApfelmus
Copy link
Contributor Author

bors retry

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 30, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit 5801800 into master Nov 30, 2021
@iohk-bors iohk-bors bot deleted the HeinrichApfelmus/ADP-1234 branch November 30, 2021 14:50
WilliamKingNoel-Bot pushed a commit that referenced this pull request Nov 30, 2021
iohk-bors bot added a commit that referenced this pull request Nov 30, 2021
3040: Currency contract for e2e tests r=piotr-iohk a=piotr-iohk

- 57972f9
  Fixture: currency contract payload template
  
- 3f46b20
  Helper methods
  
- 477d3c8
  Currency contract
  
- 397f32f
  Update fixture wallets


### Comments

Contract test from #3036 adjusted for testnet e2e tests. Appears to work fine:
```
  E2E Balance -> Sign -> Submit
    cannot balance on empty wallet
    ping-pong
    game
    mint-burn
    withdrawal
    currency <----------------
```
Executed full test suite against this branch:
 - [Linux](https://github.com/input-output-hk/cardano-wallet/runs/4365345648?check_suite_focus=true) 🟢 
 - [MacOS](https://github.com/input-output-hk/cardano-wallet/runs/4365347576?check_suite_focus=true) 🟢 
 - [Windows](https://github.com/input-output-hk/cardano-wallet/runs/4365349012?check_suite_focus=true) 🟢

### Issue Number

ADP-1222, ADP-1234


Co-authored-by: Piotr Stachyra <[email protected]>
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