Skip to content

Conversation

@orizi
Copy link
Collaborator

@orizi orizi commented Jan 17, 2026

Summary

Refactored the gas wallet mechanism in the Sierra-to-CASM compiler to improve error handling and clarify gas cost semantics. The changes include:

  1. Boxed the GasWalletError to reduce memory usage
  2. Renamed gas_change to gas_cost to better reflect its purpose
  3. Simplified the gas wallet update logic by using sub_collection instead of add_collection
  4. Improved error messages for out-of-gas conditions
  5. Fixed the sign convention for gas costs (positive values now represent costs)

Type of change

Please check one:

  • Bug fix (fixes incorrect behavior)
  • New feature
  • Performance improvement
  • Documentation change with concrete technical impact
  • Style, wording, formatting, or typo-only change

Why is this change needed?

The previous implementation had inconsistent semantics around gas costs, where negative values were used for costs and positive values for acquisitions. This made the code harder to understand and maintain. Additionally, the error handling for out-of-gas conditions was verbose and could be improved.


What was the behavior or documentation before?

Before, the gas wallet used add_collection with sign-flipped values, and error messages were less clear about the exact state that caused the out-of-gas condition.


What is the behavior or documentation after?

Now, the gas wallet directly uses sub_collection with costs represented as positive values, making the code more intuitive. Error messages are more precise about what caused the out-of-gas condition, and memory usage is reduced by boxing the error type.

@reviewable-StarkWare
Copy link

This change is Reviewable

@orizi orizi marked this pull request as ready for review January 17, 2026 08:12
Copy link
Collaborator

@TomerStarkware TomerStarkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@TomerStarkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @eytan-starkware).

@orizi orizi changed the base branch from orizi/01-15-performance_sierra-to-casm_improved_stack-tracking_algorithm to graphite-base/9492 January 20, 2026 10:03
…let update.

Change gas handling from adding a negated "change" to subtracting a
positive "cost", eliminating the intermediate CostTokenMap allocation
that was created per-branch via `.iter().map(negate).collect()`.

Also take ownership in GasWallet::update to avoid cloning the wallet.

- Rename gas_change -> gas_cost to reflect new semantics
- Use sub_collection instead of add_collection with negated values
- Box GasWalletError in AnnotationError instead of boxing cost field

SIERRA_UPDATE_PATCH_CHANGE_TAG=No interface changes.
@orizi orizi force-pushed the orizi/01-16-performance_sierra-to-casm_avoid_intermediate_allocation_in_gas_wallet_update branch from 078fbc3 to 3250cc3 Compare January 20, 2026 10:19
@orizi orizi force-pushed the graphite-base/9492 branch from d54e998 to 3ac037b Compare January 20, 2026 10:19
@orizi orizi changed the base branch from graphite-base/9492 to main January 20, 2026 10:20
@orizi orizi enabled auto-merge January 20, 2026 10:21
@orizi orizi added this pull request to the merge queue Jan 20, 2026
Merged via the queue into main with commit 27f9d1a Jan 20, 2026
108 checks passed
@orizi orizi deleted the orizi/01-16-performance_sierra-to-casm_avoid_intermediate_allocation_in_gas_wallet_update branch January 20, 2026 12:51
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.

4 participants