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

Implement resource usage report #249

Closed
amanusk opened this issue Jul 22, 2023 · 10 comments · Fixed by #605
Closed

Implement resource usage report #249

amanusk opened this issue Jul 22, 2023 · 10 comments · Fixed by #605
Assignees

Comments

@amanusk
Copy link
Contributor

amanusk commented Jul 22, 2023

To what component is your bug related to?

Forge

Feature Request

Using the Blockifier, we can get the resource used for invoke transaction invocations.
Would be great to have a flag --resource-report to print out resources used (steps, built-ins etc)

@cptartur
Copy link
Member

Thank you for the request. We are definitely considering adding transactions profiling at some point.

@MaksymilianDemitraszek MaksymilianDemitraszek added this to the Forge milestone Jul 25, 2023
@cptartur cptartur moved this from Triage to Backlog in Starknet foundry Jul 25, 2023
@cptartur cptartur modified the milestones: Forge , Cheatable Starknet Jul 25, 2023
@MaksymilianDemitraszek MaksymilianDemitraszek removed this from the Cheatable Starknet milestone Jul 26, 2023
@Arcticae Arcticae moved this from Backlog to TODO in Starknet foundry Jul 27, 2023
@Arcticae
Copy link
Contributor

This should be considered as well on cairo level - which could enable users to assert gas consumed from a test.
I.e. a contract invocation would return gas consumed.

Also i think we should profile on transaction level and as well sum the transactions gas for a whole test (if a person wants to know the whole flows' cost)

@Arcticae Arcticae self-assigned this Jul 28, 2023
@Arcticae Arcticae moved this from TODO to In Progress in Starknet foundry Jul 28, 2023
@Arcticae
Copy link
Contributor

Blocked by #327

@Arcticae Arcticae moved this from In Progress to TODO in Starknet foundry Jul 28, 2023
@MaksymilianDemitraszek
Copy link
Member

We could start by implementing it for a single test case as a whole as it will be the simplest.

Later we could try to do it on a transaction level maybe something like this

let call_info = CallInfoDispatcher.get_balance()
assert(call_info.gas_usage < 100000, `above expected gas usage`)
assert(call_info.value == 5, `incorrect value`)

@enitrat
Copy link
Contributor

enitrat commented Jul 31, 2023

Wouldn't it be better to have step counts rather than gas usage?

@MaksymilianDemitraszek
Copy link
Member

@enitrat Sure but steps are only part of the picture as builtins can impose significant cost as well. Ideally we should report:

  • steps
  • builtins usage
  • estimated gas

@enitrat
Copy link
Contributor

enitrat commented Jul 31, 2023

@enitrat Sure but steps are only part of the picture as builtins can impose significant cost as well. Ideally we should report:

  • steps
  • builtins usage
  • estimated gas

yes, indeed, I just wanted to make sure we were not talking about Sierra Gas (I'm still not sure it's actually mapped to "real" execution resources)

@MaksymilianDemitraszek
Copy link
Member

#362 Should be taken under consideration

@Arcticae Arcticae moved this from TODO to Backlog in Starknet foundry Aug 7, 2023
@Arcticae Arcticae removed their assignment Aug 7, 2023
@Arcticae
Copy link
Contributor

Related: #462

@Arcticae Arcticae moved this from Backlog to In Progress in Starknet foundry Aug 22, 2023
@piotmag769
Copy link
Member

Blocked by #547

github-merge-queue bot pushed a commit that referenced this issue Sep 12, 2023
<!-- Reference any GitHub issues resolved by this PR -->

Closes #249
Closes #362 

## Introduced changes

<!-- A brief description of the changes -->

- added gas estimation

## Breaking changes

<!-- List of all breaking changes, if applicable -->

- `call_contract`, `deploy`, and `deploy_at` Cheatnet API changed

## Checklist

<!-- Make sure all of these are complete -->

- [x] Linked relevant issue
- [ ] Updated relevant documentation
- [x] Added relevant tests
- [x] Performed self-review of the code
- [ ] Added changes to `CHANGELOG.md`

---------

Co-authored-by: Arcticae <tomekgsd@gmail.com>
@github-project-automation github-project-automation bot moved this from In Progress to Done in Starknet foundry Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

7 participants