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

Run forge tests with cairo steps explicitly set in tests #3050

Open
wants to merge 1 commit into
base: szymczyk/sg-1-add-sierra-gas-tracking
Choose a base branch
from

Conversation

THenry14
Copy link
Contributor

@THenry14 THenry14 commented Mar 6, 2025

Related to #2977

This PR stack aims to add support for new resource tracking (sierra-gas) and new gas representation (GasVector triplet instead of single l1 gas value).

-- (#3049) Add sierra gas tracking
--> (This PR) Run already existing tests with CairoSteps set explicitly
-- (#3051) Add support for sierra gas in --detailed-resources
-- (#3052) Add support for sierra gas in --save-trace-data
-- (#3053) Add sierra version assertion
-- (#3054) Introduce new gas representation
-- (#3055) Add gas tests for sierra-gas tracking
-- (#3056) Update docs with sierra-gas related changes

Copy link
Member

@ksew1 ksew1 left a comment

Choose a reason for hiding this comment

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

I am not sure about this change. Many tests do not care about the traced resource, so it seems unnecessary. Additionally, it suggests that CairoSteps is essential for this test to work.

@THenry14
Copy link
Contributor Author

I am not sure about this change. Many tests do not care about the traced resource, so it seems unnecessary. Additionally, it suggests that CairoSteps is essential for this test to work.

Some do, but mainly this just lays ground for #3055, where you get different cost depending on tracked resource

@THenry14 THenry14 force-pushed the szymczyk/sg-2-run-tests-with-cairo-steps-explicitly branch from ad52dbe to 0d2a153 Compare March 10, 2025 12:34
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.

3 participants