Skip to content

starknet_os: os resources test - remove all fee transfer syscalls#14137

Open
dorimedini-starkware wants to merge 1 commit into
graphite-base/14137from
05-23-starknet_os_os_resources_test_-_remove_all_fee_transfer_syscalls
Open

starknet_os: os resources test - remove all fee transfer syscalls#14137
dorimedini-starkware wants to merge 1 commit into
graphite-base/14137from
05-23-starknet_os_os_resources_test_-_remove_all_fee_transfer_syscalls

Conversation

@dorimedini-starkware
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator Author

dorimedini-starkware commented May 24, 2026

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

@dorimedini-starkware dorimedini-starkware self-assigned this May 24, 2026
@dorimedini-starkware dorimedini-starkware marked this pull request as ready for review May 24, 2026 07:03
@cursor
Copy link
Copy Markdown

cursor Bot commented May 24, 2026

PR Summary

Low Risk
Test-only changes to syscall trace parsing and assertions; no production fee or OS resource logic is modified.

Overview
OS resources regression testing now strips the full fee-transfer syscall tail (10 selectors: execution info, storage reads/writes, emit event) instead of only dropping a single trailing EmitEvent.

A pinned FEE_TRANSFER_SYSCALLS list documents that tail, and a new test_fee_transfer_syscalls runs a no-op __execute__ invoke so every traced syscall should come from fee transfer only. The regression test **split_at**s those syscalls off before measuring __execute__ costs, asserts the tail matches the constant, and fails if any measurable syscalls remain after the loop.

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

Comment thread crates/starknet_os_flow_tests/src/os_resources_test.rs
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from 00a30cb to 6b8d3d4 Compare May 24, 2026 20:08
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_remove_all_fee_transfer_syscalls branch from cd7ac82 to 72d6d62 Compare May 24, 2026 20:08
Copy link
Copy Markdown

@cursor cursor Bot left a comment

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 72d6d62. Configure here.

Comment thread crates/starknet_os_flow_tests/src/os_resources_test.rs
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_remove_all_fee_transfer_syscalls branch from 72d6d62 to ba9b395 Compare May 25, 2026 09:48
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_emit_event branch from 6b8d3d4 to 755576b Compare May 25, 2026 09:48
@dorimedini-starkware dorimedini-starkware changed the base branch from 05-23-starknet_os_os_resources_test_-_add_emit_event to graphite-base/14137 May 25, 2026 15:37
Copy link
Copy Markdown
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

@Yoni-Starkware made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on dorimedini-starkware).


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

const SYSCALLS_WITH_LINEAR_FACTOR: [Selector; 2] = [Selector::Deploy, Selector::MetaTxV0];

/// Expected syscalls in the fee transfer call. Should be removed from the list of syscalls during

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

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