improving proposer settings loader readability (#14868)

* updating loader code and adding change log

* updating variable names to reduce confusion

* exporting loader type

* Update config/proposer/loader/loader.go

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

* Update config/proposer/loader/loader.go

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

* Update config/proposer/loader/loader.go

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>

* gofmt

---------

Co-authored-by: Sammy Rosso <15244892+saolyn@users.noreply.github.com>
This commit is contained in:
james-prysm
2025-02-04 17:18:50 -06:00
committed by GitHub
parent 8d5090ce54
commit e331d5b371
2 changed files with 139 additions and 103 deletions

View File

@@ -28,7 +28,7 @@ const (
onlyDB
)
type settingsLoader struct {
type SettingsLoader struct {
loadMethods []settingsType
existsInDB bool
db iface.ValidatorDB
@@ -41,11 +41,11 @@ type flagOptions struct {
}
// SettingsLoaderOption sets additional options that affect the proposer settings
type SettingsLoaderOption func(cliCtx *cli.Context, psl *settingsLoader) error
type SettingsLoaderOption func(cliCtx *cli.Context, psl *SettingsLoader) error
// WithBuilderConfig applies the --enable-builder flag to proposer settings
func WithBuilderConfig() SettingsLoaderOption {
return func(cliCtx *cli.Context, psl *settingsLoader) error {
return func(cliCtx *cli.Context, psl *SettingsLoader) error {
if cliCtx.Bool(flags.EnableBuilderFlag.Name) {
psl.options.builderConfig = &proposer.BuilderConfig{
Enabled: true,
@@ -58,7 +58,7 @@ func WithBuilderConfig() SettingsLoaderOption {
// WithGasLimit applies the --suggested-gas-limit flag to proposer settings
func WithGasLimit() SettingsLoaderOption {
return func(cliCtx *cli.Context, psl *settingsLoader) error {
return func(cliCtx *cli.Context, psl *SettingsLoader) error {
sgl := cliCtx.String(flags.BuilderGasLimitFlag.Name)
if sgl != "" {
gl, err := strconv.ParseUint(sgl, 10, 64)
@@ -76,7 +76,7 @@ func WithGasLimit() SettingsLoaderOption {
}
// 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) {
func NewProposerSettingsLoader(cliCtx *cli.Context, db iface.ValidatorDB, opts ...SettingsLoaderOption) (*SettingsLoader, error) {
if cliCtx.IsSet(flags.ProposerSettingsFlag.Name) && cliCtx.IsSet(flags.ProposerSettingsURLFlag.Name) {
return nil, fmt.Errorf("cannot specify both --%s and --%s flags; choose one method for specifying proposer settings", flags.ProposerSettingsFlag.Name, flags.ProposerSettingsURLFlag.Name)
}
@@ -84,25 +84,9 @@ func NewProposerSettingsLoader(cliCtx *cli.Context, db iface.ValidatorDB, opts .
if err != nil {
return nil, err
}
psl := &settingsLoader{db: db, existsInDB: psExists, options: &flagOptions{}}
psl := &SettingsLoader{db: db, existsInDB: psExists, options: &flagOptions{}}
if cliCtx.IsSet(flags.SuggestedFeeRecipientFlag.Name) {
psl.loadMethods = append(psl.loadMethods, defaultFlag)
}
if cliCtx.IsSet(flags.ProposerSettingsFlag.Name) {
psl.loadMethods = append(psl.loadMethods, fileFlag)
}
if cliCtx.IsSet(flags.ProposerSettingsURLFlag.Name) {
psl.loadMethods = append(psl.loadMethods, urlFlag)
}
if len(psl.loadMethods) == 0 {
method := none
if psExists {
// override with db
method = onlyDB
}
psl.loadMethods = append(psl.loadMethods, method)
}
psl.loadMethods = determineLoadMethods(cliCtx, psl.existsInDB)
for _, o := range opts {
if err := o(cliCtx, psl); err != nil {
@@ -113,14 +97,34 @@ func NewProposerSettingsLoader(cliCtx *cli.Context, db iface.ValidatorDB, opts .
return psl, nil
}
func determineLoadMethods(cliCtx *cli.Context, loadedFromDB bool) []settingsType {
var methods []settingsType
if cliCtx.IsSet(flags.SuggestedFeeRecipientFlag.Name) {
methods = append(methods, defaultFlag)
}
if cliCtx.IsSet(flags.ProposerSettingsFlag.Name) {
methods = append(methods, fileFlag)
}
if cliCtx.IsSet(flags.ProposerSettingsURLFlag.Name) {
methods = append(methods, urlFlag)
}
if len(methods) == 0 && loadedFromDB {
methods = append(methods, onlyDB)
}
if len(methods) == 0 {
methods = append(methods, none)
}
return methods
}
// Load saves the proposer settings to the database
func (psl *settingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error) {
loadConfig := &validatorpb.ProposerSettingsPayload{}
func (psl *SettingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error) {
var loadedSettings, dbSettings *validatorpb.ProposerSettingsPayload
// override settings based on other options
if psl.options.builderConfig != nil && psl.options.gasLimit != nil {
psl.options.builderConfig.GasLimit = *psl.options.gasLimit
}
psl.applyOverrides()
// check if database has settings already
if psl.existsInDB {
@@ -128,9 +132,9 @@ func (psl *settingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error)
if err != nil {
return nil, err
}
loadConfig = dbps.ToConsensus()
dbSettings = dbps.ToConsensus()
log.Debugf("DB loaded proposer settings: %s", func() string {
b, err := json.Marshal(loadConfig)
b, err := json.Marshal(dbSettings)
if err != nil {
return err.Error()
}
@@ -140,72 +144,39 @@ func (psl *settingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error)
// start to process based on load method
for _, method := range psl.loadMethods {
var err error
switch method {
case defaultFlag:
suggestedFeeRecipient := cliCtx.String(flags.SuggestedFeeRecipientFlag.Name)
if !common.IsHexAddress(suggestedFeeRecipient) {
return nil, errors.Errorf("--%s is not a valid Ethereum address", flags.SuggestedFeeRecipientFlag.Name)
}
if err := config.WarnNonChecksummedAddress(suggestedFeeRecipient); err != nil {
loadedSettings, err = psl.loadFromDefault(cliCtx, dbSettings)
if err != nil {
return nil, err
}
defaultConfig := &validatorpb.ProposerOptionPayload{
FeeRecipient: suggestedFeeRecipient,
}
if psl.options.builderConfig != nil {
defaultConfig.Builder = psl.options.builderConfig.ToConsensus()
}
if psl.existsInDB && len(psl.loadMethods) == 1 {
// only log the below if default flag is the only load method
log.Debug("Overriding previously saved proposer default settings.")
}
loadConfig.DefaultConfig = defaultConfig
case fileFlag:
var settingFromFile *validatorpb.ProposerSettingsPayload
if err := config.UnmarshalFromFile(cliCtx.String(flags.ProposerSettingsFlag.Name), &settingFromFile); err != nil {
loadedSettings, err = psl.loadFromFile(cliCtx, dbSettings)
if err != nil {
return nil, err
}
if settingFromFile == nil {
return nil, errors.Errorf("proposer settings is empty after unmarshalling from file specified by %s flag", flags.ProposerSettingsFlag.Name)
}
loadConfig = psl.processProposerSettings(settingFromFile, loadConfig)
log.WithField(flags.ProposerSettingsFlag.Name, cliCtx.String(flags.ProposerSettingsFlag.Name)).Info("Proposer settings loaded from file")
case urlFlag:
var settingFromURL *validatorpb.ProposerSettingsPayload
if err := config.UnmarshalFromURL(cliCtx.Context, cliCtx.String(flags.ProposerSettingsURLFlag.Name), &settingFromURL); err != nil {
loadedSettings, err = psl.loadFromURL(cliCtx, dbSettings)
if err != nil {
return nil, err
}
if settingFromURL == nil {
return nil, errors.New("proposer settings is empty after unmarshalling from url")
}
loadConfig = psl.processProposerSettings(settingFromURL, loadConfig)
log.WithField(flags.ProposerSettingsURLFlag.Name, cliCtx.String(flags.ProposerSettingsURLFlag.Name)).Infof("Proposer settings loaded from URL")
case onlyDB:
loadConfig = psl.processProposerSettings(nil, loadConfig)
log.Info("Proposer settings loaded from the DB")
case none:
case onlyDB, none:
loadedSettings = psl.processProposerSettings(&validatorpb.ProposerSettingsPayload{}, dbSettings)
if psl.existsInDB {
log.Info("Proposer settings loaded from the DB")
}
if psl.options.builderConfig != nil {
// if there are no proposer settings provided, create a default where fee recipient is not populated, this will be skipped for validator registration on validators that don't have a fee recipient set.
// skip saving to DB if only builder settings are provided until a trigger like keymanager API updates with fee recipient values
option := &proposer.Option{
BuilderConfig: psl.options.builderConfig.Clone(),
}
loadConfig.DefaultConfig = option.ToConsensus()
}
default:
return nil, errors.New("load method for proposer settings does not exist")
}
}
// exit early if nothing is provided
if loadConfig == nil || (loadConfig.ProposerConfig == nil && loadConfig.DefaultConfig == nil) {
if loadedSettings == nil || (loadedSettings.ProposerConfig == nil && loadedSettings.DefaultConfig == nil) {
log.Warn("No proposer settings were provided")
return nil, nil
}
ps, err := proposer.SettingFromConsensus(loadConfig)
ps, err := proposer.SettingFromConsensus(loadedSettings)
if err != nil {
return nil, err
}
@@ -215,66 +186,128 @@ func (psl *settingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error)
return ps, nil
}
func (psl *settingsLoader) processProposerSettings(loadedSettings, dbSettings *validatorpb.ProposerSettingsPayload) *validatorpb.ProposerSettingsPayload {
func (psl *SettingsLoader) applyOverrides() {
if psl.options.builderConfig != nil && psl.options.gasLimit != nil {
psl.options.builderConfig.GasLimit = *psl.options.gasLimit
}
}
func (psl *SettingsLoader) loadFromDefault(cliCtx *cli.Context, dbSettings *validatorpb.ProposerSettingsPayload) (*validatorpb.ProposerSettingsPayload, error) {
suggestedFeeRecipient := cliCtx.String(flags.SuggestedFeeRecipientFlag.Name)
if !common.IsHexAddress(suggestedFeeRecipient) {
return nil, errors.Errorf("--%s is not a valid Ethereum address", flags.SuggestedFeeRecipientFlag.Name)
}
if err := config.WarnNonChecksummedAddress(suggestedFeeRecipient); err != nil {
return nil, err
}
if psl.existsInDB && len(psl.loadMethods) == 1 {
// only log the below if default flag is the only load method
log.Debug("Overriding previously saved proposer default settings.")
}
log.WithField(flags.SuggestedFeeRecipientFlag.Name, cliCtx.String(flags.SuggestedFeeRecipientFlag.Name)).Info("Proposer settings loaded from default")
return psl.processProposerSettings(&validatorpb.ProposerSettingsPayload{DefaultConfig: &validatorpb.ProposerOptionPayload{
FeeRecipient: suggestedFeeRecipient,
}}, dbSettings), nil
}
func (psl *SettingsLoader) loadFromFile(cliCtx *cli.Context, dbSettings *validatorpb.ProposerSettingsPayload) (*validatorpb.ProposerSettingsPayload, error) {
var settingFromFile *validatorpb.ProposerSettingsPayload
if err := config.UnmarshalFromFile(cliCtx.String(flags.ProposerSettingsFlag.Name), &settingFromFile); err != nil {
return nil, err
}
if settingFromFile == nil {
return nil, errors.Errorf("proposer settings is empty after unmarshalling from file specified by %s flag", flags.ProposerSettingsFlag.Name)
}
log.WithField(flags.ProposerSettingsFlag.Name, cliCtx.String(flags.ProposerSettingsFlag.Name)).Info("Proposer settings loaded from file")
return psl.processProposerSettings(settingFromFile, dbSettings), nil
}
func (psl *SettingsLoader) loadFromURL(cliCtx *cli.Context, dbSettings *validatorpb.ProposerSettingsPayload) (*validatorpb.ProposerSettingsPayload, error) {
var settingFromURL *validatorpb.ProposerSettingsPayload
if err := config.UnmarshalFromURL(cliCtx.Context, cliCtx.String(flags.ProposerSettingsURLFlag.Name), &settingFromURL); err != nil {
return nil, err
}
if settingFromURL == nil {
return nil, errors.Errorf("proposer settings is empty after unmarshalling from url specified by %s flag", flags.ProposerSettingsURLFlag.Name)
}
log.WithField(flags.ProposerSettingsURLFlag.Name, cliCtx.String(flags.ProposerSettingsURLFlag.Name)).Infof("Proposer settings loaded from URL")
return psl.processProposerSettings(settingFromURL, dbSettings), nil
}
func (psl *SettingsLoader) processProposerSettings(loadedSettings, dbSettings *validatorpb.ProposerSettingsPayload) *validatorpb.ProposerSettingsPayload {
if loadedSettings == nil && dbSettings == nil {
return nil
}
// loaded settings have higher priority than db settings
newSettings := &validatorpb.ProposerSettingsPayload{}
// Merge settings with priority: loadedSettings > dbSettings
newSettings := mergeProposerSettings(loadedSettings, dbSettings, psl.options)
// Return nil if settings remain empty
if newSettings.DefaultConfig == nil && len(newSettings.ProposerConfig) == 0 {
return nil
}
return newSettings
}
// mergeProposerSettings merges database settings with loaded settings, giving precedence to loadedSettings
func mergeProposerSettings(loaded, db *validatorpb.ProposerSettingsPayload, options *flagOptions) *validatorpb.ProposerSettingsPayload {
merged := &validatorpb.ProposerSettingsPayload{}
// Apply builder config overrides
var builderConfig *validatorpb.BuilderConfig
var gasLimitOnly *validator.Uint64
if psl.options != nil {
if psl.options.builderConfig != nil {
builderConfig = psl.options.builderConfig.ToConsensus()
if options != nil {
if options.builderConfig != nil {
builderConfig = options.builderConfig.ToConsensus()
}
if psl.options.gasLimit != nil {
gasLimitOnly = psl.options.gasLimit
if options.gasLimit != nil {
gasLimitOnly = options.gasLimit
}
}
if dbSettings != nil && dbSettings.DefaultConfig != nil {
// Merge DefaultConfig
if db != nil && db.DefaultConfig != nil {
merged.DefaultConfig = db.DefaultConfig
// db always falls back to local building if no builder settings are provided
if builderConfig == nil {
dbSettings.DefaultConfig.Builder = nil
db.DefaultConfig.Builder = nil
}
newSettings.DefaultConfig = dbSettings.DefaultConfig
}
if loadedSettings != nil && loadedSettings.DefaultConfig != nil {
newSettings.DefaultConfig = loadedSettings.DefaultConfig
if loaded != nil && loaded.DefaultConfig != nil {
merged.DefaultConfig = loaded.DefaultConfig
}
// process any builder overrides on defaults
if newSettings.DefaultConfig != nil {
newSettings.DefaultConfig.Builder = processBuilderConfig(newSettings.DefaultConfig.Builder, builderConfig, gasLimitOnly)
}
if dbSettings != nil && len(dbSettings.ProposerConfig) != 0 {
for _, option := range dbSettings.ProposerConfig {
// Merge ProposerConfig
if db != nil && len(db.ProposerConfig) > 0 {
merged.ProposerConfig = db.ProposerConfig
for _, option := range db.ProposerConfig {
// db always falls back to local building if no builder settings are provided
if builderConfig == nil {
option.Builder = nil
}
}
newSettings.ProposerConfig = dbSettings.ProposerConfig
}
if loadedSettings != nil && len(loadedSettings.ProposerConfig) != 0 {
newSettings.ProposerConfig = loadedSettings.ProposerConfig
if loaded != nil && len(loaded.ProposerConfig) > 0 {
merged.ProposerConfig = loaded.ProposerConfig
}
// process any overrides for proposer config
for _, option := range newSettings.ProposerConfig {
if merged.DefaultConfig != nil {
merged.DefaultConfig.Builder = processBuilderConfig(merged.DefaultConfig.Builder, builderConfig, gasLimitOnly)
}
for _, option := range merged.ProposerConfig {
if option != nil {
option.Builder = processBuilderConfig(option.Builder, builderConfig, gasLimitOnly)
}
}
// if default and proposer configs are both missing even after db setting
if newSettings.DefaultConfig == nil && newSettings.ProposerConfig == nil {
return nil
if merged.DefaultConfig == nil && builderConfig != nil {
merged.DefaultConfig = &validatorpb.ProposerOptionPayload{Builder: builderConfig}
}
return newSettings
return merged
}
func processBuilderConfig(current *validatorpb.BuilderConfig, override *validatorpb.BuilderConfig, gasLimitOnly *validator.Uint64) *validatorpb.BuilderConfig {