chore(blockifier, native_blockifier): update versioned constants

[amosStarkware]

This change is 

[codecov]

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Project coverage is 76.43%. Comparing base (ddc8f51) to head (0b13111).
Report is 1 commits behind head on main-v0.13.2.

Files	Patch %	Lines
crates/native_blockifier/src/py_objects.rs	0.00%	4 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##           main-v0.13.2     #559      +/-   ##
================================================
+ Coverage         76.38%   76.43%   +0.04%     
================================================
  Files               316      316              
  Lines             34282    34263      -19     
  Branches          34282    34263      -19     
================================================
+ Hits              26188    26190       +2     
+ Misses             5817     5797      -20     
+ Partials           2277     2276       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[amosStarkware]

+reviewer:@dorimedini-starkware +reviewer:@Yonatan-Starkware

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @dorimedini-starkware and @Yonatan-Starkware)

[amosStarkware]

-reviewer:@Yonatan-Starkware +reviewer:@Yoni-Starkware

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @dorimedini-starkware and @Yoni-Starkware)

[dorimedini-starkware]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @amosStarkware and @Yoni-Starkware)

---

crates/blockifier/src/versioned_constants.rs line 746 at r1 (raw file):

    pub max_recursion_depth: usize,
    pub invoke_tx_max_n_steps: Option<u32>,
}

why is only one field optional here?

I would expect either:

1. make all three optional (and override only what is Some, as you do above)
2. make none optional, and force the caller to override all three (if the caller needs only one override, the caller can explicitly put the defaults in the other two fields)

I personally prefer (2)

Code quote:

pub struct VersionedConstantsOverrides {
    pub validate_max_n_steps: u32,
    pub max_recursion_depth: usize,
    pub invoke_tx_max_n_steps: Option<u32>,
}

---

crates/blockifier/src/versioned_constants_test.rs line 97 at r1 (raw file):

/// Assert versioned constants overrides are used when provided.
#[test]
fn test_versioned_constants_overrides() {

parametrize all three fields with #[values] in this test? or is it redundant?

Code quote:

#[test]
fn test_versioned_constants_overrides() {

---

crates/native_blockifier/src/py_objects.rs line 53 at r1 (raw file):

    pub max_recursion_depth: usize,
    pub invoke_tx_max_n_steps: Option<u32>,
}

see above: I think either all optional or none optional

Code quote:

pub struct PyVersionedConstantsOverrides {
    pub validate_max_n_steps: u32,
    pub max_recursion_depth: usize,
    pub invoke_tx_max_n_steps: Option<u32>,
}

[amosStarkware]

Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @Yoni-Starkware)

---

crates/blockifier/src/versioned_constants.rs line 746 at r1 (raw file):

Previously, dorimedini-starkware wrote…

why is only one field optional here?

I would expect either:

1. make all three optional (and override only what is Some, as you do above)
2. make none optional, and force the caller to override all three (if the caller needs only one override, the caller can explicitly put the defaults in the other two fields)

I personally prefer (2)

Done.

---

crates/blockifier/src/versioned_constants_test.rs line 97 at r1 (raw file):

Previously, dorimedini-starkware wrote…

parametrize all three fields with #[values] in this test? or is it redundant?

I added the other values to the test - no need for parameterising IMO

---

crates/native_blockifier/src/py_objects.rs line 53 at r1 (raw file):

Previously, dorimedini-starkware wrote…

see above: I think either all optional or none optional

Done.

[dorimedini-starkware]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yoni-Starkware)

[dorimedini-starkware]

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Yoni-Starkware)