From a0bbd210aab63358005c918a45ce8a56ac25feec Mon Sep 17 00:00:00 2001 From: Jun Song Date: Fri, 4 Sep 2026 21:37:17 +0900 Subject: [PATCH] Warn when a proposer settings file replaces per-key settings saved in the DB selectProposerConfig lets a --proposer-settings-file/-url per-key section replace the DB's per-key section entirely, so fee recipients, gas limits, graffiti or builder configs written through the keymanager API since the last restart vanish with only "Proposer settings loaded from file" in the log. Logging only, no behavior change: after the merge, compare the normalized settings against what the DB held and warn once with the dropped and overridden keys (capped at 10 with a "+N more" tail). Comparing normalized settings rather than raw payloads keeps an unchanged restart quiet, including v1 setups where --enable-builder fills in per-key builder content. Co-Authored-By: Claude Fable 5.1 --- .../syjn99_proposer-settings-loud-logs.md | 3 + config/proposer/loader/BUILD.bazel | 1 + config/proposer/loader/loader.go | 62 ++++++++- config/proposer/loader/loader_test.go | 124 ++++++++++++++++-- .../testdata/per-key-identical-to-db.json | 7 + .../testdata/per-key-replaces-db-capped.json | 7 + .../loader/testdata/per-key-replaces-db.json | 10 ++ 7 files changed, 201 insertions(+), 13 deletions(-) create mode 100644 changelog/syjn99_proposer-settings-loud-logs.md create mode 100644 config/proposer/loader/testdata/per-key-identical-to-db.json create mode 100644 config/proposer/loader/testdata/per-key-replaces-db-capped.json create mode 100644 config/proposer/loader/testdata/per-key-replaces-db.json diff --git a/changelog/syjn99_proposer-settings-loud-logs.md b/changelog/syjn99_proposer-settings-loud-logs.md new file mode 100644 index 000000000000..e76abe7ebe17 --- /dev/null +++ b/changelog/syjn99_proposer-settings-loud-logs.md @@ -0,0 +1,3 @@ +### Changed + +- The validator client now warns at startup when per-key proposer settings saved in the validator DB (including changes made through the keymanager API) are replaced by the configured `--proposer-settings-file`/`--proposer-settings-url`, listing the dropped and overridden keys. diff --git a/config/proposer/loader/BUILD.bazel b/config/proposer/loader/BUILD.bazel index 401a0fbcb33c..2681de0d44c0 100644 --- a/config/proposer/loader/BUILD.bazel +++ b/config/proposer/loader/BUILD.bazel @@ -45,5 +45,6 @@ go_library( "@com_github_pkg_errors//:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", "@com_github_urfave_cli_v2//:go_default_library", + "@org_golang_google_protobuf//proto:go_default_library", ], ) diff --git a/config/proposer/loader/loader.go b/config/proposer/loader/loader.go index 1ecaa71dfe84..dbb4e7e48afe 100644 --- a/config/proposer/loader/loader.go +++ b/config/proposer/loader/loader.go @@ -2,7 +2,9 @@ package loader import ( "fmt" + "sort" "strconv" + "strings" "github.com/OffchainLabs/prysm/v7/cmd/validator/flags" "github.com/OffchainLabs/prysm/v7/config" @@ -14,8 +16,12 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/pkg/errors" "github.com/urfave/cli/v2" + "google.golang.org/protobuf/proto" ) +// maxLoggedKeys caps the key lists in the DB-replacement warning. +const maxLoggedKeys = 10 + type settingsType int const ( @@ -27,10 +33,11 @@ const ( ) type SettingsLoader struct { - loadMethods []settingsType - existsInDB bool - db iface.ValidatorDB - options *flagOptions + loadMethods []settingsType + existsInDB bool + replacesDBKeys bool + db iface.ValidatorDB + options *flagOptions } type flagOptions struct { @@ -123,13 +130,15 @@ func determineLoadMethods(cliCtx *cli.Context, loadedFromDB bool) []settingsType // Load saves the proposer settings to the database func (psl *SettingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error) { var loadedSettings, dbSettings *validatorpb.ProposerSettingsPayload + var dbps *proposer.Settings // override settings based on other options psl.applyOverrides() // check if database has settings already if psl.existsInDB { - dbps, err := psl.db.ProposerSettings(cliCtx.Context) + var err error + dbps, err = psl.db.ProposerSettings(cliCtx.Context) if err != nil { return nil, err } @@ -179,12 +188,52 @@ func (psl *SettingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error) } ps.WarnDeprecatedSchema() ps.WarnUnsetMaxExecutionPayment() + if psl.replacesDBKeys { + warnReplacedDBKeys(dbps, ps) + } if err := psl.db.SaveProposerSettings(cliCtx.Context, ps); err != nil { return nil, err } return ps, nil } +// warnReplacedDBKeys lists the DB per-key entries the settings file/URL replaced. +// Comparing normalized settings keeps an unchanged restart quiet. +func warnReplacedDBKeys(db, merged *proposer.Settings) { + if db == nil || len(db.ProposeConfig) == 0 { + return + } + var dropped, overridden []string + for key, dbOpt := range db.ProposeConfig { + opt, ok := merged.ProposeConfig[key] + switch { + case !ok: + dropped = append(dropped, fmt.Sprintf("%#x", key)) + case !proto.Equal(opt.ToConsensus(), dbOpt.ToConsensus()): + overridden = append(overridden, fmt.Sprintf("%#x", key)) + } + } + if len(dropped) == 0 && len(overridden) == 0 { + return + } + log.WithField("droppedKeys", capKeys(dropped)). + WithField("droppedCount", len(dropped)). + WithField("overriddenKeys", capKeys(overridden)). + WithField("overriddenCount", len(overridden)). + Warn("Per-key proposer settings saved in the validator DB by a previous run differ from the configured settings file/URL; " + + "the settings source is authoritative and the DB entries are replaced. " + + "Changes made through the keymanager API do not survive a restart while a settings file or URL is configured") +} + +// capKeys renders a sorted key list, truncated to maxLoggedKeys with a "+N more" tail. +func capKeys(keys []string) string { + sort.Strings(keys) + if len(keys) > maxLoggedKeys { + return fmt.Sprintf("%s +%d more", strings.Join(keys[:maxLoggedKeys], ","), len(keys)-maxLoggedKeys) + } + return strings.Join(keys, ",") +} + func (psl *SettingsLoader) applyOverrides() { if psl.options.builderConfig != nil && psl.options.gasLimit != nil { psl.options.builderConfig.GasLimit = *psl.options.gasLimit @@ -220,6 +269,7 @@ func (psl *SettingsLoader) loadFromFile(cliCtx *cli.Context, dbSettings *validat } markExplicitEmptyBuilders(settingFromFile) inferSchemaVersion(settingFromFile) + psl.replacesDBKeys = len(settingFromFile.ProposerConfig) > 0 log.WithField(flags.ProposerSettingsFlag.Name, cliCtx.String(flags.ProposerSettingsFlag.Name)).Info("Proposer settings loaded from file") return psl.processProposerSettings(settingFromFile, dbSettings), nil } @@ -234,6 +284,7 @@ func (psl *SettingsLoader) loadFromURL(cliCtx *cli.Context, dbSettings *validato } markExplicitEmptyBuilders(settingFromURL) inferSchemaVersion(settingFromURL) + psl.replacesDBKeys = len(settingFromURL.ProposerConfig) > 0 log.WithField(flags.ProposerSettingsURLFlag.Name, cliCtx.String(flags.ProposerSettingsURLFlag.Name)).Infof("Proposer settings loaded from URL") return psl.processProposerSettings(settingFromURL, dbSettings), nil } @@ -328,6 +379,7 @@ func inferSchemaVersion(p *validatorpb.ProposerSettingsPayload) { // selectProposerConfig keeps the pre-v2 source precedence: a loaded per-key // section replaces the DB's entirely, so restarting with a file resets the DB. +// Load reports what that replaced through warnReplacedDBKeys. func selectProposerConfig(db, loaded *validatorpb.ProposerSettingsPayload) map[string]*validatorpb.ProposerOptionPayload { if loaded != nil && len(loaded.ProposerConfig) > 0 { return loaded.ProposerConfig diff --git a/config/proposer/loader/loader_test.go b/config/proposer/loader/loader_test.go index 7f1a55a4a069..e09fc5e79b48 100644 --- a/config/proposer/loader/loader_test.go +++ b/config/proposer/loader/loader_test.go @@ -28,6 +28,10 @@ import ( func TestProposerSettingsLoader(t *testing.T) { hook := logtest.NewGlobal() + // Keys used by the per-key-replaces-db*.json testdata. + keyA := [fieldparams.BLSPubkeyLength]byte{0xaa} + keyB := [fieldparams.BLSPubkeyLength]byte{0xbb} + keyC := [fieldparams.BLSPubkeyLength]byte{0xcc} type proposerSettingsFlag struct { dir string url string @@ -45,7 +49,8 @@ func TestProposerSettingsLoader(t *testing.T) { urlResponse string wantInitErr string wantErr string - wantLog string + wantLogs []string + wantNoLogs []string withdb func(db iface.ValidatorDB) error validatorRegistrationEnabled bool skipDBSavedCheck bool @@ -235,7 +240,7 @@ func TestProposerSettingsLoader(t *testing.T) { want: func() *proposer.Settings { return nil }, - wantLog: "No proposer settings were provided", + wantLogs: []string{"No proposer settings were provided"}, skipDBSavedCheck: true, }, { @@ -288,8 +293,8 @@ func TestProposerSettingsLoader(t *testing.T) { }, } }, - wantErr: "", - wantLog: "is not a checksum Ethereum address", + wantErr: "", + wantLogs: []string{"is not a checksum Ethereum address"}, }, { name: "Happy Path Config file File multiple fee recipients", @@ -969,10 +974,112 @@ func TestProposerSettingsLoader(t *testing.T) { }, wantErr: "failed to unmarshal yaml file", }, + { + name: "file per-key entries replace db entries and warn with dropped and overridden keys", + args: args{ + proposerSettingsFlagValues: &proposerSettingsFlag{ + dir: "./testdata/per-key-replaces-db.json", + }, + }, + want: func() *proposer.Settings { + return &proposer.Settings{ + ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*proposer.Option{ + keyA: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0x3333333333333333333333333333333333333333")}}, + keyC: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0x4444444444444444444444444444444444444444")}}, + }, + } + }, + withdb: func(db iface.ValidatorDB) error { + return db.SaveProposerSettings(t.Context(), &proposer.Settings{ + ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*proposer.Option{ + keyA: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0x1111111111111111111111111111111111111111")}}, + keyB: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0x2222222222222222222222222222222222222222")}}, + }, + }) + }, + wantLogs: []string{ + "differ from the configured settings file/URL", + fmt.Sprintf("overriddenKeys=%#x", keyA), + fmt.Sprintf("droppedKeys=%#x", keyB), + "overriddenCount=1", + "droppedCount=1", + }, + }, + { + // Lowercase in the file, checksummed in the DB: the compare runs after normalization. + name: "file per-key entries identical to db entries do not warn", + args: args{ + proposerSettingsFlagValues: &proposerSettingsFlag{ + dir: "./testdata/per-key-identical-to-db.json", + }, + }, + want: func() *proposer.Settings { + return &proposer.Settings{ + ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*proposer.Option{ + keyA: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0xabcdefabcdefabcdefabcdefabcdefabcdefabcd")}}, + }, + } + }, + withdb: func(db iface.ValidatorDB) error { + return db.SaveProposerSettings(t.Context(), &proposer.Settings{ + ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*proposer.Option{ + keyA: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0xabcdefabcdefabcdefabcdefabcdefabcdefabcd")}}, + }, + }) + }, + wantNoLogs: []string{"differ from the configured settings file/URL"}, + }, + { + name: "file per-key entries with an empty db do not warn", + args: args{ + proposerSettingsFlagValues: &proposerSettingsFlag{ + dir: "./testdata/per-key-identical-to-db.json", + }, + }, + want: func() *proposer.Settings { + return &proposer.Settings{ + ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*proposer.Option{ + keyA: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0xabcdefabcdefabcdefabcdefabcdefabcdefabcd")}}, + }, + } + }, + wantNoLogs: []string{"differ from the configured settings file/URL"}, + }, + { + name: "replaced db key list is capped", + args: args{ + proposerSettingsFlagValues: &proposerSettingsFlag{ + dir: "./testdata/per-key-replaces-db-capped.json", + }, + }, + want: func() *proposer.Settings { + return &proposer.Settings{ + ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*proposer.Option{ + {0xff}: &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0x1111111111111111111111111111111111111111")}}, + }, + } + }, + withdb: func(db iface.ValidatorDB) error { + settings := &proposer.Settings{ProposeConfig: map[[fieldparams.BLSPubkeyLength]byte]*proposer.Option{}} + for i := 0; i < 15; i++ { + settings.ProposeConfig[[fieldparams.BLSPubkeyLength]byte{byte(i)}] = &proposer.Option{FeeRecipientConfig: &proposer.FeeRecipientConfig{FeeRecipient: common.HexToAddress("0x1111111111111111111111111111111111111111")}} + } + return db.SaveProposerSettings(t.Context(), settings) + }, + wantLogs: []string{ + "differ from the configured settings file/URL", + "droppedCount=15", + "+5 more", + // Sorted hex: the 10th key (first byte 0x09) is listed, the 11th (0x0a) is behind the cap. + fmt.Sprintf("%#x", [fieldparams.BLSPubkeyLength]byte{0x09}), + }, + wantNoLogs: []string{fmt.Sprintf("%#x", [fieldparams.BLSPubkeyLength]byte{0x0a})}, + }, } for _, tt := range tests { for _, isSlashingProtectionMinimal := range [...]bool{false, true} { t.Run(fmt.Sprintf("%v-minimal:%v", tt.name, isSlashingProtectionMinimal), func(t *testing.T) { + hook.Reset() app := cli.App{} set := flag.NewFlagSet("test", 0) if tt.args.proposerSettingsFlagValues.dir != "" { @@ -1029,10 +1136,11 @@ func TestProposerSettingsLoader(t *testing.T) { } else { require.NoError(t, err) } - if tt.wantLog != "" { - assert.LogsContain(t, hook, - tt.wantLog, - ) + for _, want := range tt.wantLogs { + assert.LogsContain(t, hook, want) + } + for _, notWant := range tt.wantNoLogs { + assert.LogsDoNotContain(t, hook, notWant) } w := tt.want() require.DeepEqual(t, w, got) diff --git a/config/proposer/loader/testdata/per-key-identical-to-db.json b/config/proposer/loader/testdata/per-key-identical-to-db.json new file mode 100644 index 000000000000..ba0a5da4205e --- /dev/null +++ b/config/proposer/loader/testdata/per-key-identical-to-db.json @@ -0,0 +1,7 @@ +{ + "proposer_config": { + "0xaa0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000": { + "fee_recipient": "0xabcdefabcdefabcdefabcdefabcdefabcdefabcd" + } + } +} diff --git a/config/proposer/loader/testdata/per-key-replaces-db-capped.json b/config/proposer/loader/testdata/per-key-replaces-db-capped.json new file mode 100644 index 000000000000..aedbcfc2eba6 --- /dev/null +++ b/config/proposer/loader/testdata/per-key-replaces-db-capped.json @@ -0,0 +1,7 @@ +{ + "proposer_config": { + "0xff0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000": { + "fee_recipient": "0x1111111111111111111111111111111111111111" + } + } +} diff --git a/config/proposer/loader/testdata/per-key-replaces-db.json b/config/proposer/loader/testdata/per-key-replaces-db.json new file mode 100644 index 000000000000..ec5fb4ab8c1c --- /dev/null +++ b/config/proposer/loader/testdata/per-key-replaces-db.json @@ -0,0 +1,10 @@ +{ + "proposer_config": { + "0xaa0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000": { + "fee_recipient": "0x3333333333333333333333333333333333333333" + }, + "0xcc0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000": { + "fee_recipient": "0x4444444444444444444444444444444444444444" + } + } +}