Skip to content

starknet_os: define sha256 batch resources constants#14140

Merged
dorimedini-starkware merged 1 commit into
mainfrom
05-23-starknet_os_define_sha256_batch_resources_constants
Jun 11, 2026
Merged

starknet_os: define sha256 batch resources constants#14140
dorimedini-starkware merged 1 commit into
mainfrom
05-23-starknet_os_define_sha256_batch_resources_constants

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

Low Risk
Changes are test-only constants and a new resource regression test; no production OS or fee logic is modified in this diff.

Overview
Adds documented Cairo execution resource budgets for the SHA-256 batch phase in finalize_sha256, split into per-round (linear) and fixed (constant) ExecutionResources values exposed as SHA256_BATCH_RESOURCES_LINEAR and SHA256_BATCH_RESOURCES_CONSTANT on test_utils.

A new integration test runs the OS program entrypoint finalize_sha256 via the existing Cairo test harness, samples resources at two block counts (one extra batching round apart), algebraically separates linear vs constant cost, and asserts both match the new constants.

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

@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from 062b998 to 09cbdec Compare June 1, 2026 08:37
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_keccak branch from 4417771 to f287752 Compare June 1, 2026 10:24
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from 09cbdec to c5b45e5 Compare June 1, 2026 10:24

@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 2 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on dorimedini-starkware and Yoni-Starkware).


crates/starknet_os/src/resource_utils_test.rs line 44 at r1 (raw file):

        let len = input.len();
        let message: &[u32; 16] = input[len - 24..len - 8].try_into().unwrap();
        let state: &[u32; 8] = input[len - 8..].try_into().unwrap();

Try to avoid the indexing

Suggestion:

        let message: [u32; 16] = rng.gen();
        let state: [u32; 8] = rng.gen();
        input.extend_from_slice(&message.into());
        input.extend_from_slice(&state.into());

crates/starknet_os/src/resource_utils_test.rs line 100 at r1 (raw file):

#[test]
fn test_finalize_sha256() {
    // Sha256 batching resources has a linear factor and a constant factor. Sample the execution at

Suggestion:

has a linear factor by the number of rounds 

crates/starknet_os/src/resource_utils_test.rs line 102 at r1 (raw file):

    // Sha256 batching resources has a linear factor and a constant factor. Sample the execution at
    // two points to compute both factors.
    let block_to_round = 7_usize;

From where this number come from?

Suggestion:

let blocks_in_round = 7_usize;

crates/starknet_os/src/test_utils.rs line 22 at r1 (raw file):

// Resources consumed by the SHA-256 batch phase, separated into linear and constant factors.
pub static SHA256_BATCH_RESOURCES_LINEAR: LazyLock<ExecutionResources> =

Why don't we need it in the VC?

@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from c5b45e5 to a1487a4 Compare June 2, 2026 10:21

@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 made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on dorimedini-starkware and Yoni-Starkware).


crates/starknet_os/src/test_utils.rs line 22 at r1 (raw file):

Previously, yoavGrs wrote…

Why don't we need it in the VC?

OK, these are the resources of a round.
Reflect it in the constant name / the comment.

@dorimedini-starkware dorimedini-starkware changed the base branch from 05-23-starknet_os_os_resources_test_-_add_keccak to graphite-base/14140 June 3, 2026 07:17
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from a1487a4 to db6000c Compare June 3, 2026 07:51
@dorimedini-starkware dorimedini-starkware changed the base branch from graphite-base/14140 to 05-23-starknet_os_os_resources_test_-_add_keccak June 3, 2026 07:51

@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 4 comments.
Reviewable status: 1 of 2 files reviewed, 4 unresolved discussions (waiting on yoavGrs and Yoni-Starkware).


crates/starknet_os/src/resource_utils_test.rs line 44 at r1 (raw file):

Previously, yoavGrs wrote…

Try to avoid the indexing

Done.


crates/starknet_os/src/resource_utils_test.rs line 102 at r1 (raw file):

Previously, yoavGrs wrote…

From where this number come from?

this is called BATCH_SIZE in the cairo common library implementing SHA


crates/starknet_os/src/test_utils.rs line 22 at r1 (raw file):

Previously, yoavGrs wrote…

OK, these are the resources of a round.
Reflect it in the constant name / the comment.

the resources of a batch, I think, is the terminology; hence BATCH_RESOURCES


crates/starknet_os/src/resource_utils_test.rs line 100 at r1 (raw file):

#[test]
fn test_finalize_sha256() {
    // Sha256 batching resources has a linear factor and a constant factor. Sample the execution at

Done.

@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 1 file and all commit messages, and resolved 4 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Yoni-Starkware).

@dorimedini-starkware dorimedini-starkware changed the base branch from 05-23-starknet_os_os_resources_test_-_add_keccak to graphite-base/14140 June 7, 2026 15:50
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from db6000c to 2a07010 Compare June 7, 2026 15:52
@dorimedini-starkware dorimedini-starkware changed the base branch from graphite-base/14140 to 05-23-starknet_os_os_resources_test_-_add_keccak June 7, 2026 15:52
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_keccak branch from 8b873b5 to 636be17 Compare June 7, 2026 16:01
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from 2a07010 to 335dd5f Compare June 7, 2026 16:01
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_os_resources_test_-_add_keccak branch from 636be17 to 8ba4b2d Compare June 11, 2026 13:04
@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from 335dd5f to b1625f5 Compare June 11, 2026 13:04
@dorimedini-starkware dorimedini-starkware changed the base branch from 05-23-starknet_os_os_resources_test_-_add_keccak to main June 11, 2026 15:07
@github-actions

Copy link
Copy Markdown

Artifacts upload workflows:

@dorimedini-starkware dorimedini-starkware force-pushed the 05-23-starknet_os_define_sha256_batch_resources_constants branch from b1625f5 to 4d46e0c Compare June 11, 2026 15:11
@dorimedini-starkware dorimedini-starkware added this pull request to the merge queue Jun 11, 2026
Merged via the queue into main with commit 5e40c33 Jun 11, 2026
21 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.

3 participants