-
Notifications
You must be signed in to change notification settings - Fork 65
apollo_gateway: insert optional versioned constant overrides to gateway #10284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 11-18-apollo_batcher_make_versioned_constants_override_optional
Are you sure you want to change the base?
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @TzahiTaub)
8ad7b5a to
bc65161
Compare
23c9984 to
e981a1d
Compare
TzahiTaub
left a comment
There was a problem hiding this 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_gateway_config/src/config.rs line 225 at r2 (raw file):
max_nonce_for_validation_skip: Nonce(Felt::ONE), min_gas_price_percentage: 100, versioned_constants_overrides: Some(VersionedConstantsOverrides::default()),
I think that this should either be None (and the default impl should be removed), or the default should return None.
Code quote:
Some(VersionedConstantsOverrides::default()),
Yoni-Starkware
left a comment
There was a problem hiding this 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_gateway_config/src/config.rs line 225 at r2 (raw file):
I think that this should either be
None(and the default impl should be removed)
+1
03bd26a to
4f24f7a
Compare
e981a1d to
5ad1ea7
Compare
ShahakShama
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ShahakShama reviewed all commit messages.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @itamar-starkware, @Itay-Tsabary-Starkware, and @TzahiTaub)
TzahiTaub
left a comment
There was a problem hiding this 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:complete! all files reviewed, all discussions resolved (waiting on @itamar-starkware)

Make
versioned_constants_overridesoptional in the StatefulTransactionValidatorConfig.What changed?
versioned_constants_overridesinStatefulTransactionValidatorConfigfromVersionedConstantsOverridestoOption<VersionedConstantsOverrides>VersionedConstantsOverridesinSome()dump()method to handle the optional case by usingunwrap_or_default()StatefulTransactionValidatorFactoryto pass the overrides directly without wrapping in `Some()