Skip to content
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

Closed
wants to merge 39 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
8ce7287
wip
james-prysm Jan 5, 2024
ba73f17
proposer settings wip
james-prysm Jan 8, 2024
6741223
Merge branch 'develop' into builder-boost-settings
james-prysm Jan 18, 2024
5d50550
adding builder boost factor to builder settings
james-prysm Jan 18, 2024
86bc39d
adding tests for builder boost
james-prysm Jan 19, 2024
91c9bc4
Merge branch 'develop' into builder-boost-settings
james-prysm Jan 19, 2024
c880301
adding another unit test and gaz
james-prysm Jan 19, 2024
f3eb41d
Merge branch 'develop' into builder-boost-settings
james-prysm Jan 19, 2024
414a463
Merge branch 'develop' into builder-boost-settings
james-prysm Jan 22, 2024
e9b6193
fixing getBeaconBlock rest API
james-prysm Jan 22, 2024
8749e54
fixing linting
james-prysm Jan 22, 2024
26ba7aa
Merge branch 'develop' into builder-boost-settings
james-prysm Jan 22, 2024
36ade7b
Merge branch 'develop' into builder-boost-settings
james-prysm Feb 14, 2024
4e430a5
fixing some conflicts but wip to fix branch
james-prysm Mar 21, 2024
ad942a8
fixing branch
james-prysm Mar 21, 2024
996192f
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 21, 2024
75038e4
fixing linting, and wip adding a builder boost factor flag
james-prysm Mar 21, 2024
ae3be20
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 21, 2024
2291865
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 22, 2024
bea4620
fixing logic and adding tests
james-prysm Mar 22, 2024
b0be7ca
renaming test
james-prysm Mar 22, 2024
1bf7b44
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 22, 2024
f8c4a94
adding more tests
james-prysm Mar 22, 2024
5492018
Update config/proposer/settings.go
james-prysm Mar 25, 2024
bdb2158
Update config/proposer/settings.go
james-prysm Mar 25, 2024
421f967
Update validator/client/propose.go
james-prysm Mar 25, 2024
8aa5f73
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 25, 2024
062518b
addressing comments
james-prysm Mar 25, 2024
c8181ad
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 25, 2024
3086d0e
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 26, 2024
ee5703d
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 26, 2024
041b077
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 28, 2024
4680af1
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 29, 2024
fd7df55
ignore builder boost if local boost is set
james-prysm Mar 29, 2024
749cb98
adding test based on review change
james-prysm Mar 29, 2024
099d393
Merge branch 'develop' into builder-boost-settings
james-prysm Mar 29, 2024
85772bf
small update to use the builder boost if it's below the default of 100
james-prysm Apr 1, 2024
fb09cc5
Merge branch 'develop' into builder-boost-settings
james-prysm Apr 1, 2024
719c4aa
Merge branch 'develop' into builder-boost-settings
james-prysm Apr 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,16 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc
// Use builder payload if the following in true:
// builder_bid_value * builderBoostFactor(default 100) > local_block_value * (local-block-value-boost + 100)
boost := params.BeaconConfig().LocalBlockValueBoost
higherValueBuilder := builderValueGwei*builderBoostFactor > localValueGwei*(100+boost)
if boost > 0 && builderBoostFactor != defaultBuilderBoostFactor {
if boost > 0 && builderBoostFactor > defaultBuilderBoostFactor {
log.WithFields(logrus.Fields{
"localGweiValue": localValueGwei,
"localBoostPercentage": boost,
"builderGweiValue": builderValueGwei,
"builderBoostFactor": builderBoostFactor,
}).Warn("Proposer: both local boost and builder boost are using non default values")
}).Warn("Proposer: builder boost will be ignored, because local boost is activated and builder boost is above the default")
builderBoostFactor = defaultBuilderBoostFactor
}
higherValueBuilder := builderValueGwei*builderBoostFactor > localValueGwei*(100+boost)
builderValueGweiGauge.Set(float64(builderValueGwei))
localValueGweiGauge.Set(float64(localValueGwei))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func TestServer_setExecutionData(t *testing.T) {
require.NoError(t, err)
require.Equal(t, uint64(2), e.BlockNumber()) // builder block
})
t.Run("Builder builder has higher value but forced to local payload with builder boost factor", func(t *testing.T) {
t.Run("Builder has higher value but forced to local payload with builder boost factor at 0", func(t *testing.T) {
blk, err := blocks.NewSignedBeaconBlock(util.NewBlindedBeaconBlockCapella())
require.NoError(t, err)
require.NoError(t, vs.BeaconDB.SaveRegistrationsByValidatorIDs(ctx, []primitives.ValidatorIndex{blk.Block().ProposerIndex()},
Expand Down Expand Up @@ -370,6 +370,23 @@ func TestServer_setExecutionData(t *testing.T) {

require.LogsContain(t, hook, "builderGweiValue=1 localBoostPercentage=0 localGweiValue=2")
})
t.Run("Builder configured. Local block has higher value even with higher builder boost factor", func(t *testing.T) {
blk, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella())
require.NoError(t, err)
vs.ExecutionEngineCaller = &powtesting.EngineClient{PayloadIDBytes: id, ExecutionPayloadCapella: &v1.ExecutionPayloadCapella{BlockNumber: 3}, BlockValue: 2 * 1e9}
b := blk.Block()
localPayload, _, err := vs.getLocalPayload(ctx, b, capellaTransitionState)
require.NoError(t, err)
builderPayload, builderKzgCommitments, err := vs.getBuilderPayloadAndBlobs(ctx, b.Slot(), b.ProposerIndex())
require.NoError(t, err)
require.DeepEqual(t, [][]uint8(nil), builderKzgCommitments)
require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, 99999))
e, err := blk.Block().Body().Execution()
require.NoError(t, err)
require.Equal(t, uint64(3), e.BlockNumber()) // Local block

require.LogsContain(t, hook, "builderGweiValue=1 localBoostPercentage=0 localGweiValue=2")
})
t.Run("Builder configured. Local block and local boost has higher value", func(t *testing.T) {
cfg := params.BeaconConfig().Copy()
cfg.LocalBlockValueBoost = 1 // Boost 1%.
Expand Down
4 changes: 2 additions & 2 deletions cmd/beacon-chain/flags/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ var (
// LocalBlockValueBoost sets a percentage boost for local block construction while using a custom builder.
LocalBlockValueBoost = &cli.Uint64Flag{
Name: "local-block-value-boost",
Usage: "A percentage boost for local block construction as a Uint64. This is used to prioritize local block construction over relay/builder block construction" +
"Boost is an additional percentage to multiple local block value. Use builder block if: builder_bid_value * 100 > local_block_value * (local-block-value-boost + 100)",
Usage: "A percentage boost for local block construction as a Uint64. This is used to prioritize local block construction over relay/builder block construction. " +
"Boost is an additional percentage to multiple local block value. Use builder block if: builder_value * builder-boost-factor > local_value * (local-block-value-boost + 100)",
Value: 10,
}
// ExecutionEngineEndpoint provides an HTTP access endpoint to connect to an execution client on the execution layer
Expand Down
8 changes: 8 additions & 0 deletions cmd/validator/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,14 @@ var (
Value: fmt.Sprint(params.BeaconConfig().DefaultBuilderGasLimit),
}

// BuilderBoostFactorFlag sets a multiplier for builder block construction as a Uint64.
BuilderBoostFactorFlag = &cli.Uint64Flag{
Name: "builder-boost-factor",
Usage: "A multiplier for builder block construction as a Uint64. This is used to prioritize relay/builder block construction over local block construction. " +
"Boost factor is a multiplier against the builder value. Use builder block if: builder_value * builder-boost-factor > local_value * (local-block-value-boost + 100)",
Value: 100,
}

// ValidatorsRegistrationBatchSizeFlag sets the maximum size for one batch of validator registrations. Use a non-positive value to disable batching.
ValidatorsRegistrationBatchSizeFlag = &cli.IntFlag{
Name: "validators-registration-batch-size",
Expand Down
1 change: 1 addition & 0 deletions cmd/validator/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ var appFlags = []cli.Flag{
flags.ProposerSettingsFlag,
flags.EnableBuilderFlag,
flags.BuilderGasLimitFlag,
flags.BuilderBoostFactorFlag,
flags.ValidatorsRegistrationBatchSizeFlag,
////////////////////
cmd.DisableMonitoringFlag,
Expand Down
1 change: 1 addition & 0 deletions cmd/validator/usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ var appHelpFlagGroups = []flagGroup{
flags.SuggestedFeeRecipientFlag,
flags.EnableBuilderFlag,
flags.BuilderGasLimitFlag,
flags.BuilderBoostFactorFlag,
flags.ValidatorsRegistrationBatchSizeFlag,
flags.EnableDistributed,
},
Expand Down
83 changes: 53 additions & 30 deletions config/proposer/loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
Expand All @@ -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)
}
}

Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not test covered.

Copy link
Contributor Author

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

}
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will return nil if both options.builderConfig and current are nil. Is this OK?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not test covered.

}

return current
}

func reviewGasLimit(gasLimit validator.Uint64) validator.Uint64 {
Expand Down
Loading
Loading