Skip to content

Conversation

@itamar-starkware
Copy link
Contributor

@itamar-starkware itamar-starkware commented Nov 18, 2025

TL;DR

Made versioned_constants_overrides an optional field in both BlockBuilderConfig and StatefulTransactionValidatorConfig.

What changed?

  • Changed versioned_constants_overrides from a required field to an optional field (Option<VersionedConstantsOverrides>) in both BlockBuilderConfig and StatefulTransactionValidatorConfig
  • Updated the default implementations to use Some(VersionedConstantsOverrides::default()) to maintain backward compatibility
  • Modified VersionedConstants::get_versioned_constants() to accept an Option<VersionedConstantsOverrides> parameter
  • Updated the implementation to use the latest constants when None is provided
  • Updated all callers to handle the new optional parameter
  • Added appropriate schema entries for the optional field

@itamar-starkware itamar-starkware self-assigned this Nov 18, 2025
@github-actions
Copy link

github-actions bot commented Nov 18, 2025

Copy link
Contributor Author

itamar-starkware commented Nov 18, 2025

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 6 files reviewed, all discussions resolved (waiting on @TzahiTaub)


crates/blockifier/src/blockifier_versioned_constants.rs line 442 at r1 (raw file):

        versioned_constants_overrides: Option<VersionedConstantsOverrides>,
    ) -> Self {
        match versioned_constants_overrides {

Since both branches (some/none) end up calling Self::latest_constants().clone(), consider doing that once before. (non-blocking)

Code quote:

match versioned_constants_overrides {

@itamar-starkware
Copy link
Contributor Author

crates/blockifier/src/blockifier_versioned_constants.rs line 442 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

Since both branches (some/none) end up calling Self::latest_constants().clone(), consider doing that once before. (non-blocking)

Done

@itamar-starkware itamar-starkware force-pushed the 11-18-apollo_batcher_make_versioned_constants_override_optional branch 2 times, most recently from bc65161 to 03bd26a Compare November 24, 2025 11:34
Copy link
Contributor

@TzahiTaub TzahiTaub left a comment

Choose a reason for hiding this comment

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

@TzahiTaub reviewed 2 of 6 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

Copy link
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@Yoni-Starkware reviewed 2 of 6 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

@itamar-starkware itamar-starkware force-pushed the 11-18-apollo_batcher_make_versioned_constants_override_optional branch from 03bd26a to 4f24f7a Compare November 26, 2025 14:01
Copy link
Collaborator

@ShahakShama ShahakShama left a comment

Choose a reason for hiding this comment

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

:lgtm:

@ShahakShama reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

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.

7 participants