-
Notifications
You must be signed in to change notification settings - Fork 1k
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
adding builder boost factor to proposer settings #13422
Changes from 31 commits
8ce7287
ba73f17
6741223
5d50550
86bc39d
91c9bc4
c880301
f3eb41d
414a463
e9b6193
8749e54
26ba7aa
36ade7b
4e430a5
ad942a8
996192f
75038e4
ae3be20
2291865
bea4620
b0be7ca
1bf7b44
f8c4a94
5492018
bdb2158
421f967
8aa5f73
062518b
c8181ad
3086d0e
ee5703d
041b077
4680af1
fd7df55
749cb98
099d393
85772bf
fb09cc5
719c4aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,8 +35,9 @@ type settingsLoader struct { | |
} | ||
|
||
type flagOptions struct { | ||
builderConfig *proposer.BuilderConfig | ||
gasLimit *validator.Uint64 | ||
builderConfig *proposer.BuilderConfig | ||
gasLimit *validator.Uint64 | ||
builderBoostFactor *uint64 | ||
} | ||
|
||
// SettingsLoaderOption sets additional options that affect the proposer settings | ||
|
@@ -74,6 +75,17 @@ func WithGasLimit() SettingsLoaderOption { | |
} | ||
} | ||
|
||
// WithBuilderBoostFactor applies the --builder-boost-factor flag to proposer settings | ||
func WithBuilderBoostFactor() SettingsLoaderOption { | ||
return func(cliCtx *cli.Context, psl *settingsLoader) error { | ||
if cliCtx.IsSet(flags.BuilderBoostFactorFlag.Name) { | ||
bbf := cliCtx.Uint64(flags.BuilderBoostFactorFlag.Name) | ||
psl.options.builderBoostFactor = &bbf | ||
} | ||
return nil | ||
} | ||
} | ||
|
||
// NewProposerSettingsLoader returns a new proposer settings loader that can process the proposer settings based on flag options | ||
func NewProposerSettingsLoader(cliCtx *cli.Context, db iface.ValidatorDB, opts ...SettingsLoaderOption) (*settingsLoader, error) { | ||
if cliCtx.IsSet(flags.ProposerSettingsFlag.Name) && cliCtx.IsSet(flags.ProposerSettingsURLFlag.Name) { | ||
|
@@ -117,8 +129,11 @@ func (psl *settingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error) | |
loadConfig := &validatorpb.ProposerSettingsPayload{} | ||
|
||
// override settings based on other options | ||
if psl.options.builderConfig != nil && psl.options.gasLimit != nil { | ||
psl.options.builderConfig.GasLimit = *psl.options.gasLimit | ||
if psl.options != nil && psl.options.builderConfig != nil { | ||
if psl.options.gasLimit != nil { | ||
psl.options.builderConfig.GasLimit = *psl.options.gasLimit | ||
} | ||
psl.options.builderConfig.BuilderBoostFactor = psl.options.builderBoostFactor | ||
} | ||
|
||
// check if database has settings already | ||
|
@@ -205,20 +220,8 @@ func (psl *settingsLoader) processProposerSettings(loadedSettings, dbSettings *v | |
// loaded settings have higher priority than db settings | ||
newSettings := &validatorpb.ProposerSettingsPayload{} | ||
|
||
var builderConfig *validatorpb.BuilderConfig | ||
var gasLimitOnly *validator.Uint64 | ||
|
||
if psl.options != nil { | ||
if psl.options.builderConfig != nil { | ||
builderConfig = psl.options.builderConfig.ToConsensus() | ||
} | ||
if psl.options.gasLimit != nil { | ||
gasLimitOnly = psl.options.gasLimit | ||
} | ||
} | ||
|
||
if dbSettings != nil && dbSettings.DefaultConfig != nil { | ||
if builderConfig == nil { | ||
if psl.options == nil || psl.options.builderConfig == nil { | ||
dbSettings.DefaultConfig.Builder = nil | ||
} | ||
newSettings.DefaultConfig = dbSettings.DefaultConfig | ||
|
@@ -229,12 +232,12 @@ func (psl *settingsLoader) processProposerSettings(loadedSettings, dbSettings *v | |
|
||
// process any builder overrides on defaults | ||
if newSettings.DefaultConfig != nil { | ||
newSettings.DefaultConfig.Builder = processBuilderConfig(newSettings.DefaultConfig.Builder, builderConfig, gasLimitOnly) | ||
newSettings.DefaultConfig.Builder = processBuilderConfig(newSettings.DefaultConfig.Builder, psl.options) | ||
} | ||
|
||
if dbSettings != nil && len(dbSettings.ProposerConfig) != 0 { | ||
for _, option := range dbSettings.ProposerConfig { | ||
if builderConfig == nil { | ||
if psl.options == nil || psl.options.builderConfig == nil { | ||
option.Builder = nil | ||
} | ||
} | ||
|
@@ -247,7 +250,7 @@ func (psl *settingsLoader) processProposerSettings(loadedSettings, dbSettings *v | |
// process any overrides for proposer config | ||
for _, option := range newSettings.ProposerConfig { | ||
if option != nil { | ||
option.Builder = processBuilderConfig(option.Builder, builderConfig, gasLimitOnly) | ||
option.Builder = processBuilderConfig(option.Builder, psl.options) | ||
} | ||
} | ||
|
||
|
@@ -259,18 +262,38 @@ func (psl *settingsLoader) processProposerSettings(loadedSettings, dbSettings *v | |
return newSettings | ||
} | ||
|
||
func processBuilderConfig(current *validatorpb.BuilderConfig, override *validatorpb.BuilderConfig, gasLimitOnly *validator.Uint64) *validatorpb.BuilderConfig { | ||
if current != nil { | ||
current.GasLimit = reviewGasLimit(current.GasLimit) | ||
if override != nil { | ||
current.Enabled = override.Enabled | ||
} | ||
if gasLimitOnly != nil { | ||
current.GasLimit = *gasLimitOnly | ||
} | ||
func processBuilderConfig(current *validatorpb.BuilderConfig, options *flagOptions) *validatorpb.BuilderConfig { | ||
// If there are no options, return what was passed in | ||
if options == nil { | ||
return current | ||
} | ||
return override | ||
|
||
// Initialize an override variable | ||
var override *validatorpb.BuilderConfig | ||
if options.builderConfig != nil { | ||
// Convert the builder config to consensus form if it exists | ||
override = options.builderConfig.ToConsensus() | ||
} | ||
// If there's nothing to process further, return the override or current based on what's available | ||
if current == nil { | ||
return override | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will return nil if both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah it means there's nothing to override |
||
} | ||
|
||
if override != nil { | ||
current.Enabled = override.Enabled | ||
} | ||
|
||
if options.gasLimit != nil { | ||
current.GasLimit = *options.gasLimit | ||
} | ||
|
||
current.GasLimit = reviewGasLimit(current.GasLimit) | ||
|
||
if options.builderBoostFactor != nil { | ||
current.BuilderBoostFactor = options.builderBoostFactor | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not test covered. |
||
} | ||
|
||
return current | ||
} | ||
|
||
func reviewGasLimit(gasLimit validator.Uint64) validator.Uint64 { | ||
|
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.
not test covered.
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.
in the current setup it's not possible I guess, I can either remove this check but was just trying to be safe