Add GET/POST/DELETE /eth/v1/validator/{pubkey}/builders keymanager endpoints

changelog/james-prysm_keymanager-builders-endpoints.md

@@ -0,0 +1,16 @@
+### Added
+
+- Add `GET`/`POST`/`DELETE /eth/v1/validator/{pubkey}/builders` keymanager endpoints for per-key builder configuration (keymanager-APIs #88).
+
+### Changed
+
+- Add v2 proposer settings (`"version": 2`): builder fields a key does not set inherit from `default_config`. `--enable-builder` still forces the default builder on; a key with explicit `enabled: false` stays opted out.
+- Setting a fee recipient, gas limit, or graffiti no longer snapshots `default_config`'s builder settings onto that key; the key keeps following the default as it changes.
+
+### Fixed
+
+- Fix the minimal slashing protection database decoding unset builder settings fields as explicit zero values; both validator DB backends now read the same stored settings identically.
+
+### Removed
+
+- Remove the unused `relays` field from the builder config; a settings file that still contains it is unaffected.

cmd/prysmctl/validator/proposer_settings.go

@@ -69,8 +69,9 @@ func getProposerSettings(c *cli.Context, r io.Reader) error {
 		log.Infof("The default fee recipient is set to %s", defaultFeeRecipient)
 		var builderSettings *validatorpb.BuilderConfig
 		if c.Bool(WithBuilderFlag.Name) {
+			enabled := true
 			builderSettings = &validatorpb.BuilderConfig{
-				Enabled:  true,
+				Enabled:  &enabled,
 				GasLimit: validatorType.Uint64(params.BeaconConfig().DefaultBuilderGasLimit),
 			}
 		} else {

config/proposer/BUILD.bazel

@@ -24,17 +24,22 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["settings_test.go"],
+    srcs = [
+        "effective_builder_config_test.go",
+        "settings_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//config/fieldparams:go_default_library",
         "//config/params:go_default_library",
         "//consensus-types/validator:go_default_library",
         "//encoding/bytesutil:go_default_library",
+        "//proto/prysm/v1alpha1/validator-client:go_default_library",
         "//testing/assert:go_default_library",
         "//testing/require:go_default_library",
         "@com_github_ethereum_go_ethereum//common:go_default_library",
         "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
         "@com_github_sirupsen_logrus//hooks/test:go_default_library",
+        "@org_golang_google_protobuf//proto:go_default_library",
     ],
 )

config/proposer/effective_builder_config_test.go

@@ -0,0 +1,104 @@
+package proposer
+
+import (
+	"testing"
+
+	"github.com/OffchainLabs/prysm/v7/consensus-types/validator"
+	"github.com/OffchainLabs/prysm/v7/testing/require"
+	"google.golang.org/protobuf/proto"
+)
+
+// These cases mirror the test vectors proposed upstream on keymanager-APIs #87
+// for builder-config inheritance granularity.
+func TestEffectiveBuilderConfig(t *testing.T) {
+	entryA := &BuilderEntry{URL: "https://a"}
+	entryB := &BuilderEntry{URL: "https://b"}
+	entryC := &BuilderEntry{URL: "https://c"}
+
+	t.Run("nil per-key returns default", func(t *testing.T) {
+		def := &BuilderConfig{Enabled: true}
+		require.Equal(t, def, effectiveBuilderConfig(nil, def))
+	})
+	t.Run("nil default returns per-key", func(t *testing.T) {
+		perKey := &BuilderConfig{Enabled: true}
+		require.Equal(t, perKey, effectiveBuilderConfig(perKey, nil))
+	})
+	t.Run("min_bid inherits when per-key omits it", func(t *testing.T) {
+		def := &BuilderConfig{Enabled: true, Builders: []*BuilderEntry{entryA, entryB}, MinBid: uint64ValPtr(5000000)}
+		perKey := &BuilderConfig{Enabled: true, Builders: []*BuilderEntry{entryC}}
+		eff := effectiveBuilderConfig(perKey, def)
+		require.NotNil(t, eff.MinBid)
+		require.Equal(t, validator.Uint64(5000000), *eff.MinBid)
+		require.Equal(t, 1, len(eff.Builders))
+		require.Equal(t, "https://c", eff.Builders[0].URL)
+	})
+	t.Run("explicit zero max payment is preserved, not inherited over", func(t *testing.T) {
+		def := &BuilderConfig{MaxExecutionPayment: uint64ValPtr(1000000000)}
+		perKey := &BuilderConfig{Enabled: true, MaxExecutionPayment: uint64ValPtr(0)}
+		eff := effectiveBuilderConfig(perKey, def)
+		require.NotNil(t, eff.MaxExecutionPayment)
+		require.Equal(t, validator.Uint64(0), *eff.MaxExecutionPayment)
+	})
+	t.Run("unset max payment inherits default", func(t *testing.T) {
+		def := &BuilderConfig{MaxExecutionPayment: uint64ValPtr(1000000000)}
+		perKey := &BuilderConfig{Enabled: true}
+		eff := effectiveBuilderConfig(perKey, def)
+		require.NotNil(t, eff.MaxExecutionPayment)
+		require.Equal(t, validator.Uint64(1000000000), *eff.MaxExecutionPayment)
+	})
+	t.Run("explicit per-key disable wins over enabled default", func(t *testing.T) {
+		def := &BuilderConfig{Enabled: true}
+		perKey := &BuilderConfig{Enabled: false, MinBid: uint64ValPtr(1)}
+		require.Equal(t, false, effectiveBuilderConfig(perKey, def).IsEnabled())
+	})
+	t.Run("present per-key builder config is authoritative on enabled", func(t *testing.T) {
+		// A per-key config with enabled false does not inherit an enabled default;
+		// whole-config inheritance happens only when the per-key builder config is nil.
+		def := &BuilderConfig{Enabled: true}
+		perKey := &BuilderConfig{MinBid: uint64ValPtr(1)}
+		require.Equal(t, false, effectiveBuilderConfig(perKey, def).IsEnabled())
+		require.Equal(t, true, effectiveBuilderConfig(nil, def).IsEnabled())
+	})
+	t.Run("present builders list replaces, never unions", func(t *testing.T) {
+		def := &BuilderConfig{Builders: []*BuilderEntry{entryA, entryB}}
+		perKey := &BuilderConfig{Builders: []*BuilderEntry{entryC}}
+		eff := effectiveBuilderConfig(perKey, def)
+		require.Equal(t, 1, len(eff.Builders))
+		require.Equal(t, "https://c", eff.Builders[0].URL)
+	})
+	t.Run("absent builders list inherits default list", func(t *testing.T) {
+		def := &BuilderConfig{Builders: []*BuilderEntry{entryA, entryB}}
+		perKey := &BuilderConfig{Enabled: true}
+		require.Equal(t, 2, len(effectiveBuilderConfig(perKey, def).Builders))
+	})
+	t.Run("zero gas limit inherits default gas limit", func(t *testing.T) {
+		def := &BuilderConfig{GasLimit: validator.Uint64(30000000)}
+		perKey := &BuilderConfig{Enabled: true}
+		require.Equal(t, validator.Uint64(30000000), effectiveBuilderConfig(perKey, def).GasLimit)
+	})
+	t.Run("proxy and boost inherit per field", func(t *testing.T) {
+		def := &BuilderConfig{Proxy: proto.String("http://side-car:9001"), BuilderBoostFactor: uint64ValPtr(90)}
+		perKey := &BuilderConfig{Enabled: true, BuilderBoostFactor: uint64ValPtr(120)}
+		eff := effectiveBuilderConfig(perKey, def)
+		require.Equal(t, "http://side-car:9001", *eff.Proxy)
+		require.Equal(t, validator.Uint64(120), *eff.BuilderBoostFactor)
+	})
+	t.Run("both-set min_bid: per-key wins", func(t *testing.T) {
+		def := &BuilderConfig{MinBid: uint64ValPtr(5000000)}
+		perKey := &BuilderConfig{MinBid: uint64ValPtr(7000000)}
+		require.Equal(t, validator.Uint64(7000000), *effectiveBuilderConfig(perKey, def).MinBid)
+	})
+	t.Run("both-set proxy: per-key wins", func(t *testing.T) {
+		def := &BuilderConfig{Proxy: proto.String("http://default:1")}
+		perKey := &BuilderConfig{Proxy: proto.String("http://mine:2")}
+		require.Equal(t, "http://mine:2", *effectiveBuilderConfig(perKey, def).Proxy)
+	})
+	t.Run("nonzero per-key gas limit wins over default", func(t *testing.T) {
+		def := &BuilderConfig{GasLimit: validator.Uint64(30000000)}
+		perKey := &BuilderConfig{GasLimit: validator.Uint64(45000000)}
+		require.Equal(t, validator.Uint64(45000000), effectiveBuilderConfig(perKey, def).GasLimit)
+	})
+	t.Run("nil nil is nil", func(t *testing.T) {
+		require.Equal(t, (*BuilderConfig)(nil), effectiveBuilderConfig(nil, nil))
+	})
+}

config/proposer/loader/BUILD.bazel

@@ -22,6 +22,7 @@ go_test(
         "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
         "@com_github_sirupsen_logrus//hooks/test:go_default_library",
         "@com_github_urfave_cli_v2//:go_default_library",
+        "@org_golang_google_protobuf//proto:go_default_library",
     ],
 )
 

config/proposer/loader/loader.go

@@ -1,7 +1,6 @@
 package loader
 
 import (
-	"encoding/json"
 	"fmt"
 	"strconv"
 
@@ -132,13 +131,9 @@ func (psl *SettingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error)
 			return nil, err
 		}
 		dbSettings = dbps.ToConsensus()
-		log.Debugf("DB loaded proposer settings: %s", func() string {
-			b, err := json.Marshal(dbSettings)
-			if err != nil {
-				return err.Error()
-			}
-			return string(b)
-		}())
+		log.WithField("version", dbSettings.Version).
+			WithField("proposerKeys", len(dbSettings.ProposerConfig)).
+			Debug("Loaded proposer settings from DB")
 	}
 
 	// start to process based on load method
@@ -186,18 +181,6 @@ func (psl *SettingsLoader) Load(cliCtx *cli.Context) (*proposer.Settings, error)
 	return ps, nil
 }
 
-func hasBuilderShape(p *validatorpb.ProposerSettingsPayload) bool {
-	if p.DefaultConfig != nil && p.DefaultConfig.Builder != nil {
-		return true
-	}
-	for _, o := range p.ProposerConfig {
-		if o != nil && o.Builder != nil {
-			return true
-		}
-	}
-	return false
-}
-
 func (psl *SettingsLoader) applyOverrides() {
 	if psl.options.builderConfig != nil && psl.options.gasLimit != nil {
 		psl.options.builderConfig.GasLimit = *psl.options.gasLimit
@@ -268,13 +251,22 @@ func (psl *SettingsLoader) processProposerSettings(loadedSettings, dbSettings *v
 // through Builder; v2 lives on Option directly.
 func mergeProposerSettings(loaded, db *validatorpb.ProposerSettingsPayload, options *flagOptions) *validatorpb.ProposerSettingsPayload {
 	merged := &validatorpb.ProposerSettingsPayload{}
+	// The schema version never regresses: the highest version wins and the
+	// lower-version side's content is promoted before merging.
 	if db != nil {
 		merged.Version = db.Version
 	}
-	// v1-shaped source content must not inherit DB's v2, else the runtime upgrade is skipped.
-	if loaded != nil && (loaded.Version != 0 || hasBuilderShape(loaded)) {
+	if loaded != nil && loaded.Version > merged.Version {
 		merged.Version = loaded.Version
 	}
+	if merged.Version == proposer.SchemaV2 {
+		if db != nil && db.Version < proposer.SchemaV2 {
+			promotePayloadToV2(db)
+		}
+		if loaded != nil && loaded.Version < proposer.SchemaV2 {
+			promotePayloadToV2(loaded)
+		}
+	}
 
 	var builderConfig *validatorpb.BuilderConfig
 	var gasLimitOnly *validator.Uint64
@@ -286,11 +278,40 @@ func mergeProposerSettings(loaded, db *validatorpb.ProposerSettingsPayload, opti
 	}
 
 	if merged.Version == proposer.SchemaV2 {
-		return mergeProposerSettingsV2(merged, loaded, db, gasLimitOnly)
+		return mergeProposerSettingsV2(merged, loaded, db, builderConfig, gasLimitOnly)
 	}
 	return mergeProposerSettingsV1(merged, loaded, db, builderConfig, gasLimitOnly)
 }
 
+// promotePayloadToV2 mirrors Settings.UpgradeToV2 plus legacy presence
+// normalization, so v1 content merged into a v2 result reads correctly.
+func promotePayloadToV2(p *validatorpb.ProposerSettingsPayload) {
+	promote := func(opt *validatorpb.ProposerOptionPayload) {
+		if opt == nil || opt.Builder == nil {
+			return
+		}
+		if opt.GasLimit == 0 {
+			opt.GasLimit = opt.Builder.GasLimit
+		}
+	}
+	promote(p.DefaultConfig)
+	for _, opt := range p.ProposerConfig {
+		promote(opt)
+	}
+}
+
+// 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.
+func selectProposerConfig(db, loaded *validatorpb.ProposerSettingsPayload) map[string]*validatorpb.ProposerOptionPayload {
+	if loaded != nil && len(loaded.ProposerConfig) > 0 {
+		return loaded.ProposerConfig
+	}
+	if db != nil && len(db.ProposerConfig) > 0 {
+		return db.ProposerConfig
+	}
+	return nil
+}
+
 func mergeProposerSettingsV1(merged, loaded, db *validatorpb.ProposerSettingsPayload, builderConfig *validatorpb.BuilderConfig, gasLimitOnly *validator.Uint64) *validatorpb.ProposerSettingsPayload {
 	stripDBBuilder := builderConfig == nil
 
@@ -304,17 +325,12 @@ func mergeProposerSettingsV1(merged, loaded, db *validatorpb.ProposerSettingsPay
 		merged.DefaultConfig = loaded.DefaultConfig
 	}
 
-	if db != nil && len(db.ProposerConfig) > 0 {
-		merged.ProposerConfig = db.ProposerConfig
-		if stripDBBuilder {
-			for _, option := range db.ProposerConfig {
-				option.Builder = nil
-			}
+	if db != nil && stripDBBuilder {
+		for _, option := range db.ProposerConfig {
+			option.Builder = nil
 		}
 	}
-	if loaded != nil && len(loaded.ProposerConfig) > 0 {
-		merged.ProposerConfig = loaded.ProposerConfig
-	}
+	merged.ProposerConfig = selectProposerConfig(db, loaded)
 
 	if merged.DefaultConfig != nil {
 		merged.DefaultConfig.Builder = processBuilderConfig(merged.DefaultConfig.Builder, builderConfig, gasLimitOnly)
@@ -331,25 +347,33 @@ func mergeProposerSettingsV1(merged, loaded, db *validatorpb.ProposerSettingsPay
 			merged.DefaultConfig = &validatorpb.ProposerOptionPayload{Builder: builderConfig}
 		case gasLimitOnly != nil:
 			merged.DefaultConfig = &validatorpb.ProposerOptionPayload{
-				Builder: &validatorpb.BuilderConfig{Enabled: false, GasLimit: *gasLimitOnly},
+				Builder: &validatorpb.BuilderConfig{GasLimit: *gasLimitOnly},
 			}
 		}
 	}
 	return merged
 }
 
-func mergeProposerSettingsV2(merged, loaded, db *validatorpb.ProposerSettingsPayload, gasLimitOnly *validator.Uint64) *validatorpb.ProposerSettingsPayload {
+func mergeProposerSettingsV2(merged, loaded, db *validatorpb.ProposerSettingsPayload, builderConfig *validatorpb.BuilderConfig, gasLimitOnly *validator.Uint64) *validatorpb.ProposerSettingsPayload {
 	if db != nil && db.DefaultConfig != nil {
 		merged.DefaultConfig = db.DefaultConfig
 	}
 	if loaded != nil && loaded.DefaultConfig != nil {
 		merged.DefaultConfig = loaded.DefaultConfig
 	}
-	if db != nil && len(db.ProposerConfig) > 0 {
-		merged.ProposerConfig = db.ProposerConfig
-	}
-	if loaded != nil && len(loaded.ProposerConfig) > 0 {
-		merged.ProposerConfig = loaded.ProposerConfig
+	merged.ProposerConfig = selectProposerConfig(db, loaded)
+
+	// --enable-builder forces the default toggle on, matching v1; per-key
+	// enabled is untouched so a single key can still opt out (keymanager #88).
+	if builderConfig != nil {
+		if merged.DefaultConfig == nil {
+			merged.DefaultConfig = &validatorpb.ProposerOptionPayload{}
+		}
+		if merged.DefaultConfig.Builder == nil {
+			merged.DefaultConfig.Builder = &validatorpb.BuilderConfig{}
+		}
+		enabled := true
+		merged.DefaultConfig.Builder.Enabled = &enabled
 	}
 
 	if gasLimitOnly == nil {
@@ -383,7 +407,7 @@ func processBuilderConfig(current *validatorpb.BuilderConfig, override *validato
 		return override
 	}
 	if gasLimitOnly != nil {
-		return &validatorpb.BuilderConfig{Enabled: false, GasLimit: *gasLimitOnly}
+		return &validatorpb.BuilderConfig{GasLimit: *gasLimitOnly}
 	}
 	return nil
 }