-
Notifications
You must be signed in to change notification settings - Fork 157
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
Conversation
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.
There was a problem hiding this 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.
@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. |
There was a problem hiding this 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) |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Label fields here?
There was a problem hiding this comment.
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] |
There was a problem hiding this comment.
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] |
There was a problem hiding this comment.
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...
[ "\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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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] |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
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 inValidationTagMismatch
with descriptive types for all the different scenarios. Previously, theEq
instance forValidationTagMismatch
was ignoring this field, I suspect to make testing easier. I've made theEq
instance care about all the fields, and gave the tests a way to ignore the inner details of the plutus error message.