Skip to content

starknet_os_flow_tests: initial commit of os_resources module#14091

Closed
dorimedini-starkware wants to merge 1 commit into
05-19-starknet_os_blockifier_make_some_vc_fields_publicfrom
05-19-starknet_os_flow_tests_initial_commit_of_os_resources_module
Closed

starknet_os_flow_tests: initial commit of os_resources module#14091
dorimedini-starkware wants to merge 1 commit into
05-19-starknet_os_blockifier_make_some_vc_fields_publicfrom
05-19-starknet_os_flow_tests_initial_commit_of_os_resources_module

Conversation

@dorimedini-starkware
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Collaborator Author

dorimedini-starkware commented May 19, 2026

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

@cursor
Copy link
Copy Markdown

cursor Bot commented May 19, 2026

PR Summary

Medium Risk
Test-only change, but it introduces strict resource-extraction logic and assertion-based comparisons against VersionedConstants that may be sensitive to trace format or minor VM step drift, potentially causing flaky/fragile CI failures.

Overview
Adds a new os_resources module to starknet_os_flow_tests to measure and validate Starknet OS resource costs from an OsTransactionTrace.

The module extracts per-syscall ResourcesParams (including special handling for CallContract/LibraryCall inner-call subtraction plus fixed return-step overhead), appends hardcoded costs for legacy/unexercised syscalls, and provides compare_os_resources to assert measured vs expected OsResources with step-drift margins (exact match on builtins).

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

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 74a148a. Configure here.


// Syscalls that make an inner call whose resources must be subtracted from the trace total.
pub(crate) const CALL_CONTRACT_SYSCALLS: &[DeprecatedSyscallSelector] =
&[DeprecatedSyscallSelector::CallContract, DeprecatedSyscallSelector::LibraryCall];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MetaTxV0 missing from CALL_CONTRACT_SYSCALLS loses inner-call subtraction

Medium Severity

MetaTxV0 is classified as a calling syscall (per is_calling_syscall()) and the test contract confirms it calls execute_inner_call, which pushes an entry into execute_call_info.inner_calls. However, CALL_CONTRACT_SYSCALLS only lists CallContract and LibraryCall, so MetaTxV0's inner-call VM resources are never subtracted from its raw trace resources. This inflates the extracted constant cost for MetaTxV0 by the cost of the inner call. The comparison margin may mask this today (since the inner call targets an empty function), but the measured value is nonetheless incorrect.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 74a148a. Configure here.

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.

2 participants