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

Changing the order of constructors in ADT representing State has side-effects on validation #3601

Closed
abailly-iohk opened this issue Jul 23, 2021 · 4 comments
Assignees
Labels

Comments

@abailly-iohk
Copy link
Contributor

Area

[x] Plutus Foundation Related to the GHC plugin, Haskell-to-Plutus compiler, on-chain code
[x] Plutus Application Framework Related to the Plutus application backend (PAB), emulator, Plutus libraries
[] Marlowe Related to Marlowe
[] Other Any other topic (Playgrounds, etc.)

Summary

We are working on plutus smart contracts for Hydra Head, currently trying to complete the lifecycle with a close operation.
When we try to test that operation using Plutus test framework in emulator, we are observing odd behavior in the validation process: Depending on the order of constructors of the ADT representing the State of the contract, emulator fails on different endpoint calls.

Steps to reproduce

Steps to reproduce the behavior:

  1. Clone hydra-poc repository at commit https://github.com/input-output-hk/hydra-poc/tree/378c9e62ef6f94207e8193fd1c2c83df66cb6ecc
  2. run nix-shell and then cabal test hydra-plutus. Tests should fail on close endpoint.
  3. Change the order of the constructors in the State ADT, for example switching Final and Closed Snapshot (https://github.com/input-output-hk/hydra-poc/blob/378c9e62ef6f94207e8193fd1c2c83df66cb6ecc/hydra-plutus/src/Hydra/Contract/OnChain.hs#L61)
  4. run cabal test hydra-plutus again: Tests fail on collectCom endpoint.

Expected behavior

Tests outcome should not change depending on the order of constructors, they should fail on close always.

System info (please complete the following information):

  • OS: Ubuntu
  • Version: 20.04
  • Plutus version or commit hash: e3e220f
@luigy luigy self-assigned this Jul 23, 2021
@michaelpj
Copy link
Contributor

  • Changing the order of the constructors will change the script and the script hash, as well as the order in which matching branches get evaluated, so in principle this can result in e.g. different side effects happening. I'd be surprised if that's what's happening, though, it doesn't sound like a side-effect ordering issue.
  • You are using unstableMakeIsData - do you have any checked in datums by any chance? That could result in the datum failing to decode.

@ch1bo
Copy link

ch1bo commented Jul 29, 2021

Changing the order of the constructors will change the script and the script hash, as well as the order in which matching branches get evaluated, so in principle this can result in e.g. different side effects happening. I'd be surprised if that's what's happening, though, it doesn't sound like a side-effect ordering issue.

Of course the hash is a different one, but that's not our problem. The problem is that our tests (assertions on an EmulatorTrace) behave different when do simply change the order of data constructors. This is also true for adding traceIfFalse expressions to the validator which makes it somewhat impossible to us to debug / progress the contract development.

You are using unstableMakeIsData - do you have any checked in datums by any chance? That could result in the datum failing to decode.

We are not using any seriaized / golden test data, if that's what you mean.

@effectfully
Copy link
Contributor

I'm cleaning up tickets, @abailly-iohk does the problem described in this ticket still persist or can we close the ticket?

@abailly-iohk
Copy link
Contributor Author

We don't know if the problem persists as we don't use EmulatorTrace anymore. I close the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants