Improved error message for failed asset name decode #4626

newhoggy · 2022-11-09T13:39:11Z

The new error message can be seen in this output:

cardano-cli transaction calculate-min-required-utxo --protocol-params-file ../protocol-params.json --babbage-era --tx-out "addr1q9h2kw7neeh26cmhspye2gyvhca6qaqxakp7qcp3uydzphqu62gur507vnszr24dyt2z6lrmpmrufs6jjdhc2vy04hzqyuuw8z+1 ee659e0a80e8a9815afe8aa3906741c34d234923f4da0543ec1628d4.5468654D616E6472696C6C7A576F726C6443757045646974696F6E233335302337"
option --tx-out:
unexpected end of input
expecting hexadecimal digit
AssetName deserisalisation failed: Failed to deserialise 5468654D616E6472696C6C7A576F726C6443757045646974696F6E233335302337 as AssetName. Unable to deserialise AssetName (the bytestring should be no longer than 32 bytes long which corresponds to a hex representation of 64 characters)

Usage: cardano-cli transaction calculate-min-required-utxo
            [ --byron-era
            | --shelley-era
            | --allegra-era
            | --mary-era
            | --alonzo-era
            | --babbage-era
            ]
            (--genesis FILE | --protocol-params-file FILE)
            --tx-out ADDRESS VALUE
            [ --tx-out-datum-hash HASH
            | --tx-out-datum-hash-cbor-file CBOR FILE
            | --tx-out-datum-hash-file JSON FILE
            | --tx-out-datum-hash-value JSON VALUE
            | --tx-out-datum-embed-cbor-file CBOR FILE
            | --tx-out-datum-embed-file JSON FILE
            | --tx-out-datum-embed-value JSON VALUE
            | --tx-out-inline-datum-cbor-file CBOR FILE
            | --tx-out-inline-datum-file JSON FILE
            | --tx-out-inline-datum-value JSON VALUE
            ]
            [--tx-out-reference-script-file FILE]

  Calculate the minimum required UTxO for a transaction output.

Resolves #4619

newhoggy · 2022-11-09T13:43:57Z

Notes:

The protocol parameters were obtained like this:

cardano-cli query protocol-parameters --mainnet > protocol-params.json

newhoggy · 2022-11-10T09:17:51Z

cardano-api/src/Cardano/Api/SerialiseRaw.hs

@@ -26,6 +26,9 @@ class (HasTypeProxy a, Typeable a) => SerialiseAsRawBytes a where
  serialiseToRawBytes :: a -> ByteString

  deserialiseFromRawBytes :: AsType a -> ByteString -> Maybe a
+  deserialiseFromRawBytes asType bs = rightToMaybe $ eitherDeserialiseFromRawBytes asType bs
+
+  eitherDeserialiseFromRawBytes :: AsType a -> ByteString -> Either String a


New method eitherDeserialiseFromRawBytes. Old method deserialiseFromRawBytes delegates to this.

CarlosLopezDeLara · 2022-11-10T14:05:34Z

Much better!

Jimbo4350

Looking good so far but a few comments

cardano-api/cardano-api.cabal

cardano-api/src/Cardano/Api/SerialiseUsing.hs

Jimbo4350 · 2022-11-10T14:25:02Z

cardano-api/src/Cardano/Api/SpecialByron.hs

    let lBs = LB.fromStrict bs
    in case Binary.decodeFull lBs of
-        Left _deserFail -> Nothing
-        Right proposal -> Just (ByronUpdateProposal proposal')
+        Left e -> Left $ "Unable to deserialise ByronUpdateProposal: " <> show e


In order to (more easily) maintain consistency in the errors what do you think about the following:

... eitherDeserialiseFromRawBytes :: AsType a -> ByteString -> Either (SerialiseAsRawBytesError (AsType a)) a data SerialiseAsRawBytesError a = SerialiseAsRawBytesError a instance Error (SerialiseAsRawBytesError a) where displayError (SerialiseAsRawBytesError a) = "Unable to deserialise " <> show a

Example usage:

... eitherDeserialiseFromRawBytes AsScriptHash bs = maybeToRight (SerialiseAsRawBytesError AsScriptHash) $ ScriptHash . Shelley.ScriptHash <$> Crypto.hashFromBytes bs ...

You could potentially also include the original ByteString in the SerialiseAsRawBytesError

If we did this we would only have one error value per AsType, so we couldn't have more than one kind of serialisation error per type. If deserialisation could fail in more than one way depending on what is in the byte string we would be unable to communicate to the user that distinction.

For the moment, I created a new type SerialiseAsRawBytesError which is just a wrapper around String whilst waiting for further feedback.

Comments responded to.

Jimbo4350

Looking good, few minor comments. Please squash the last two commits.

cardano-api/src/Cardano/Api/KeysPraos.hs

cardano-api/src/Cardano/Api/KeysShelley.hs

cardano-cli/src/Cardano/CLI/Byron/UpdateProposal.hs

Comments addressed

cardano-api/src/Cardano/Api/Address.hs

newhoggy · 2022-12-10T00:44:48Z

bors r+

4626: Improved error message for failed asset name decode r=newhoggy a=newhoggy The new error message can be seen in this output: ``` cardano-cli transaction calculate-min-required-utxo --protocol-params-file ../protocol-params.json --babbage-era --tx-out "addr1q9h2kw7neeh26cmhspye2gyvhca6qaqxakp7qcp3uydzphqu62gur507vnszr24dyt2z6lrmpmrufs6jjdhc2vy04hzqyuuw8z+1 ee659e0a80e8a9815afe8aa3906741c34d234923f4da0543ec1628d4.5468654D616E6472696C6C7A576F726C6443757045646974696F6E233335302337" option --tx-out: unexpected end of input expecting hexadecimal digit AssetName deserisalisation failed: Failed to deserialise 5468654D616E6472696C6C7A576F726C6443757045646974696F6E233335302337 as AssetName. Unable to deserialise AssetName (the bytestring should be no longer than 32 bytes long which corresponds to a hex representation of 64 characters) Usage: cardano-cli transaction calculate-min-required-utxo [ --byron-era | --shelley-era | --allegra-era | --mary-era | --alonzo-era | --babbage-era ] (--genesis FILE | --protocol-params-file FILE) --tx-out ADDRESS VALUE [ --tx-out-datum-hash HASH | --tx-out-datum-hash-cbor-file CBOR FILE | --tx-out-datum-hash-file JSON FILE | --tx-out-datum-hash-value JSON VALUE | --tx-out-datum-embed-cbor-file CBOR FILE | --tx-out-datum-embed-file JSON FILE | --tx-out-datum-embed-value JSON VALUE | --tx-out-inline-datum-cbor-file CBOR FILE | --tx-out-inline-datum-file JSON FILE | --tx-out-inline-datum-value JSON VALUE ] [--tx-out-reference-script-file FILE] Calculate the minimum required UTxO for a transaction output. ``` Resolves #4619 Co-authored-by: John Ky <[email protected]>

iohk-bors · 2022-12-10T05:28:42Z

Timed out.

…class More descriptive error message for decode of AssetName Introduce new SerialiseAsRawBytesError error type

newhoggy marked this pull request as ready for review November 9, 2022 13:41

newhoggy requested review from dcoutts, Jimbo4350, erikd, JaredCorduan, LudvikGalois and CarlosLopezDeLara as code owners November 9, 2022 13:41

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch 3 times, most recently from c24782d to 7531c4d Compare November 10, 2022 09:17

newhoggy commented Nov 10, 2022

View reviewed changes

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch from 7531c4d to acf4941 Compare November 10, 2022 11:39

Jimbo4350 previously requested changes Nov 10, 2022

View reviewed changes

newhoggy requested a review from Jimbo4350 November 13, 2022 11:06

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch 2 times, most recently from c26831a to 02ea740 Compare November 15, 2022 09:55

Jimbo4350 previously requested changes Nov 15, 2022

View reviewed changes

cardano-api/src/Cardano/Api/KeysPraos.hs Outdated Show resolved Hide resolved

cardano-api/src/Cardano/Api/KeysShelley.hs Outdated Show resolved Hide resolved

cardano-cli/src/Cardano/CLI/Byron/UpdateProposal.hs Outdated Show resolved Hide resolved

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch from 02ea740 to ade7df0 Compare November 16, 2022 12:13

newhoggy requested a review from Jimbo4350 November 16, 2022 12:13

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch from ade7df0 to 7fedaa4 Compare December 9, 2022 05:41

Jimbo4350 approved these changes Dec 9, 2022

View reviewed changes

cardano-api/src/Cardano/Api/Address.hs Outdated Show resolved Hide resolved

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch from 7fedaa4 to f034ded Compare December 10, 2022 00:43

newhoggy requested a review from a team as a code owner December 11, 2022 07:55

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch from f59bda4 to 9b9df9b Compare December 11, 2022 22:58

Add eitherDeserialiseFromRawBytes method to SerialiseAsRawBytes type …

1246771

…class More descriptive error message for decode of AssetName Introduce new SerialiseAsRawBytesError error type

newhoggy force-pushed the newhoggy/improve-error-message-for-failed-asset-name-decode branch from 9b9df9b to 1246771 Compare December 22, 2022 03:57

newhoggy merged commit 35b74ca into master Dec 22, 2022

iohk-bors bot deleted the newhoggy/improve-error-message-for-failed-asset-name-decode branch December 22, 2022 07:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved error message for failed asset name decode #4626

Improved error message for failed asset name decode #4626

newhoggy commented Nov 9, 2022 •

edited

Loading

newhoggy commented Nov 9, 2022

newhoggy Nov 10, 2022

CarlosLopezDeLara commented Nov 10, 2022

Jimbo4350 left a comment

Jimbo4350 Nov 10, 2022

Jimbo4350 Nov 10, 2022 •

edited

Loading

newhoggy Nov 13, 2022

newhoggy Nov 13, 2022

Jimbo4350 left a comment •

edited

Loading

newhoggy commented Dec 10, 2022

iohk-bors bot commented Dec 10, 2022

Improved error message for failed asset name decode #4626

Improved error message for failed asset name decode #4626

Conversation

newhoggy commented Nov 9, 2022 • edited Loading

newhoggy commented Nov 9, 2022

newhoggy Nov 10, 2022

Choose a reason for hiding this comment

CarlosLopezDeLara commented Nov 10, 2022

Jimbo4350 left a comment

Choose a reason for hiding this comment

Jimbo4350 Nov 10, 2022

Choose a reason for hiding this comment

Jimbo4350 Nov 10, 2022 • edited Loading

Choose a reason for hiding this comment

newhoggy Nov 13, 2022

Choose a reason for hiding this comment

newhoggy Nov 13, 2022

Choose a reason for hiding this comment

Jimbo4350 left a comment • edited Loading

Choose a reason for hiding this comment

newhoggy commented Dec 10, 2022

iohk-bors bot commented Dec 10, 2022

newhoggy commented Nov 9, 2022 •

edited

Loading

Jimbo4350 Nov 10, 2022 •

edited

Loading

Jimbo4350 left a comment •

edited

Loading