AVM: report structure txn failure info

[jannotti]

This is a proposal to add structured info (PC, stack, scratch, logs) from failed transactions in the JSON response.

I am less concerned with guaranteeing backward compatibility on this structure until some time and usage has occurred, so it is not codified in an openapi file. I think we should consider the .data field of the returned JSON to fall outside the usual semvar promise --- it is more like an error message that we hope is useful, but is not yet cast in stone. For example, we do not specify the the potential Data returns on error in /v2/applications/{application-id}/boxes and /v2/accounts/{address}.

See avm-failure-info.py and TestAppErrorDetails in evalStateful_test.go for the best examples of the proposed return values.

[codecov]

Codecov Report

Attention: 14 lines in your changes are missing coverage. Please review.

Comparison is base (e7aa0d4) 55.95% compared to head (35526b7) 55.97%.
Report is 1 commits behind head on master.

Files	Patch %	Lines
data/basics/serr.go	82.00%	8 Missing and 1 partial ⚠️
daemon/algod/api/server/v2/utils.go	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5875      +/-   ##
==========================================
+ Coverage   55.95%   55.97%   +0.02%     
==========================================
  Files         477      478       +1     
  Lines       67436    67535      +99     
==========================================
+ Hits        37736    37805      +69     
- Misses      27149    27174      +25     
- Partials     2551     2556       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nullun]

I've been playing around with this all morning and it helps tremendously with identifying where and in what state something failed at! I briefly thought showing resources like foreign assets/apps and app accounts would be useful too, but those are in the transaction you submit, so there's no real benefit to that.

One thing, which may fall outside of this PR is the contents of the message/inner-msg. Is there any reason to keep appending them if you can see them individually within the inners? For example I created a 8-deep inner transaction call which failed on the deepest one, and this is the bottom of the data, lots of duplicated/appended messages.

[jannotti]

One thing, which may fall outside of this PR is the contents of the message/inner-msg. Is there any reason to keep appending them if you can see them individually within the inners? For example I created a 8-deep inner transaction call which failed on the deepest one, and this is the bottom of the data, lots of duplicated/appended messages.

That does start to look a bit out of hand, but my thinking was that it made the single top-level message (in cases where existing code or UIs only reports that) more useful. It becomes more akin to a stack trace so you can see which inner actually caused a problem. I know I had run into problems where a test with inner fails and it was not immediately obvious which depth had the error.

Your example looks bad, but I think in practice you're only one or two levels deep and so the duplication is not so bad and remains helpful. If we did only report a single error, would it be the top-level message and PC (which would always be on a ixtn_submit), or the deepest?

[nullun]

That makes sense, there's likely systems out there that are already passing the top-level message as it is.

If we did only report a single error, would it be the top-level message and PC (which would always be on a ixtn_submit), or the deepest?

Hmmm, I'd say the inner-most error would be the most useful, as that's where something actually failed. Now I'm thinking about it, if you were just interested in decoding the error to display something to the enduser, you might also want to know the appID of the inner for some more context. Just in case you're expecting multiple different inners to be called, and I assume if the appID is missing then it's more of a "failed to deploy" message.

[jannotti]

Now I'm thinking about it, if you were just interested in decoding the error to display something to the enduser, you might also want to know the appID of the inner for some more context. Just in case you're expecting multiple different inners to be called, and I assume if the appID is missing then it's more of a "failed to deploy" message.

Maybe I can add that in as I concatenate the inner error. It may also be useful enough that it should go in the structured data.