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

CLI Alonzo script support #2774

Merged
merged 17 commits into from
Jun 7, 2021
Merged

Conversation

dcoutts
Copy link
Contributor

@dcoutts dcoutts commented Jun 1, 2021

No description provided.

Instead of using a
  Map PolicyId (Witness WitCtxMint era)
we instead use
  Map PolicyId (ScriptWitness WitCtxMint era)

Whereas other uses of witnesses in the tx body allow either key or
script witnesses, the minting only uses script witnesses, not key
witnesses.

Using the more specific type is logically the right thing to do, and
will also make the job for the cli slightly easier.
Make it simpler and clearer. Check both for too few as well as too many
witnesses being provided for minting.

Alsoange it to use the existing createScriptWitness utility. This will
mean we can extend it for Plutus scripts more easily, since we will only
have to change that utility function, and not validateTxMintValue.
To be able to support plutus scripts for witnesses, we need to deal with
script witnesses rather than just scripts. In the CLI this means dealing
with the files needed to make a script witness, rather than just the
single file needed to make a simple script witness.

So we add a new ScriptWitnessFiles GADT that covers both the simple and
the plutus witness case, that contains the various files: not just the
script file but also files for the datum and redeemer, and also the
exection units.

This patch does not actually add in the cases to actually support the
plutus case, it just switches the types over in preparation. Both the
parser and the createScriptWitness helper will need to be extended for
that.
@dcoutts dcoutts force-pushed the dcoutts/cli-alonzo-script-support branch from ca82881 to 7d6e8a6 Compare June 2, 2021 23:09
The remaining TODO here relies on the ScriptData JSON support.
This also relies on the JSON instance for ScriptData, which is not
included yet.
And also as concrete data files in the repo.

These scripts always succeed, or always fail, so are only useful for
quick sanity tests.
It'll be getting larger once we add JSON serialisation.
Following the style of the TxMetadata
Supporting the simple schema-less JSON representation of script data.
@dcoutts dcoutts force-pushed the dcoutts/cli-alonzo-script-support branch from 6797995 to 738f311 Compare June 3, 2021 12:36
@dcoutts dcoutts marked this pull request as ready for review June 3, 2021 15:26
Collateral inouts are needed for every tx that uses Plutus scripts.
It was inverted. Oops.

Now we correctly reject txs that do not specify protocol params.
Unfortunately, the cli for providing them is not wired up yet, so not
yet possible to make a proper Alonzo tx.
Transaction that use Plutus scripts have to provide collateral inputs.
@dcoutts
Copy link
Contributor Author

dcoutts commented Jun 3, 2021

The current state here is that we cannot yet construct valid Alonzo era txs. The next bit to make that work is to be able to provide the current protocol parameters to the cardano-cli transaction build-raw command.

In the CLI code we have these TODOs:

      TxBodyContent
        <$> validateTxIns  era inputsAndScripts
        <*> validateTxInsCollateral
                           era inputsCollateral
        <*> validateTxOuts era txouts
        <*> validateTxFee  era mFee
        <*> ((,) <$> validateTxValidityLowerBound era mLowerBound
                 <*> validateTxValidityUpperBound era mUpperBound)
        <*> validateTxMetadataInEra  era metadataSchema metadataFiles
        <*> validateTxAuxScripts     era scriptFiles
        <*> pure TxAuxScriptDataNone     --TODO alonzo: support this
        <*> pure TxExtraKeyWitnessesNone --TODO alonzo: support this
        <*> pure (BuildTxWith Nothing) --TODO alonzo: support this
        <*> validateTxWithdrawals    era withdrawals
        <*> validateTxCertificates   era certFiles
        <*> validateTxUpdateProposal era mUpdatePropFile
        <*> validateTxMintValue      era mValue

It's the 3rd one there that's critical. That's the protocol params. The others are non-critical features.

One of the necessary pieces for making valid Alonzo era txs.

Not yet tested.
@dcoutts
Copy link
Contributor Author

dcoutts commented Jun 3, 2021

Ok, with any luck that'll now work. Needs to be tested.

Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

I haven't tested this as yet but it LGTM

@@ -1028,7 +1028,8 @@ data TxMintValue build era where

TxMintValue :: MultiAssetSupportedInEra era
-> Value
-> BuildTxWith build (Map PolicyId (Witness WitCtxMint era))
-> BuildTxWith build
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

--
-- It is era-independent, but witness context-dependent.
--
data ScriptWitnessFiles witctx where
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on having this in the API in some kind of IO module?

-> ExceptT ShelleyTxCmdError IO (ScriptDatum witctx)
readScriptDatumOrFile (ScriptDatumOrFileForTxIn df) = ScriptDatumForTxIn <$>
readScriptDataOrFile df
readScriptDatumOrFile NoScriptDatumOrFileForMint = pure NoScriptDatumForMint
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, nothing is stopping us from supplying a Datum with a plutus script involved in minting or withdrawals right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The consistent use of the witctx type argument should ensure that we're using only the right one in the right context. So it should be impossible to supply a datum for minting or stake contexts. For starters, there's no cli flag for it: we only have the datum cli flag for the txin context.

Copy link
Contributor

@Jimbo4350 Jimbo4350 Jun 4, 2021

Choose a reason for hiding this comment

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

I meant from the perspective of the ledger. The ledger doesn't care if we supply a datum when using a minting or stake script. IIRC Lars was utilizing tx datums in minting scrips in the plutus pioneers course.

{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TypeFamilies #-}

module Cardano.Api.ScriptData (
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

The API's TxBody binary format is not an externally defined format. It
does not appear on the chain. It is an intermediate format used by the
cli and is not guaranteed to be stable or interoperable. External tools
should not rely on this format.

Nevertheless, for now since it's easy, we'll adjust the output to be the
same as pre-Alonzo eras when not using any Alonzo-era features.
@Jimbo4350 Jimbo4350 self-requested a review June 7, 2021 09:16
@dcoutts
Copy link
Contributor Author

dcoutts commented Jun 7, 2021

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 7, 2021

@iohk-bors iohk-bors bot merged commit b3cabae into master Jun 7, 2021
@iohk-bors iohk-bors bot deleted the dcoutts/cli-alonzo-script-support branch June 7, 2021 10:56
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