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

Feat/clarity cli costs json #2597

Merged
merged 15 commits into from
May 6, 2021
Merged

Feat/clarity cli costs json #2597

merged 15 commits into from
May 6, 2021

Conversation

jcnelson
Copy link
Member

@jcnelson jcnelson commented Apr 21, 2021

This PR addresses #2573 and implements PR #2587.

  • The clarity-cli binary will install the chain bootcode into the CLI's database, so .pox, .bns, .costs, and .cost-voting are now always available.
  • The clarity-cli binary, when instantiating the database, will take an optional --testnet flag to indicate to use the testnet settings for the chain bootcode, as well as use the testnet block limits when running code. This will be honored in subsequent commands that rely on the database.
  • The clarity-cli binary now accepts a --costs switch for check, launch, execute, eval, eval_at_chaintip, and eval_at_block. If given, the program will print out the runtime cost of the code executed.
  • The clarity-cli binary now accepts a --assets switch for launch and execute. If given, the program will print out the JSON representation of the AssetMap that was generated when the code executed.
  • The clarity-cli binary will now enforce runtime costs on check, launch, execute, eval, eval_at_chaintip, and eval_at_block. Depending on whether or not the DB was instantiated with --testnet, the mainnet or testnet block limits will be used. The eval_raw and repl directives are not affected by this; they continue to use the unlimited cost.
  • All clarity-cli directives now write a JSON representation to stdout, which should make it much easier for clarity-js-sdk and clarinet to interact it.

I've asked @hstove and @zone117x to take a look at this as well, since this affects downstream clients.

Example run:

$ clarity-cli initialize /tmp/test.db
INFO [1618970453.510913] [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"}
$ clarity-cli check ./sample-contracts/tokens.clar /tmp/test.db
{"message":"Checks passed."}
$ clarity-cli check --costs ./sample-contracts/tokens.clar /tmp/test.db
{"costs":{"read_count":1,"read_length":1,"runtime":821000,"write_count":1,"write_length":165},"message":"Checks passed."}
$ clarity-cli launch --costs SPFXTNFSQ4CGTGHZQ4M9YYZHG38XZRM40ET838XJ.tokens ./sample-contracts/tokens.clar /tmp/test.db | jq
{
  "costs": {
    "read_count": 4,
    "read_length": 406,
    "runtime": 15626000,
    "write_count": 4,
    "write_length": 1778
  },
  "events": [],
  "message": "Contract initialized!"
}
$ clarity-cli launch S1G2081040G2081040G2081040G208105NK8PE5.tokens-ft sample-contracts/tokens-ft.clar /tmp/test.db --assets | jq
{
  "assets": {
    "assets": {},
    "burns": {},
    "stx": {},
    "tokens": {
      "S1G2081040G2081040G2081040G208105NK8PE5": {
        "S1G2081040G2081040G2081040G208105NK8PE5.tokens-ft::tokens": "10300"
      }
    }
  },
  "events": [
    {
      "committed": true,
      "event_index": 0,
      "ft_mint_event": {
        "amount": "10300",
        "asset_identifier": "S1G2081040G2081040G2081040G208105NK8PE5.tokens-ft::tokens",
        "recipient": "S1G2081040G2081040G2081040G208105NK8PE5"
      },
      "txid": "0x0000000000000000000000000000000000000000000000000000000000000000",
      "type": "ft_mint_event"
    },
    {
      "committed": true,
      "event_index": 0,
      "ft_transfer_event": {
        "amount": "10000",
        "asset_identifier": "S1G2081040G2081040G2081040G208105NK8PE5.tokens-ft::tokens",
        "recipient": "SZ2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQ9H6DPR",
        "sender": "S1G2081040G2081040G2081040G208105NK8PE5"
      },
      "txid": "0x0000000000000000000000000000000000000000000000000000000000000000",
      "type": "ft_transfer_event"
    },
    {
      "committed": true,
      "event_index": 0,
      "ft_transfer_event": {
        "amount": "300",
        "asset_identifier": "S1G2081040G2081040G2081040G208105NK8PE5.tokens-ft::tokens",
        "recipient": "SM2J6ZY48GV1EZ5V2V5RB9MP66SW86PYKKQVX8X0G",
        "sender": "S1G2081040G2081040G2081040G208105NK8PE5"
      },
      "txid": "0x0000000000000000000000000000000000000000000000000000000000000000",
      "type": "ft_transfer_event"
    }
  ],
  "message": "Contract initialized!"
}

@psq
Copy link
Contributor

psq commented Apr 21, 2021

with this, I can get the oracle contract costs:

clarity-cli initialize /tmp/test.db
clarity-cli launch --costs ST31HHVBKYCYQQJ5AQ25ZHA6W2A548ZADDQ6S16GP.oracle ./contracts/oracle.clar /tmp/test.db
clarity-cli execute --costs /tmp/test2.db ST31HHVBKYCYQQJ5AQ25ZHA6W2A548ZADDQ6S16GP.oracle add-price ST31HHVBKYCYQQJ5AQ25ZHA6W2A548ZADDQ6S16GP '"coinbase"' 0x000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000607fe55800000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000ce5a2cfa00000000000000000000000000000000000000000000000000000000000000006707269636573000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000034254430000000000000000000000000000000000000000000000000000000000 0xb41dc3807934116ad43e3066fa08355daddbc9dc1b1fc667f32cae35c1a0576f8c7adf3528e6da8fb9617cc87f1bb1b16750dead79dc204bcc0484825969de1500
{"costs":{"read_count":10,"read_length":12708,"runtime":229492000,"write_count":1,"write_length":204},"events":[],"message":"Transaction executed and committed.","output":{"Bool":true}}

thank you @jcnelson ! now I can experiment and reduce cost, hopefully dramatically

src/clarity.rs Outdated Show resolved Hide resolved
src/clarity.rs Show resolved Hide resolved
src/clarity.rs Show resolved Hide resolved
src/clarity.rs Show resolved Hide resolved
src/clarity.rs Outdated Show resolved Hide resolved
src/clarity.rs Show resolved Hide resolved
src/clarity.rs Show resolved Hide resolved
src/clarity.rs Show resolved Hide resolved
src/clarity.rs Outdated Show resolved Hide resolved
src/clarity.rs Outdated Show resolved Hide resolved
src/clarity.rs Show resolved Hide resolved
@jcnelson
Copy link
Member Author

I think there's still a bug in the check error path when used without a DB:

$ clarity-cli check /tmp/invalid-var.clar 
INFO [1619163243.759567] [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"
{"error":{"analysis":{"level":"Error","message":"contract cost computation failed: Error evaluating result of cost function SP000000000000000000002Q6VF78.costs.cost_analysis_lookup_variable_depth: Arithmetic(\"log2 must be passed a positive integer\")\n Stack Trace: \nSP000000000000000000002Q6VF78.costs:cost_analysis_lookup_variable_depth\nSP000000000000000000002Q6VF78.costs:nlogn\n_native_:native_log2\n","spans":[{"end_column":21,"end_line":2,"start_column":12,"start_line":2}],"suggestion":null}},"message":"Checks failed."}

@jcnelson
Copy link
Member Author

Actually, I'm not sure this is a bug -- I think this is may be working as intended. Please see #2605 for further analysis.

Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me -- I think it should just avoid duplicating the HELIUM limit definition.

@jcnelson
Copy link
Member Author

This looks good to me -- I think it should just avoid duplicating the HELIUM limit definition.

Done

Copy link
Member

@kantai kantai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Contributor

@lgalabru lgalabru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @jcnelson!

@jcnelson jcnelson dismissed pavitthrap’s stale review May 6, 2021 18:32

I think all feedback has been addressed, and the PR has since received 2 approvals. Moving to dismiss this review so as to merge the PR (Github won't let me otherwise).

@jcnelson jcnelson merged commit 0fb9f85 into develop May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants