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

Initial CLI update for Allegra and Mary eras #2129

Merged
merged 18 commits into from
Nov 27, 2020
Merged

Conversation

dcoutts
Copy link
Contributor

@dcoutts dcoutts commented Nov 26, 2020

Re-implement the tx build-raw command in terms of the new era-generic TxBodyContent API.

This should allow it to work for all eras, including Byron. It checks that each feature can be used in the selected era.

There are still a number of TODOs, but this establishes the pattern.

@dcoutts
Copy link
Contributor Author

dcoutts commented Nov 26, 2020

This builds on #2121 and should be rebased once that PR is merged.

@dcoutts dcoutts force-pushed the dcoutts/cli-allegra-mary branch 2 times, most recently from 4f8b2c6 to 38700db Compare November 26, 2020 10:39
@Jimbo4350 Jimbo4350 force-pushed the dcoutts/cli-allegra-mary branch 2 times, most recently from c4a53ca to 3a5591e Compare November 26, 2020 13:20
@kevinhammond
Copy link
Contributor

Quick comment - should we be exposing the word "era" in the CLI? We've backed away from this in the product/marketing areas

dcoutts and others added 15 commits November 26, 2020 22:05
It's more convenient this way since it means we can use it when we have
a CardanoEra value or when we have an IsCardanoEra class constraint.
This lets us dynamically compare era tokens and establish if they're the
same. This will be useful in the CLI for example where we read two files
and need to check they're from the same era.
This pairs up some era-dependent type with a 'CardanoEra' value that
tells us what era it is, but hides the era type. This is useful when
the era is not statically known, for example when deserialising from
a file.

We will use this in the CLI when reading files, like tx, txbody and
witness files.

Also add InAnyShelleyBasedEra following the same pattern, which we need
for Shelley-based era features like scripts.
Adjust CardanoEraStyle, InAnyCardanoEra and InAnyShelleyBasedEra to
carry a corresponding class context as well as the explicit value
representation of the era.

This turns out to be rather useful since it lets us use functions that
expect an explicit arg, or ones that expect an implicit one via a class
constraint.
They tell us if the feature is available in a given era, and if so
provide us with the evidence. This will make working with these features
(e.g. in the CLI) easier to do in a era-general kind of way.
GADTs can only provide positive evidence, and we sometimes need positive
evidence of things that are negations of each other.

For example: whether the era supports multi-asset or ada-only are
opposites of each other. We need to provide this positive evidence
in either direction when making a TxOutValue for example

data TxOutValue era where
     TxOutAdaOnly :: OnlyAdaSupportedInEra era -> ...
     TxOutValue   :: MultiAssetSupportedInEra era -> ...

So it is more helpful if our feature functions give us the evidence in
both directions for these cases, so specifically:

multiAssetSupportedInEra :: CardanoEra era
                         -> Either (OnlyAdaSupportedInEra era)
                                   (MultiAssetSupportedInEra era)

The same holds for tx fees: implicit vs explicit. Adjust the TxFee
representation so it follows the same pattern with positive and negative
evidence with a new type TxFeesImplicitInEra, and provide

txFeesExplicitInEra :: CardanoEra era
                    -> Either (TxFeesImplicitInEra era)
                              (TxFeesExplicitInEra era)
Defaults to --shelley-era

When we make a transaction we need to know which era it is for.
For going from an untyped UseCardanoEra enumeration to the strongly
typed CardanoEra era.
We now use a per-field validation that checks that the feature is
available to use in that era.
Switch the type that the command line parsers use from TxOut ShelleyEra
to a new universal type TxOutAnyEra.

Add CLI parser support for tx lower bounds, aux scripts and value
minting. Make the tx upper bound and fee optional.

Update runTxBuildRaw to use all of these, resolving several TODOs.

Add validation that multi-asset tx-outs can only be used in eras
supporting them, with appropriate failure messages otherwise.
For Tx, TxBody and Witness. We need to be able to read these files at
the types corresponding to any of the supported eras.
Easy because all the functions they call have already been generalised.
This is slightly awkward. In principle the right thing to do is probably
to do what we did with addresses and split out the language vs the era,
so that we can talk about script languages independently from the eras
in which those languages are supported.

This would let us read scripts into some universal type and later check
that the script language is supported in the era in which it is intended
to be used.

For now however in this PR we will hack it for now. We'll do the
language vs era separation in a later PR. To do the temporary hack we
add a function coerceSimpleScriptEra that lets us use the type
MultiSigScript ShelleyEra as our "universal" type and the coerce
function is to convert into the era. In this case we're using the
minimal rather than maximal features, so the conversion is total.

In this first step we're doing the identity era coersion. That will
change in the next step.
These all work by reading a tx body. We use the era tx body is for to
determine the era to use for everything else in each command.

For a simple command like txid there's nothing else, so it will work for
any era in a straightforward way.

For most other commands the code can currently only support
Shelley-based eras, so we add a dynamic test to fail for the Byron era.

For witness and sign commands the tx body era simply determines the
witness era.

For assemble things are more interesting since we have to combine a
txbody with a number of witnesses. We use a testEquality to check if the
era of each witness matches the era of the txbody.
@dcoutts
Copy link
Contributor Author

dcoutts commented Nov 26, 2020

Quick comment - should we be exposing the word "era" in the CLI? We've backed away from this in the product/marketing areas

Yes. The CLI exposes the technical constraints. The tx build command needs to know the era in which the tx is intended to be used, because different features are available in different eras, and they have potentially different binary formats. This is a technicality that we cannot hide.

Needs a slight tweak to submitTx in the API to cover the Allegra and
Mary era cases.
Comment on lines +1504 to +1505
readValue :: String -> Either String Value
readValue _maCliSyntax = Left "Need 2072 for MA cli syntax parser"
Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I'll patch this up once we merge this PR.

Comment on lines 1507 to 1523
pTxLowerBound :: Parser SlotNo
pTxLowerBound =
SlotNo <$>
Opt.option Opt.auto
( Opt.long "ttl"
( Opt.long "lower-bound"
<> Opt.metavar "SLOT"
<> Opt.help "Time to live (in slots)."
<> Opt.help "Time that transaction is valid from (in slots)."
)

pTxUpperBound :: Parser SlotNo
pTxUpperBound =
SlotNo <$>
Opt.option Opt.auto
( Opt.long "upper-bound"
<> Opt.metavar "SLOT"
<> Opt.help "Time that transaction is valid until (in slots)."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should also note whether these endpoints (lower and upper bound) are inclusive or exclusive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should yes. It's inclusive on the lower bound and exclusive on the upper.

Comment on lines +227 to +239
txBodyContent <-
TxBodyContent
<$> validateTxIns era txins
<*> validateTxOuts era txouts
<*> validateTxFee era mFee
<*> ((,) <$> validateTxValidityLowerBound era mLowerBound
<*> validateTxValidityUpperBound era mUpperBound)
<*> validateTxMetadataInEra era metadataSchema metadataFiles
<*> validateTxAuxScripts era scriptFiles
<*> validateTxWithdrawals era withdrawals
<*> validateTxCertificates era certFiles
<*> validateTxUpdateProposal era mUpdatePropFile
<*> validateTxMintValue era mValue
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! I like how this came out.

@intricate
Copy link
Contributor

bors r+

iohk-bors bot added a commit that referenced this pull request Nov 26, 2020
2129: Initial CLI update for Allegra and Mary eras r=intricate a=dcoutts

Re-implement the tx build-raw command in terms of the new era-generic TxBodyContent API.

This should allow it to work for all eras, including Byron. It checks that each feature can be used in the selected era.

There are still a number of TODOs, but this establishes the pattern.

Co-authored-by: Duncan Coutts <duncan@well-typed.com>
Co-authored-by: Jordan Millar <jordan.millar@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 27, 2020

Build failed:

@intricate
Copy link
Contributor

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 27, 2020

@iohk-bors iohk-bors bot merged commit 62faef3 into master Nov 27, 2020
@iohk-bors iohk-bors bot deleted the dcoutts/cli-allegra-mary branch November 27, 2020 03:25
@kevinhammond
Copy link
Contributor

Also, will the txout syntax need to be updated? Seems to only allow lovelace at the moment

@intricate
Copy link
Contributor

intricate commented Nov 27, 2020

@kevinhammond: Yup, there's a patch for that in #2072.

@kevinhammond
Copy link
Contributor

Thanks!

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.

4 participants