adding builder boost factor to get block v3 #13409

james-prysm · 2024-01-03T17:03:29Z

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Implements the query parameter builder_boost_factor used for [/eth/v3/validator/blocks/{slot}](https://ethereum.github.io/beacon-APIs/?urls.primaryName=dev#/Validator/produceBlockV3)

description from beacon apis

Percentage multiplier to apply to the builder's payload value when choosing between a builder payload header and payload from the paired execution node. This parameter is only relevant if the beacon node is connected to a builder, deems it safe to produce a builder payload, and receives valid responses from both the builder endpoint and the paired execution node. When these preconditions are met, the server MUST act as follows:

if exec_node_payload_value >= builder_boost_factor * (builder_payload_value // 100), then return a full (unblinded) block containing the execution node payload.
otherwise, return a blinded block containing the builder payload header.
Servers must support the following values of the boost factor which encode common preferences:

builder_boost_factor=0: prefer the execution node payload unless an error makes it unviable.
builder_boost_factor=100: default profit maximization mode; choose whichever payload pays more.
builder_boost_factor=2**64 - 1: prefer the builder payload unless an error or beacon node health check makes it unviable.
Servers should use saturating arithmetic or another technique to ensure that large values of the builder_boost_factor do not trigger overflows or errors. If this parameter is provided and the beacon node is not configured with a builder then the beacon node MUST respond with a full block, which the caller can choose to reject if it wishes. If this parameter is not provided then it should be treated as having the default value of 100. If the value is provided but out of range for a 64-bit unsigned integer, then an error response with status code 400 MUST be returned.

Which issues(s) does this PR fix?

ethereum/beacon-APIs#386, ethereum/beacon-APIs#398

Other notes for review

potuz · 2024-01-03T18:01:25Z

beacon-chain/rpc/eth/validator/handlers_block.go

+	switch trimmed {
+	case "": // default to 100 if it's not provided
+		return 100, nil
+	case "2**64-1":


Lol I thought that the user would have to specify explicitly 18446744073709551615

potuz · 2024-01-03T18:06:22Z

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go

@@ -80,9 +80,9 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc
 		}

 		// Use builder payload if the following in true:
-		// builder_bid_value * 100 > local_block_value * (local-block-value-boost + 100)
+		// builder_bid_value * builderBoostFactor(default 100) > local_block_value * (local-block-value-boost + 100)


Hmmm I don't like this. The point of builderBoostFactor is that it replaces our current parameter local-block-value-boost not to use both... I'm not sure what the right approach is here but at the very least we should log if there's a request and the local boost is set to non-zero and the boost factor to non default

I added a log for now we'll see if others have an opinion

rkapka · 2024-01-04T12:55:19Z

proto/prysm/v1alpha1/validator.proto

@@ -566,6 +567,9 @@ message BlockRequest {

    // Signal server to skip outsourcing block request from mev-boost/relayer so that returned block will always be a local block.
    bool skip_mev_boost = 4;
+
+    // Percentage multiplier to apply to the builder's payload value when choosing between a builder payload header and payload from the paired execution node
+    google.protobuf.UInt64Value builder_boost_factor = 5;


Wait, why isn't this simply uint64?

I wanted to keep it nullable since 0 is a valid value and we don't have defaults in protobuf3. this way i can check if it was set or not and add a default

rkapka · 2024-01-04T13:03:13Z

beacon-chain/rpc/eth/validator/handlers_block.go

 	}, any)
 }

+func processBuilderBoostFactor(raw string) (*wrapperspb.UInt64Value, error) {


The spec defines the boost factor as Uint64. There is no need for this function, you can just call shared.UintFromQuery

I need it as nullable, i'll doublecheck sharedUintFromQuery though

rkapka · 2024-01-04T13:09:34Z

beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go

@@ -96,7 +97,7 @@ func TestServer_setExecutionData(t *testing.T) {
 		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))
+		require.NoError(t, setExecutionData(context.Background(), blk, localPayload, builderPayload, builderKzgCommitments, 100))


Please use the constant default value here and everywhere else

james-prysm and others added 6 commits January 3, 2024 10:56

adding builder boost factor to functions

5509187

gaz

a510a79

Merge branch 'develop' into builder-boost-factor

66b1351

fixing linting

55aa6bb

fixing unit tests

130d9fd

gaz

299ab1e

james-prysm added the API Api related tasks label Jan 3, 2024

james-prysm marked this pull request as ready for review January 3, 2024 17:55

james-prysm requested a review from a team as a code owner January 3, 2024 17:55

james-prysm requested review from kasey, rauljordan, prestonvanloon, potuz and terencechain and removed request for kasey January 3, 2024 17:55

james-prysm added the Ready For Review A pull request ready for code review label Jan 3, 2024

potuz reviewed Jan 3, 2024

View reviewed changes

addressing review comments

c4e4606

james-prysm changed the title ~~adding builder boost factor to functions~~ adding builder boost factor to get block v3 Jan 3, 2024

james-prysm and others added 2 commits January 3, 2024 13:26

fixing tests

ef0218e

Merge branch 'develop' into builder-boost-factor

b01ade2

rkapka reviewed Jan 4, 2024

View reviewed changes

james-prysm and others added 6 commits January 4, 2024 08:13

Merge branch 'develop' into builder-boost-factor

35f2aff

addressing review feedback

7bbe222

gaz

b542ca8

Merge branch 'develop' into builder-boost-factor

36609ea

changing log based on review

2cfe4bb

Merge branch 'develop' into builder-boost-factor

43f1b24

rkapka approved these changes Jan 4, 2024

View reviewed changes

james-prysm enabled auto-merge January 4, 2024 17:18

james-prysm added this pull request to the merge queue Jan 4, 2024

Merged via the queue into develop with commit d439e6d Jan 4, 2024
17 checks passed

james-prysm deleted the builder-boost-factor branch January 4, 2024 17:46

james-prysm mentioned this pull request Jan 19, 2024

adding builder boost factor to proposer settings #13422

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding builder boost factor to get block v3 #13409

adding builder boost factor to get block v3 #13409

james-prysm commented Jan 3, 2024 •

edited

Loading

potuz Jan 3, 2024

potuz Jan 3, 2024 •

edited

Loading

james-prysm Jan 3, 2024

rkapka Jan 4, 2024

james-prysm Jan 4, 2024

rkapka Jan 4, 2024

james-prysm Jan 4, 2024

rkapka Jan 4, 2024

adding builder boost factor to get block v3 #13409

adding builder boost factor to get block v3 #13409

Conversation

james-prysm commented Jan 3, 2024 • edited Loading

potuz Jan 3, 2024

Choose a reason for hiding this comment

potuz Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

james-prysm Jan 3, 2024

Choose a reason for hiding this comment

rkapka Jan 4, 2024

Choose a reason for hiding this comment

james-prysm Jan 4, 2024

Choose a reason for hiding this comment

rkapka Jan 4, 2024

Choose a reason for hiding this comment

james-prysm Jan 4, 2024

Choose a reason for hiding this comment

rkapka Jan 4, 2024

Choose a reason for hiding this comment

james-prysm commented Jan 3, 2024 •

edited

Loading

potuz Jan 3, 2024 •

edited

Loading