Skip to content

starknet_os: os resources test - add emit event#14136

Merged
dorimedini-starkware merged 1 commit into
mainfrom
05-23-starknet_os_os_resources_test_-_add_emit_event
Jun 7, 2026
Merged

starknet_os: os resources test - add emit event#14136
dorimedini-starkware merged 1 commit into
mainfrom
05-23-starknet_os_os_resources_test_-_add_emit_event

Conversation

@dorimedini-starkware

Copy link
Copy Markdown
Collaborator

No description provided.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@cursor

cursor Bot commented May 24, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes published syscall OS resource constants used for fee/gas estimation; incorrect values would affect transaction costing across the network stack.

Overview
EmitEvent is now part of the OS syscall resource regression harness instead of being skipped as unmeasurable.

The measurement contract invokes emit_event_syscall with keys [5] and data [7]. The OS flow test asserts that event and strips the trailing EmitEvent trace from fee transfer so syscall accounting stays correct. EmitEvent n_steps in versioned constants 0.14.4 is set to 47 (was 61), with the constants diff regression file updated accordingly.

execution_flavors_test folds measured EmitEvent steps into expected gas for the balance-too-low transfer scenario. Inner-call resource deduction in the OS resources test is slightly refactored so filter_unused_builtins always runs on the result.

Reviewed by Cursor Bugbot for commit f00371f. Bugbot is set up for automated code reviews on this repo. Configure here.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3ee3ec7. Configure here.

Comment thread crates/starknet_os_flow_tests/src/os_resources_test.rs

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 3ee3ec7. Configure here.

Comment thread crates/starknet_os_flow_tests/src/os_resources_test.rs
@dorimedini-starkware dorimedini-starkware changed the base branch from 05-23-starknet_os_os_resources_test_-_add_meta_tx to graphite-base/14136 June 2, 2026 10:29
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from 3ee3ec7 to 8a9154e Compare June 2, 2026 15:17

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@dorimedini-starkware made 2 comments and resolved 2 discussions.
Reviewable status: 2 of 5 files reviewed, 2 unresolved discussions (waiting on yoavGrs and Yoni-Starkware).


crates/starknet_os_flow_tests/src/os_resources_test.rs line 220 at r1 (raw file):

Previously, Yoni-Starkware (Yoni) wrote…

You can run this tx with trivial fee bounds and make this test a bit cleaner

fixed properly in the next PR. your way could also work but I don't want to break if for some reason we want to avoid trivial resource bounds


crates/starknet_os_flow_tests/src/os_resources_test.rs line 227 at r2 (raw file):

Previously, yoavGrs wrote…

IMO you can omit these lines.
The while loop ensures that if EmitEvent is a measurable syscall, it appears only once more.

this is fixed properly in the next PR

@dorimedini-starkware dorimedini-starkware changed the base branch from graphite-base/14136 to 05-23-starknet_os_os_resources_test_-_add_meta_tx June 2, 2026 15:17

@yoavGrs yoavGrs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@yoavGrs reviewed 3 files and all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on Yoni-Starkware).

@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from 8a9154e to fa3879f Compare June 3, 2026 07:17
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_meta_tx branch from 5a9f77f to b949893 Compare June 3, 2026 07:17
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from fa3879f to b72cdb7 Compare June 3, 2026 07:51
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_meta_tx branch from b949893 to 7dd4dcd Compare June 3, 2026 07:51
@dorimedini-starkware dorimedini-starkware changed the base branch from 05-23-starknet_os_os_resources_test_-_add_meta_tx to graphite-base/14136 June 7, 2026 07:40
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from b72cdb7 to 2c1bfd1 Compare June 7, 2026 12:33
@dorimedini-starkware dorimedini-starkware changed the base branch from graphite-base/14136 to main June 7, 2026 12:34
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from 2c1bfd1 to 8929f71 Compare June 7, 2026 12:48
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from 8929f71 to f00371f Compare June 7, 2026 12:52

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@dorimedini-starkware made 1 comment.
Reviewable status: 3 of 7 files reviewed, 3 unresolved discussions (waiting on dorimedini-starkware, yoavGrs, and Yoni-Starkware).


crates/blockifier/src/transaction/execution_flavors_test.rs line 910 at r5 (raw file):

            )
            + u64_from_usize(get_const_syscall_resources(SyscallSelector::EmitEvent).n_steps)
            + 3527)

3527 == 3588 - 61, where 61 is the old step cost of EmitEvent

Code quote:

3527

@Yoni-Starkware Yoni-Starkware left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:lgtm:

@Yoni-Starkware reviewed 5 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on dorimedini-starkware and yoavGrs).

@dorimedini-starkware dorimedini-starkware left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@dorimedini-starkware resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on dorimedini-starkware).

@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue Jun 7, 2026
Merged via the queue into main with commit fd9c8c3 Jun 7, 2026
41 of 44 checks passed
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