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

VM: analysis pass should consider argument count when deploying a contract #2683

Open
jcnelson opened this issue Jun 1, 2021 · 3 comments
Open

Comments

@jcnelson
Copy link
Member

jcnelson commented Jun 1, 2021

It was discovered in #2677 that the Clarity VM does not verify in the analysis pass that a function call contains the correct number of arguments. This is a consensus bug, so it cannot be fixed without a hard fork. We'll have to address it in Stacks 2.1, and it'll have to be gated. Specifically:

  • Contracts deployed after the epoch switch to Stacks 2.1 should fail analysis if they call a function with the wrong number of arguments
  • Contracts deployed before the epoch switch should continue to pass analysis, but if they would fail analysis in 2.1 due to this bug, they should be marked as such
  • (at-block ..) calls to Stacks 2.0 contracts from Stacks 2.1 contracts should fail analysis if the contract was tagged as buggy per the above.
@jcnelson
Copy link
Member Author

jcnelson commented Jun 1, 2021

No remediation action is necessary. Attempts to contract-call? another contract's public function with the wrong number of arguments, or attempts to contract-call? a function that ultimately calls a function with the wrong number of arguments will both fail with UncheckedError, which invalidates the whole block.

$ cat /tmp/clarity-error.clar
(define-read-only (main-function (param1 uint) (param2 uint))
  (some u10)
)

;; here we call function using 3 arguments instead of 2
(define-read-only (first-function)
  (match (main-function u1 u2 u3)
    value (+ value u10)
    u0
  )
)

;; here we call function using only 1 argument instead of 2
(define-read-only (second-function)
  (main-function u1)
)
$ clarity-cli initialize /tmp/clarity-error.db
INFO [1622578955.200364] [src/vm/functions/mod.rs:445] [main] "``... to be a completely separate network and separate block chain, yet share CPU power with Bitcoin`` - Satoshi Nakamoto"
{"message":"Database created.","network":"mainnet"}
$
$
$ # bug manifests below -- this should have failed
$ clarity-cli launch SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error /tmp/clarity-error.clar /tmp/clarity-error.db
{"events":[],"message":"Contract initialized!"}
$
$ cat /tmp/clarity-error-cc.clar
(begin
    (contract-call? 'SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error first-function)
)
$ clarity-cli check /tmp/clarity-error-cc.clar /tmp/clarity-error.db
{"error":{"initialization":"Unchecked(IncorrectArgumentCount(2, 3))"}}
$ clarity-cli launch SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error-cc-001 /tmp/clarity-error-cc.clar /tmp/clarity-error.db
{"error":{"initialization":"Unchecked(IncorrectArgumentCount(2, 3))"}}
$ clarity-cli launch SP1KWKM131TW5NAMX6X59A1ZZFVT731RYC9BD35X6.clarity-error-cc-001 /tmp/clarity-error-cc.clar /tmp/clarity-error.db
{"error":{"initialization":"Unchecked(IncorrectArgumentCount(2, 3))"}}

@jcnelson
Copy link
Member Author

Transactions with the wrong number of arguments will be mined in 2.1 because we convert all analysis errors to runtime errors.

@jcnelson
Copy link
Member Author

Assigning to @obycode so this issue has an owner. Please feel free to re-assign.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Status: 🆕 New
Development

No branches or pull requests

3 participants