Skip to content

Conversation

@itamar-starkware
Copy link
Contributor

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

Make versioned_constants_overrides optional in BlockBuilderConfig.

What changed?

  • Changed versioned_constants_overrides field in BlockBuilderConfig to be Option<VersionedConstantsOverrides> instead of VersionedConstantsOverrides
  • Updated the default implementation to wrap the default value in Some()
  • Modified the dump() method to handle the case when the value is None
  • Updated the call to VersionedConstants::get_versioned_constants() to pass the option directly instead of wrapping it in Some()

Why make this change?

This change allows for more flexibility in configuration by making the versioned constants overrides optional. It maintains backward compatibility by preserving the default behavior while allowing users to explicitly opt out of providing overrides if desired.

@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.

:lgtm:

@Itay-Tsabary-Starkware reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)

@itamar-starkware itamar-starkware force-pushed the 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_batcher branch from 74b46eb to 21939ad Compare November 24, 2025 06:47
@itamar-starkware itamar-starkware force-pushed the 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_gateway branch 2 times, most recently from 23c9984 to e981a1d Compare November 24, 2025 11:34
@itamar-starkware itamar-starkware force-pushed the 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_batcher branch from 21939ad to c8e15be 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 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itamar-starkware)


crates/apollo_batcher_config/src/config.rs line 41 at r2 (raw file):

            proposer_idle_detection_delay_millis: Duration::from_millis(2000),
            // Preserve behavior by defaulting to Some(default overrides).
            versioned_constants_overrides: Some(VersionedConstantsOverrides::default()),

As in the GW, shiuld be None by default.

Code quote:

 versioned_constants_overrides: Some(VersionedConstantsOverrides::default()),

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.

@Yoni-Starkware reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itamar-starkware and @TzahiTaub)


crates/apollo_batcher_config/src/config.rs line 41 at r2 (raw file):

I think that this should either be None (and the default impl should be removed)

+1

@itamar-starkware itamar-starkware force-pushed the 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_batcher branch from c8e15be to ca29fcc Compare November 26, 2025 14:01
@itamar-starkware itamar-starkware force-pushed the 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_gateway branch from e981a1d to 5ad1ea7 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 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @itamar-starkware and @TzahiTaub)

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 1 of 1 files at r3, 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-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_gateway branch from 5ad1ea7 to d8cb1a7 Compare December 2, 2025 13:09
@itamar-starkware itamar-starkware force-pushed the 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_batcher branch from ca29fcc to b0808ae Compare December 2, 2025 13:09
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 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

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 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

@itamar-starkware itamar-starkware changed the base branch from 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_gateway to graphite-base/10285 December 3, 2025 09:39
@itamar-starkware itamar-starkware force-pushed the 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_batcher branch from b0808ae to 3f28fa2 Compare December 3, 2025 09:41
@itamar-starkware itamar-starkware changed the base branch from graphite-base/10285 to 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_gateway December 3, 2025 09:41
@itamar-starkware itamar-starkware changed the base branch from 11-19-apollo_gateway_insert_optional_versioned_constant_overrides_to_gateway to main-v0.14.1 December 3, 2025 11:35
Copy link
Contributor Author

@itamar-starkware itamar-starkware left a comment

Choose a reason for hiding this comment

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

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

@itamar-starkware itamar-starkware added this pull request to the merge queue Dec 3, 2025
Merged via the queue into main-v0.14.1 with commit 2afb01c Dec 3, 2025
29 of 40 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.

7 participants