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

Create list of high-value error messages to improve #849

Closed
nicc opened this issue Apr 11, 2023 · 11 comments
Closed

Create list of high-value error messages to improve #849

nicc opened this issue Apr 11, 2023 · 11 comments
Assignees
Labels
error-messages Issues about making error messages better product-eng For tracking our team's issues

Comments

@nicc
Copy link
Collaborator

nicc commented Apr 11, 2023

We are intent on improving our error messages in general. There are some initivates that unblock this. In preparation for these improvement, we'd like a list of the most impactful error messages to focus on first.

Consider all errors (protocol, crypto, snarkyjs). Timeboxed to an estimate of 5. No more than 20 items.

Preemptive candidates:

  • Out of memory
  • Circuit size

Resources for when we are ready to improve the error messages:

@nicc nicc added error-messages Issues about making error messages better product-eng For tracking our team's issues labels Apr 11, 2023
@nicc nicc changed the title Create list of highest-value error messages to improve Create list of high-value error messages to improve Apr 11, 2023
@barriebyron
Copy link
Contributor

@ymekuria shared https://github.com/MinaProtocol/docs/blob/main/pages/en/node-operators/transaction-failures.mdx that has been floating around, not published in the docs as of April 11, 2023

@nicc
Copy link
Collaborator Author

nicc commented Apr 12, 2023

Another candidate:

Account_action_state_precondition_unsatisfied error during Reducer.reduce usually when you reduce the wrong set of actions (because you hash the wrong actions and the target hash doesn't match the committed on-chain hash anymore)

@nicc
Copy link
Collaborator Author

nicc commented Apr 12, 2023

IRI: @nicc

@mitschabaude
Copy link
Member

mitschabaude commented Apr 12, 2023

Here's a truly bad one: #808
If you forget to add tx.prove() when doing a zkApp call, the error is just:

Assert_failure src/lib/transaction_logic/zkapp_command_logic.ml:1266:17

This is because the authorizationKind is implicitly set to "proof" but then the actual authorization doesn't match that (missing proof).
(thrown from inside the transaction logic, called by LocalBlockchain)

@mitschabaude
Copy link
Member

mitschabaude commented Apr 12, 2023

Basically, every error thrown from inside snarky is bad, because of the long and noisy stack trace. @mrmr1993 had some good ideas to address this.

Here's an example (cut off) for when a constraint fails inside the prover (for example, during tx.prove()):

image

constraint-unsatisfied.txt

@Kirol54
Copy link

Kirol54 commented Apr 12, 2023

I'm assuming it's coming from graphql but it's not a really helpful error message regardless

image

@rpanic
Copy link
Contributor

rpanic commented Apr 13, 2023

I guess Can't evaluate prover code outside an as_prover block can be improved

@mitschabaude
Copy link
Member

'Permutation failed' for when you create a different circuit every time because you use a constant instead of a variable

@nicc
Copy link
Collaborator Author

nicc commented Apr 25, 2023

@nicc to review / create new issues

@nicc nicc self-assigned this Apr 25, 2023
@nicc
Copy link
Collaborator Author

nicc commented May 8, 2023

All improvements identified here have been reflected as new issues with the error-messages label. Closing this issue. Thank you, everyone!

@nicc nicc closed this as completed May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-messages Issues about making error messages better product-eng For tracking our team's issues
Projects
None yet
Development

No branches or pull requests

6 participants