stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 664 forks source link

Panic in Clarity contracts should provide a meaningful error message #3177

Open njordhov opened 2 years ago

njordhov commented 2 years ago

Panic in Clarity contracts results in a useless and uninformative (err none) reported to the caller when the contract aborts, whether triggered by unwrap-panic or a failure such as an uint subtraction underflow.

Instead, a panic should provide a useful error message so the caller can understand why the contract call was aborted.

The lack of helpful panic messages leads to confused developers, ad-hoc workarounds, and avoidance of unwrap-panic, an anti-pattern. The clarity-lang book even explicitly advises against using unwrap-panic due to the lack of a useful response:

You should ideally not use the -panic variants unless you absolutely have to, because they confer no meaningful information when they fail. A transaction will revert with a vague "runtime error" and users as well as developers are left to figure out exactly what went wrong.

See also:

jcnelson commented 2 years ago

Hey @njordhov, are you asking that the transaction receipt include something more informative than (err none)? Because if so, this isn't a breaking change, and could be done in the next release. Please elaborate?

njordhov commented 2 years ago

@jcnelson Yes, it's about making the transaction receipt more informative on Clarity panics. Minimally, it should provide an error message with the specific reason for the failure, or the triggering error for unwrap-panic. Better, a record (aka tuple) with more details including the name of the function in which the panic occurred and the location in the contract code of the failing call.

jcnelson commented 1 year ago

Temporarily assigning to @obycode. Feel free to re-assign :)

EDIT: damn fat fingers