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

Debug help for failed Plutus scripts #2430

Merged
merged 2 commits into from
Aug 24, 2021
Merged

Debug help for failed Plutus scripts #2430

merged 2 commits into from
Aug 24, 2021

Conversation

JaredCorduan
Copy link
Contributor

@JaredCorduan JaredCorduan commented Aug 20, 2021

This adds all the necessary information for re-runing a failed plutus script inside the ValidationTagMismatch predicate failure.

Additionally, I've replaced the [Text] field in ValidationTagMismatch with descriptive types for all the different scenarios. Previously, the Eq instance for ValidationTagMismatch was ignoring this field, I suspect to make testing easier. I've made the Eq instance care about all the fields, and gave the tests a way to ignore the inner details of the plutus error message.

This adds the hex encoding of the plutus inputs to the predicate failure
associated with failed scripts. It also adds a function 'debugPlutus'
which can be used to re-run a Plutus script in verbose mode.
Copy link
Contributor

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

Seems fine. We could move the executable into plutus if we wanted: we have an executable that does this sort of thing, it's only not set up to take its arguments as a single serialized blob. But here is fine too.

@JaredCorduan JaredCorduan marked this pull request as ready for review August 23, 2021 17:52
@JaredCorduan JaredCorduan changed the title Experiment: Debug help for failed Plutus scripts Debug help for failed Plutus scripts Aug 23, 2021
@JaredCorduan
Copy link
Contributor Author

@michaelpj I've removed your review since I've taken it out of draft and changed it a lot. Don't feel obligated to review it again (unless you want to), I can get a review from the usual suspects.

Copy link
Contributor

@nc6 nc6 left a comment

Choose a reason for hiding this comment

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

So there's some weird stuff potentially with evaluation in this PR, and NoThunks checks on types that will be decidedly thunky. But none of that should change semantics, and in the name of getting a RC today, I'll merge and we can update this later.

data TagMismatchDescription
= PassedUnexpectedly
| FailedUnexpectedly [FailureDescription]
deriving (Show, Eq, Generic, NoThunks)
Copy link
Contributor

Choose a reason for hiding this comment

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

Deriving NoThunks here is a bit weird, since it's not obvious that we will have no thunks. Are we forcing the descriptions during construction?

data ScriptResult = Passes | Fails ![String]
data FailureDescription
= OnePhaseFailure Text
| PlutusFailure Text ByteString
Copy link
Contributor

Choose a reason for hiding this comment

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

Label fields here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean add some comments? that seems a good idea to me, thanks

dec 1 = SumD PlutusFailure <! From <! From
dec n = Invalid n

data ScriptResult = Passes | Fails ![FailureDescription]
Copy link
Contributor

Choose a reason for hiding this comment

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

This bang will only force the head of the list...

"The third data argument, does not decode to a context\n" ++ show info
]
db = B64.encode . serialize' $ PlutusDebug cm eu scriptbytestring ds PlutusV1
Just info2 -> Fails [PlutusFailure line db]
Copy link
Contributor

Choose a reason for hiding this comment

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

line and db will be thunks here...

@nc6 nc6 merged commit 0686b71 into master Aug 24, 2021
@iohk-bors iohk-bors bot deleted the jc/debug-help-for-plutus branch August 24, 2021 08:14
[ "\nThe 2 arg plutus script (" ++ name ++ ") fails.",
show e,
"The redeemer is: " ++ show redeemer,
"The second data argument, does not decode to a context\n" ++ show info
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the textual information here now redundant with the debug information?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, it is. maybe the human readable part should be whittled down, good point, thanks.

-- A three data argument script.
let ss :: Script era
ss = PlutusScript scriptbytestring
name :: String
name = show ss
in case P.fromData info of
Nothing -> Fails [line]
Nothing -> Fails [PlutusFailure line db]
Copy link
Contributor

Choose a reason for hiding this comment

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

also I forget, but could we put the structured type in here and just print it specially later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the train has left the station, sorry. But as for you question, I don't know, we would have to write our own show for it to produce the base64 in the logs messages, which is maybe a weird/not-always-what-you-want show instance?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're using Show for log messages? I'm not totally sure that's a good idea, but if that's the situation then okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not actually sure, to be honest.

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.

3 participants