clarinet icon indicating copy to clipboard operation
clarinet copied to clipboard

Using `try!` as variable name in `let` should not pass a clarinet check

Open FriendsFerdinand opened this issue 3 years ago • 6 comments

Description

Consider the following contract where I use try! as a variable name:

(define-public (call-this (fail bool))
  (let (
    (try! (try-me fail))
  )
    (ok true)
  )
)

(define-public (try-me (fail bool))
  (begin
    (asserts! fail (err u999))
    (ok true)
  )
)

This passes a clarinet check, but when executed on clarinet console returns a RuntimeError: Runtime Error: Runtime error while interpreting ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM.contract-7: Unchecked(NameAlreadyUsed("try!"))

If you use an asserts! instead, it will not pass clarinet check, naming a syntax error. If I write: (define-data-var try! uint u0), then it will return a Runtime error NameAlreadyUsed after running clarinet check.

Conclusion

I think running clarinet check should be able to detect this kind of error before trying to run the contract code on clarinet console/test.

FriendsFerdinand avatar Jun 20 '22 23:06 FriendsFerdinand

Thanks for reporting this @FriendsFerdinand.

I see that the type-checker does not include the built-in reserved keywords when it does its checks, but during execution, this is checked. We should be able to just add this check into ContractContext::check_name_used.

obycode avatar Jun 21 '22 20:06 obycode

I submitted a PR to fix this in clarinet, but this should also be fixed in the clarity lib in stacks-blockchain. I will open a corresponding issue there.

obycode avatar Jun 21 '22 21:06 obycode

We should also ensure that the unexpected behavior from https://github.com/stacks-network/stacks-blockchain/issues/2696 is unaffected by this change.

obycode avatar Jun 21 '22 21:06 obycode

This will be fixed at the canonical vm level, will close this issue. Feel free to reopen if you see another path forward.

lgalabru avatar Oct 06 '22 11:10 lgalabru

This change is not included in 2.1. For the time being, let's report this as an error in Clarinet, and then push to get this included in the next network fork.

obycode avatar Dec 05 '22 16:12 obycode

@hugocaillard Has this been resolved upstream?

smcclellan avatar Aug 16 '23 00:08 smcclellan