clarinet icon indicating copy to clipboard operation
clarinet copied to clipboard

Runtime Error: Unchecked(NameAlreadyUsed("err")

Open whoabuddy opened this issue 3 years ago • 7 comments

While testing out a new implementation of our VRF contract I ran into an error while testing, but clarinet check passes without issue. I could also reproduce the error in the console, but it's not clear to me what the issue is.

The new public function is here on this branch.

The resulting error when testing is:

Runtime error: citycoin-vrf-v2::get-save-rnd(u10) -> Runtime Error: Runtime error while interpreting ST1J4G6RR643BCG8G8SR6M2D9Z9KXT2NJDRK3FBTK.contract-22: Unchecked(NameAlreadyUsed("err"))

To reproduce, uncomment these lines in the test file, then run:

npm run clarinet:prepare && clarinet test ./tests/citycoin-vrf.test.ts

OR

To reproduce in clarinet console:

>> (contract-call? .citycoin-vrf-v2 get-rnd u10)
(err u3000)
>> (contract-call? .citycoin-vrf-v2 get-save-rnd u10)
error: Runtime Error: Runtime error while interpreting ST1HTBVD3JG9C05J7HBJTHGR0GGW7KXW28M5JS8QE.contract-22: Unchecked(NameAlreadyUsed("err"))

whoabuddy avatar Apr 15 '22 17:04 whoabuddy

It doesn't like you using err as a variable name. Using something else will work:

(define-public (get-save-rnd (block uint))
  (match (map-get? RandomUintAtBlock block)
    rnd (ok rnd)
    (match (read-rnd block)
      rnd (begin (map-set RandomUintAtBlock block rnd) (ok rnd))
      err-val (err err-val)
    ) 
  )
)

obycode avatar Apr 22 '22 19:04 obycode

But yes, clarinet check should catch that too. Thanks for reporting.

obycode avatar Apr 22 '22 19:04 obycode

@obycode I'd like to clarify that original code is 100% valid and fully acceptable by stacks-node Clarity VM. https://stacks-node-api.testnet.stacks.co/extended/v1/tx/0xfd74da232ea8e5c70459183cb5cecc21b209df6eb09e756a66286a5a77b40d7f

In my opinion Clarinet should use syntax checker that is as close to the one used in original Clarity VM as possible. Because of that, I believe that err variable should not cause any errors in Clarinet. It can be reported as warning, though.

LNow avatar Apr 22 '22 23:04 LNow

An, interesting. Thanks @LNow. I agree, clarinet check, the repl and the tests should all be the same as on chain. I'll take a look.

obycode avatar Apr 23 '22 00:04 obycode

This reminded me of another issue from @LNow here: https://github.com/stacks-network/stacks-blockchain/issues/2696

And the related comments here: https://github.com/stacks-network/stacks-blockchain/issues/3052#issuecomment-1047144664

I was under the impression this would be a reserved word, and I think it should be, but I also agree that Clarinet should match what the Clarity VM allows for consistency.

whoabuddy avatar Apr 23 '22 13:04 whoabuddy

This error is coming up again, this time it showed up in CI and now I see it locally but am not sure why it's coming up now when it worked before.

Readonly Contract call runtime error: ccd001-direct-execute::direct-execute('ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM.test-ccd002-treasury-011) -> Runtime Error: Runtime error while interpreting ST2JHG361ZXG51QTKY2NQCVBPPRRE2KZB1HR05NNC.contract-call: Unchecked(NameAlreadyUsed("err"))
  • Error in CI, with clarinet v1.4.0.
  • Tested with clarinet v1.4.1 locally and produced the same result

The test in question constructs the DAO and executes a proposal through a common function used in other tests.

The proposal is test-ccd002-treasury-011.clar and code is below:

(impl-trait .proposal-trait.proposal-trait)

(define-public (execute (sender principal))
  (begin
    (try! (contract-call? .ccd002-treasury-mia-mining delegate-stx u1000000000 'ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM))
    (try! (contract-call? .ccd002-treasury-mia-stacking delegate-stx u1000000000 'ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM))
    (try! (contract-call? .ccd002-treasury-nyc-mining delegate-stx u1000000000 'ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM))
    (try! (contract-call? .ccd002-treasury-nyc-stacking delegate-stx u1000000000 'ST1PQHQKV0RJXZFY1DGX8MNSNYVE3VGZJSRTPGZGM))
    (ok true)
  )
)

The delegate-stx function calls into pox to perform the delegation for a given treasury:

(define-public (delegate-stx (maxAmount uint) (to principal))
  (begin
    (try! (is-dao-or-extension))
    (print {
      event: "delegate-stx",
      amount: maxAmount,
      caller: contract-caller,
      delegate: to,
      sender: tx-sender
    })
    ;; MAINNET: (match (as-contract (contract-call? 'SP000000000000000000002Q6VF78.pox delegate-stx maxAmount to none none))
    (match (as-contract (contract-call? 'ST000000000000000000002AMW42H.pox delegate-stx maxAmount to none none))
      success (ok success)
      err (err (to-uint err))
    )
  )
)

The treasuries all reference the same initial file in Clarinet.toml, which simulates deploying multiple versions on mainnet.

[contracts.ccd002-treasury-mia-mining]
path = "contracts/extensions/ccd002-treasury.clar"

[contracts.ccd002-treasury-mia-stacking]
path = "contracts/extensions/ccd002-treasury.clar"

[contracts.ccd002-treasury-nyc-mining]
path = "contracts/extensions/ccd002-treasury.clar"

[contracts.ccd002-treasury-nyc-stacking]
path = "contracts/extensions/ccd002-treasury.clar"

To reproduce:

Update:

After laying it out above, I tried changing this line in the match: err (err (to-uint err)) to zerr (err (to-uint zerr)) and it's working. I may not had seen it before because these functions were originally succeeding and now I'm getting an (err u20) from pox.

Update2:

Found the source of the problem, in one proposal we already delegated the mining treasuries so doing it again via this proposal caused the (err u20) which exposed the error above by hitting the err branch of the match. We can work around it for now but documenting it's still there!

whoabuddy avatar Feb 04 '23 18:02 whoabuddy

Hey @whoabuddy Is this issue still relevant? Thx

hugoclrd avatar Mar 25 '24 13:03 hugoclrd