Condense Read and Validation modules in cardano-cli #4516
Conversation
Jimbo4350
force-pushed
the
jordan/separate-reading-and-validation
branch
4 times, most recently
from
44bee30
to
48f3600
Compare
Jimbo4350
force-pushed
the
jordan/separate-reading-and-validation
branch
from
48f3600
to
c9585e4
Compare
Jimbo4350
requested review from
deepfire,
denisshevchenko,
jutaro,
MarcFontaine,
mgmeier,
fmaste,
cleverca22,
dcoutts,
erikd,
newhoggy,
JaredCorduan,
LudvikGalois and
CarlosLopezDeLara
as code owners
reading all of the necessary things for transaction construction
with disallowing features that are not possible in a particular era.
…lidate throughout cardano-cli Delete Cardano.CLI.Shelley.Script
Jimbo4350
force-pushed
the
jordan/separate-reading-and-validation
branch
from
c9585e4
to
cd8b9af
Compare
| import Cardano.Api | ||
| import Cardano.Api.Shelley | ||
|
|
||
| --TODO: do this nicely via the API too: |
There was a problem hiding this comment.
Not sure what this TODO means.
There was a problem hiding this comment.
That's actually a comment from Duncan. I think he wanted to expose the CBOR stuff from ledger in a nicer way.
| Text.pack $ "Invalid JSON format in file: " <> show fp <> | ||
| "\nJSON parse error: " <> jsonErr | ||
| renderMetadataError (MetadataErrorConversionError fp metadataErr) = | ||
| Text.pack $ "Error reading metadata at: " <> show fp <> |
There was a problem hiding this comment.
Sometimes we say "in file: FilePath" and sometimes we say "at: FilePath".
Is there something different going on? Or should we use the same language in all cases?
There was a problem hiding this comment.
Nope just an inconsistency. I wouldn't fix that in this PR however. Open an issue if you think its worth fixing and we can get to it.
| SimpleScriptLanguage _v -> | ||
| -- TODO: We likely need another datatype eg data ReferenceScriptWitness lang | ||
| -- in order to make this branch unrepresentable. | ||
| error "readScriptWitness: Should not be possible to specify a simple script" |
| displayError sDataRangeErr | ||
| renderScriptDataError (ScriptDataErrorMetadataDecode fp decoderErr) = | ||
| Text.pack $ "Error decoding CBOR metadata at: " <> show fp <> | ||
| " Error: " <> show decoderErr |
There was a problem hiding this comment.
Language "in file" vs "at"
There was a problem hiding this comment.
See comment above
| renderCddlError :: CddlError -> Text | ||
| renderCddlError (CddlErrorTextEnv textEnvErr cddlErr) = | ||
| "Failed to decode neither the cli's serialisation format nor the ledger's \ | ||
| \CDDL serialisation format. TextEnvelope error: " <> Text.pack (displayError textEnvErr) <> "\n" <> |
There was a problem hiding this comment.
Prefer to not use multiline strings. Prefer using <>.
There was a problem hiding this comment.
Yeah fair enough. We should purge the use of \. Can you open an issue for this?
newhoggy
requested review from
newhoggy
and removed request for
denisshevchenko
|
bors r+ |
1 similar comment
|
bors r+ |
4516: Condense Read and Validation modules in cardano-cli r=Jimbo4350 a=Jimbo4350 Two things occur before constructing a tx. You must first read the parts you need and then validate that they can indeed be used in a particular era. We condense these into two module in the cli, with the eventual aim of moving them to cardano-api. Co-authored-by: Jordan Millar <[email protected]>
|
Timed out. |
| -> Either TxProtocolParametersValidationError (BuildTxWith BuildTx (Maybe ProtocolParameters)) | ||
| validateProtocolParameters _ Nothing = return (BuildTxWith Nothing) | ||
| validateProtocolParameters era (Just pparams) = | ||
| case scriptDataSupportedInEra era of |
There was a problem hiding this comment.
What's the reason we case match on scriptDataSupportedInEra in validateProtocolParameters?
Two things occur before constructing a tx. You must first read the parts you need and then validate that they can indeed be used in a particular era. We condense these into two module in the cli, with the eventual aim of moving them to cardano-api.