committer: generate random state diff test util.

[yoavGrs]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2025-08-21T09:00:31Z
• Started at 2025-08-21T13:10:52Z
• Started at 2025-08-24T08:48:27Z
• Started at 2025-09-15T08:35:31Z
• Started at 2025-09-16T13:24:33Z
• Started at 2025-09-16T13:42:14Z
• Started at 2025-09-16T14:39:23Z
• Started at 2025-09-17T09:52:29Z
• Started at 2025-09-17T11:12:10Z
• Started at 2025-09-17T11:15:33Z
• Started at 2025-09-17T11:25:34Z
• Started at 2025-09-17T11:37:06Z

[github-actions]

Benchmark movements: No major performance changes detected.

[TzahiTaub]

@TzahiTaub reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @yoavGrs)

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 15 at r1 (raw file):

pub mod state_diff_generator_test;

pub(crate) const GENERATED_CONTRACT_ADDRESS: u32 = 500_u32;

Suggestion:

CONTRACT_ADDRESS:

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 49 at r1 (raw file):

    random_bytes[0] &= mask;

    hex::encode(random_bytes)

I didn't undetsrand the mask computation. I understand it tries to be sort of a general computation for the first byte depending on how much bits we want in total (an assuming only the first byte should be masked). Can't you just write 7 or 0b111? :)
In any case, I see we have Felt::random in the CLI crate. Maybe we should move it.

Code quote:

fn generate_random_hex_251_bits(rng: &mut StdRng) -> String {
    const N_BYTES: usize = 32; // 251 bits / 8 bits per byte
    let mut random_bytes = [0u8; N_BYTES];
    rng.fill(&mut random_bytes);

    // Mask the extra bits if n_bits is not a multiple of 8
    let n_bits = 251;
    let mask = (1u8 << (n_bits % 8)) - 1;
    random_bytes[0] &= mask;

    hex::encode(random_bytes)

---

crates/starknet_committer/src/block_committer/state_diff_generator_test.rs line 20 at r1 (raw file):

        state_diff.storage_updates.get(&ContractAddress::from(GENERATED_CONTRACT_ADDRESS)).unwrap();
    assert_eq!(contract.len(), N_STORAGE_UPDATES);
}

Can you make a loop of some iterations (100 if it's fast) and assert that the squashed state diff is of len of approx 100*1000? To see that they are random on the entire domain.

Code quote:

    let contract =
        state_diff.storage_updates.get(&ContractAddress::from(GENERATED_CONTRACT_ADDRESS)).unwrap();
    assert_eq!(contract.len(), N_STORAGE_UPDATES);
}

[TzahiTaub]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @yoavGrs)

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 20 at r1 (raw file):

pub fn generate_random_state_diff(rng: &mut StdRng) -> StateDiff {
    let mut storage_updates = HashMap::new();
    let mut contract_updates = HashMap::new();

Suggestion:

 let mut contract_updates = HashMap::with_capacity(N_STORAGE_UPDATES);

[yoavGrs]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @TzahiTaub)

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 49 at r1 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

I didn't undetsrand the mask computation. I understand it tries to be sort of a general computation for the first byte depending on how much bits we want in total (an assuming only the first byte should be masked). Can't you just write 7 or 0b111? :)
In any case, I see we have Felt::random in the CLI crate. Maybe we should move it.

Removed!

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 15 at r1 (raw file):

pub mod state_diff_generator_test;

pub(crate) const GENERATED_CONTRACT_ADDRESS: u32 = 500_u32;

Done.

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 20 at r1 (raw file):

pub fn generate_random_state_diff(rng: &mut StdRng) -> StateDiff {
    let mut storage_updates = HashMap::new();
    let mut contract_updates = HashMap::new();

Done.

[yoavGrs]

• starknet_committer: test mdbx storage #9276
• starknet_committer: add mdbx storage #8917
• committer: add cli for committer benchmark #8751
• committer: generate random state diff test util. #8738 👈 (View in Graphite)
• starknet_committer: move random structs from cli #8765
• committer_storage_benchmark

This stack of pull requests is managed by Graphite. Learn more about stacking.

[TzahiTaub]

@TzahiTaub reviewed 1 of 6 files at r2, all commit messages.
Reviewable status: 1 of 6 files reviewed, all discussions resolved (waiting on @yoavGrs)

[TzahiTaub]

@TzahiTaub reviewed 1 of 6 files at r2, 2 of 2 files at r4, all commit messages.
Reviewable status: 4 of 6 files reviewed, 1 unresolved discussion (waiting on @yoavGrs)

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 34 at r4 (raw file):

}

pub(crate) fn generate_random_storage_entry(

Can be private and still be used in the test module (though I don't mind it to be of crate level)

Code quote:

pub(crate)

[TzahiTaub]

@TzahiTaub reviewed 5 of 426 files at r5, 421 of 421 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yoavGrs)

[TzahiTaub]

@TzahiTaub reviewed 420 of 426 files at r7.
Reviewable status: 420 of 426 files reviewed, 1 unresolved discussion (waiting on @yoavGrs)

[TzahiTaub]

@TzahiTaub reviewed 6 of 426 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)

[TzahiTaub]

@TzahiTaub reviewed 2 of 6 files at r8, all commit messages.
Reviewable status: 422 of 426 files reviewed, all discussions resolved (waiting on @yoavGrs)

[dorimedini-starkware]

@dorimedini-starkware reviewed 418 of 426 files at r7, 3 of 6 files at r8, 2 of 4 files at r9, 3 of 3 files at r10, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yoavGrs)

---

crates/starknet_committer/src/block_committer/random_structs.rs line 196 at r10 (raw file):

            None => upper_bound,
            Some(caller_max) => min(upper_bound, caller_max),
        };

non-blocking

Suggestion:

        let max_patricia_key = min(upper_bound, max.unwrap_or(upper_bound));

---

crates/starknet_committer/src/block_committer/random_structs.rs line 197 at r10 (raw file):

            Some(caller_max) => min(upper_bound, caller_max),
        };
        PatriciaKey::try_from(Felt::random(rng, Some(max_patricia_key))).unwrap()

non-blocking

Suggestion:

Self::try_from(Felt::random(rng, Some(max_patricia_key))).unwrap()

---

crates/starknet_committer/src/block_committer/state_diff_generator_test.rs line 37 at r10 (raw file):

    }
    assert!(storage_updates.len() >= (n_iterations * 99 / 100), "Key distribution is limited");
}

maybe this works? I think fixtures are run in test scope, not module scope, so you should get a recreated seeded randomness each test

Suggestion:

#[fixture]
fn rng() -> StdRng {
    let seed = 42_u64; // Constant seed for reproducibility.
    StdRng::seed_from_u64(seed)    
}

#[rstest]
fn generate_random_state_diff_test(rng: StdRng) {
    let state_diff = generate_random_state_diff(&mut rng);
    let contract =
        state_diff.storage_updates.get(&ContractAddress::from(CONTRACT_ADDRESS)).unwrap();
    assert_eq!(contract.len(), N_STORAGE_UPDATES);
}

#[rstest]
fn key_distribution_test(rng: StdRng) {
    let n_iterations = N_STORAGE_UPDATES * 100;
    let mut storage_updates = HashMap::with_capacity(n_iterations);
    for _ in 0..n_iterations {
        let (key, value) = generate_random_storage_entry(&mut rng);
        storage_updates.insert(key, value);
    }
    assert!(storage_updates.len() >= (n_iterations * 99 / 100), "Key distribution is limited");
}

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 14 at r10 (raw file):

pub mod state_diff_generator_test;

pub(crate) const CONTRACT_ADDRESS: u32 = 500_u32;

so it's easier to search for, and so it doesn't come up in other searches

Suggestion:

const RANDOM_STATE_DIFF_CONTRACT_ADDRESS: u32 = 500_u32;

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 17 at r10 (raw file):

pub(crate) const N_STORAGE_UPDATES: usize = 1000_usize;

pub fn generate_random_state_diff(rng: &mut StdRng) -> StateDiff {

no need to enforce specific RNG type, right?

Suggestion:

pub fn generate_random_state_diff<R: Rng>(rng: &mut R) -> StateDiff {

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 31 at r10 (raw file):

        class_hash_to_compiled_class_hash: HashMap::new(),
        storage_updates,
    }

this doesn't work?

Suggestion:

    StateDiff {
        storage_updates,
        ..Default::default()
    }

---

crates/starknet_committer/src/block_committer.rs line 5 at r10 (raw file):

pub mod input;
#[cfg(any(feature = "testing", test))]
pub mod random_structs;

why wasn't this module gated before...? by mistake? no business logic uses it, right?

Code quote:

#[cfg(any(feature = "testing", test))]
pub mod random_structs;

---

crates/starknet_committer/Cargo.toml line 39 at r10 (raw file):

[features]
testing = ["starknet_patricia/testing"]

move to above [dependencies]?

Code quote:

[features]
testing = ["starknet_patricia/testing"]

[TzahiTaub]

@TzahiTaub reviewed 4 of 4 files at r9.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @yoavGrs)

---

crates/starknet_committer/Cargo.toml line 39 at r10 (raw file):

Previously, dorimedini-starkware wrote…

move to above [dependencies]?

I think it's better to have it imported only when using the testing feature, we shouldn't use it in prod builds.

---

crates/starknet_committer/src/block_committer.rs line 5 at r10 (raw file):

Previously, dorimedini-starkware wrote…

why wasn't this module gated before...? by mistake? no business logic uses it, right?

Don't know, I think this should be done on main as well.

---

crates/starknet_committer/src/block_committer/random_structs.rs line 196 at r10 (raw file):

Previously, dorimedini-starkware wrote…

non-blocking

Apparently the random of felt ignores the max param it gets, so it doesn't matter too much :)

---

crates/starknet_committer/src/block_committer/state_diff_generator_test.rs line 37 at r10 (raw file):

Previously, dorimedini-starkware wrote…

maybe this works? I think fixtures are run in test scope, not module scope, so you should get a recreated seeded randomness each test

Correct.

[TzahiTaub]

Reviewable status: 423 of 426 files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 17 at r10 (raw file):

Previously, dorimedini-starkware wrote…

no need to enforce specific RNG type, right?

No, but what is the gain from making this generic?

[TzahiTaub]

Reviewable status: 423 of 426 files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)

---

crates/starknet_committer/src/block_committer/state_diff_generator.rs line 17 at r10 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

No, but what is the gain from making this generic?

OK, actually replaced the RNG as the previous was crypto safe, and the current should be faster

[dorimedini-starkware]

@dorimedini-starkware reviewed 1 of 3 files at r11, 2 of 2 files at r13, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)

---

crates/starknet_committer/Cargo.toml line 39 at r10 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

I think it's better to have it imported only when using the testing feature, we shouldn't use it in prod builds.

yes yes, I just mean: move the [features] block up in this file

---

crates/starknet_committer/src/block_committer/random_structs.rs line 196 at r10 (raw file):

Previously, TzahiTaub (Tzahi) wrote…

Apparently the random of felt ignores the max param it gets, so it doesn't matter too much :)

hmmmmm damn... why??

[TzahiTaub]

Reviewable status: 423 of 426 files reviewed, all discussions resolved (waiting on @dorimedini-starkware)

---

crates/starknet_committer/Cargo.toml line 39 at r10 (raw file):

Previously, dorimedini-starkware wrote…

yes yes, I just mean: move the [features] block up in this file

Oh

---

crates/starknet_committer/src/block_committer/random_structs.rs line 196 at r10 (raw file):

Previously, dorimedini-starkware wrote…

hmmmmm damn... why??

Cause it would be too obvious to have it the other way around? I'll open a PR on main...

[TzahiTaub]

@TzahiTaub reviewed 1 of 3 files at r10, 1 of 3 files at r11, 3 of 3 files at r14, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)