refactor(batcher): make versioned_constants_overrides field optional in BlockBuilderConfig

[ArniStarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[ArniStarkware]

• refactor(batcher): make versioned_constants_overrides field optional in BlockBuilderConfig #1832 👈
• main

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

Join @ArniStarkware and the rest of your teammates on Graphite

[ArniStarkware]

Replaces #1414.

[github-actions]

Artifacts upload triggered. View details here

[codecov]

Codecov Report

Attention: Patch coverage is 23.52941% with 39 lines in your changes missing coverage. Please review.

Project coverage is 55.84%. Comparing base (e3165c4) to head (8dd7366).
Report is 251 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/batcher/src/block_builder.rs	27.27%	32 Missing ⚠️
crates/blockifier/src/versioned_constants.rs	0.00%	7 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1832       +/-   ##
===========================================
+ Coverage   40.10%   55.84%   +15.73%     
===========================================
  Files          26      153      +127     
  Lines        1895    17640    +15745     
  Branches     1895    17640    +15745     
===========================================
+ Hits          760     9851     +9091     
- Misses       1100     7349     +6249     
- Partials       35      440      +405     

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

[ayeletstarkware]

Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @Yael-Starkware)

---

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

    /// Returns the latest versioned constants, applying the given overrides if provided.
    pub fn latest_with_overrides(

do you think this function will be used in other places? why not use if,else in BlockBuilderFactory

Code quote:

latest_with_overrides

[ArniStarkware]

Reviewable status: 0 of 3 files reviewed, all discussions resolved (waiting on @Yael-Starkware)

---

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

Previously, ayeletstarkware (Ayelet Zilber) wrote…

do you think this function will be used in other places? why not use if,else in BlockBuilderFactory

Yes. It seems very useful to me.
(My master plan is to squash latest_with_overrides, get_versioned_constants, latest_constants_with_overrides together - if this is the case - I want them all to be close to each other).

[ayeletstarkware]

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

[Yael-Starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware)

---

crates/batcher/src/block_builder.rs line 143 at r1 (raw file):

        dump
    }
}

lets move this to be under the BlockBuilderConfig struct

Code quote:

impl Default for BlockBuilderConfig {
    fn default() -> Self {
        Self {
            // TODO: update the default values once the actual values are known.
            chain_info: ChainInfo::default(),
            execute_config: TransactionExecutorConfig::default(),
            bouncer_config: BouncerConfig::default(),
            sequencer_address: ContractAddress::default(),
            use_kzg_da: true,
            tx_chunk_size: 100,
            versioned_constants_overrides: None,
        }
    }
}

impl SerializeConfig for BlockBuilderConfig {
    fn dump(&self) -> BTreeMap<ParamPath, SerializedParam> {
        let mut dump = append_sub_config_name(self.chain_info.dump(), "chain_info");
        dump.append(&mut append_sub_config_name(self.execute_config.dump(), "execute_config"));
        dump.append(&mut append_sub_config_name(self.bouncer_config.dump(), "bouncer_config"));
        dump.append(&mut BTreeMap::from([ser_param(
            "sequencer_address",
            &self.sequencer_address,
            "The address of the sequencer.",
            ParamPrivacyInput::Public,
        )]));
        dump.append(&mut BTreeMap::from([ser_param(
            "use_kzg_da",
            &self.use_kzg_da,
            "Indicates whether the kzg mechanism is used for data availability.",
            ParamPrivacyInput::Public,
        )]));
        dump.append(&mut BTreeMap::from([ser_param(
            "tx_chunk_size",
            &self.tx_chunk_size,
            "The size of the transaction chunk.",
            ParamPrivacyInput::Public,
        )]));
        dump.append(&mut ser_optional_sub_config(
            &self.versioned_constants_overrides,
            "versioned_constants_overrides",
        ));
        dump
    }
}

---

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

    pub fn get_versioned_constants(
        versioned_constants_overrides: VersionedConstantsOverrides,
    ) -> Self {

why nor make this function get an option directly instead of having another function to wrap it?

Code quote:

    pub fn get_versioned_constants(
        versioned_constants_overrides: VersionedConstantsOverrides,
    ) -> Self {

[ArniStarkware]

Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @ayeletstarkware and @Yael-Starkware)

---

crates/batcher/src/block_builder.rs line 143 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

lets move this to be under the BlockBuilderConfig struct

Done.

---

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

Previously, Yael-Starkware (YaelD) wrote…

why nor make this function get an option directly instead of having another function to wrap it?

This is a topic for another PR.
I claim these other instances (of get_versioned_constants) should be examined further and potentially refactored. I do not want to complicate this PR any further.
Added a TODO.

[github-actions]

Artifacts upload triggered. View details here

[Yael-Starkware]

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