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

Contract instance #243

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Contract instance #243

wants to merge 11 commits into from

Conversation

carlhammann
Copy link
Contributor

@carlhammann carlhammann commented Feb 1, 2023

This PR implements the MonadBlockChain instance for the plutus-apps Contract monad. As this necessitated an update to the plutus-apps version we depend on, it might be worthwhile to look at this PR in two diffs:

  1. A mechanical adjustment of dependencies and import statements.
  2. The interesting part.

Merging this PR will close #129 and close #78. Here are the main changes:

The same balancing for both monads

In order to close #129, I decided to factor out the common steps of balancing the TxSkel and generating a balanced Tx BabbageEra into a new module Cooked.MockChain.Balancing, which is used by both for the MonadBlockChain instance of our own direct implementation and for the Contract monad. You will see that the size of Cooked.MockChain.Direct decreased drastically as a consequence and that the implementation of validateTxSkel is now very concise (as it also is for the Contract monad).

Changed primitives in MonadBlockChain

In order to implement balancing for both monads, I decided to introduce another class below MonadBlockChainWithoutValidation, called MonadBlockChainBalancing that has a minimal set of primitives needed for balancing.

I also decided to change the type of the primitives that return a TxOut so that they return a Ledger.TxOut, and not a TxInfo-TxOut. The latter can always be obtained from the former, but not vice versa. This means that allUtxos and friends are now not primitives any more, but defined in terms of primitives. See Issue #239 for some further improvements I think are necessary in this area.

Open questions / What's left to do

  • Write some tests for the instance. I want to make sure that it actually works. Does someone here have experience with the Contract monad? If yes, I'd like to pair on that.
  • Actually close Change validateTx for the contract monad, making it more flexible #78. This amounts to adding one or two lines to the validateTxSkel implementation of the Contract monad, which allow the application of RawModTx.
    • Edit: Addressed in 6c305f7. I decided to make applying the RawModTx at transaction generation time, because it is a genuine part of the TxSkel, and not the task of either monad to explicitly remember applying it.
  • I'm not happy with the MonadError/MonadFail situation.
    • We rely on MonadBlockChain being a MonadFail quite heavily: Out usual style of writing offchain code is full of incomplete pattern matches that we know will never fail.
    • We also want to be able to throw informative errors (and catch them, at least during balancing), so MonadError is also useful.
    • The MonadFail and MonadError instances for the Contract monad feel like hacks, and they are, because Contract is not a MonadFail natively.
  • The Contract monad has no facility to list all known UTxOs, as the functions around allUtxos do. It's only possible to return all UTxOs belonging to a specific address. This is now a primitive utxosAtLedger. The Contract instance throws an error if one tries to retrieve all UTxOs. Is this a good solution, or should we abandon allUtxos or move it to a separate class?

@carlhammann
Copy link
Contributor Author

As this turns out to be hairier than expected, I decided to separate individually valuable contributions out to separate PRs #250 and #251.

@Niols
Copy link
Member

Niols commented Mar 1, 2023

Now that #311 has been merged, you will have a hairy conflict with main to resolve... Apart from that, what is the status of this PR? It hasn't moved in a month.

@carlhammann
Copy link
Contributor Author

what is the status of this PR?

The status is that we decided to not have the (surprisingly hairy) Contract instance as a part of the v2.0.0 release, and focus our energies on something else. I think this specific PR will never be merged, but I want to keep it around as reference for when we finally decide to bite the bullet an implement the instance.

@carlhammann
Copy link
Contributor Author

carlhammann commented Mar 2, 2023

In particular, the three (merged) PRs #252, #250, and #251 were all derived from work done here first. So, I think this has already served as a good exploration vessel.

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