Consensys / corset

7 stars 12 forks source link

Exit Code on Error #193

Open DavePearce opened 1 month ago

DavePearce commented 1 month ago

Currently, corset does not actually fail when certain errors are identified. For example, consider this input:

(module test)
(defcolumns X)
(defconstraint test () X)

With commands such as debug, compile, wizard-iop, check an error is reported for this:

[ERROR] constraint test.test should be loobean, found [𝔽]

But, this does not generate a non-zero exit code. Therefore, we cannot take action upon an error like this.

DavePearce commented 1 month ago

This can be implemented by replacing error! logs to using bail! instead. Obviously, we need to be selective about this to ensure we're not being overly strict.

delehef commented 1 month ago

This was implemented in this way as a smooth transition toward a fully typed constraint system that would not suddenly break the whole zkEVM.

Ideally that would be converted to an error when the transition is done (i.e. no more error raised at compile time), but if the abandonment of loobs/bools goes forward as mentioned in the other issue, then this warning will be moot and should be removed.

DavePearce commented 1 month ago

This was implemented in this way as a smooth transition toward a fully typed constraint system that would not suddenly break the whole zkEVM.

I figured it had something to do with not breaking existing code. My plan is to carefully reenable stricter checks whilst making sure they don't break anything. Then see what happens :)

DavePearce commented 1 month ago

Ideally that would be converted to an error when the transition is done (i.e. no more error raised at compile time)

Here's a specific question. I didn't figure this out yesterday. Is it possible (at the end) to determine than an error! log was raised and then exit with a non-zero error code? So, basically, all the errors would be printed and then we terminate. I didn't figure that out ...

if the abandonment of loobs/bools goes forward as mentioned in the other issue

Well, one step at a time. I don't know when that is likely to happen given the current priorities. I am still working hard on trying to get the go port operational, but also fixing stuff as it arises here. So, I think this feature (removing conditioning) is a low priority for now.

delehef commented 1 month ago

Is it possible (at the end) to determine than an error! log was raised and then exit with a non-zero error code? So, basically, all the errors would be printed and then we terminate. I didn't figure that out ...

As you mentioned above, the error! only cares about writing the log message, and an early return makes the error aborting. As far as I know, there is no automated way to do both, safe for writing a custom function.