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 get block v3 #13409

Merged
merged 15 commits into from
Jan 4, 2024
5 changes: 4 additions & 1 deletion beacon-chain/rpc/eth/shared/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ go_library(

go_test(
name = "go_default_test",
srcs = ["errors_test.go"],
srcs = [
"errors_test.go",
"request_test.go",
],
embed = [":go_default_library"],
deps = [
"//beacon-chain/rpc/lookup:go_default_library",
Expand Down
9 changes: 5 additions & 4 deletions beacon-chain/rpc/eth/shared/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"strconv"
"strings"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/gorilla/mux"
Expand All @@ -15,15 +16,15 @@ import (
)

func UintFromQuery(w http.ResponseWriter, r *http.Request, name string, required bool) (string, uint64, bool) {
raw := r.URL.Query().Get(name)
if raw == "" && !required {
trimmed := strings.ReplaceAll(r.URL.Query().Get(name), " ", "")
if trimmed == "" && !required {
return "", 0, true
}
v, valid := ValidateUint(w, name, raw)
v, valid := ValidateUint(w, name, trimmed)
if !valid {
return "", 0, false
}
return raw, v, true
return trimmed, v, true
}

func UintFromRoute(w http.ResponseWriter, r *http.Request, name string) (string, uint64, bool) {
Expand Down
96 changes: 96 additions & 0 deletions beacon-chain/rpc/eth/shared/request_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package shared

import (
"math"
"net/http/httptest"
"testing"
)

func TestUintFromQuery_BuilderBoostFactor(t *testing.T) {
type args struct {
raw string
}
tests := []struct {
name string
args args
wantRaw string
wantValue uint64
wantOK bool
}{
{
name: "builder boost factor of 0 returns 0",
args: args{
raw: "0",
},
wantRaw: "0",
wantValue: 0,
wantOK: true,
},
{
name: "builder boost factor of the default, 100 returns 100",
args: args{raw: "100"},
wantRaw: "100",
wantValue: 100,
wantOK: true,
},
{
name: "builder boost factor max uint64 returns max uint64",
args: args{raw: "18446744073709551615"},
wantRaw: "18446744073709551615",
wantValue: math.MaxUint64,
wantOK: true,
},
{
name: "builder boost factor as a percentage returns error",
args: args{raw: "0.30"},
wantRaw: "",
wantValue: 0,
wantOK: false,
},
{
name: "builder boost factor negative int returns error",
args: args{raw: "-100"},
wantRaw: "",
wantValue: 0,
wantOK: false,
},
{
name: "builder boost factor max uint64 +1 returns error",
args: args{raw: "18446744073709551616"},
wantRaw: "",
wantValue: 0,
wantOK: false,
},
{
name: "builder boost factor of invalid string returns error",
args: args{raw: "asdf"},
wantRaw: "",
wantValue: 0,
wantOK: false,
},
{
name: "builder boost factor of number bigger than uint64 string returns error",
args: args{raw: "9871398721983721908372190837219837129803721983719283798217390821739081273918273918273918273981273982139812739821"},
wantRaw: "",
wantValue: 0,
wantOK: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
query := "builder_boost_factor"
bbreq := httptest.NewRequest("GET", "/eth/v3/validator/blocks/{slot}?builder_boost_factor="+tt.args.raw, nil)
w := httptest.NewRecorder()
got, got1, got2 := UintFromQuery(w, bbreq, query, false)
if got != tt.wantRaw {
t.Errorf("UintFromQuery() got = %v, wantRaw %v", got, tt.wantRaw)
}
if got1 != tt.wantValue {
t.Errorf("UintFromQuery() got1 = %v, wantRaw %v", got1, tt.wantValue)
}
if got2 != tt.wantOK {
t.Errorf("UintFromQuery() got2 = %v, wantRaw %v", got2, tt.wantOK)
}
})
}
}
1 change: 1 addition & 0 deletions beacon-chain/rpc/eth/validator/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ go_library(
"@io_opencensus_go//trace:go_default_library",
"@org_golang_google_grpc//codes:go_default_library",
"@org_golang_google_grpc//status:go_default_library",
"@org_golang_google_protobuf//types/known/wrapperspb:go_default_library",
],
)

Expand Down
19 changes: 15 additions & 4 deletions beacon-chain/rpc/eth/validator/handlers_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
eth "github.com/prysmaticlabs/prysm/v4/proto/prysm/v1alpha1"
"github.com/prysmaticlabs/prysm/v4/runtime/version"
"go.opencensus.io/trace"
"google.golang.org/protobuf/types/known/wrapperspb"
)

type blockType uint8
Expand Down Expand Up @@ -155,6 +156,15 @@ func (s *Server) ProduceBlockV3(w http.ResponseWriter, r *http.Request) {
rawGraffiti := r.URL.Query().Get("graffiti")
rawSkipRandaoVerification := r.URL.Query().Get("skip_randao_verification")

var bbFactor *wrapperspb.UInt64Value // default the factor via fall back
rawBbFactor, bbValue, ok := shared.UintFromQuery(w, r, "builder_boost_factor", false)
if !ok {
return
}
if rawBbFactor != "" {
bbFactor = &wrapperspb.UInt64Value{Value: bbValue}
}

slot, valid := shared.ValidateUint(w, "slot", rawSlot)
if !valid {
return
Expand Down Expand Up @@ -182,10 +192,11 @@ func (s *Server) ProduceBlockV3(w http.ResponseWriter, r *http.Request) {
}

s.produceBlockV3(ctx, w, r, &eth.BlockRequest{
Slot: primitives.Slot(slot),
RandaoReveal: randaoReveal,
Graffiti: graffiti,
SkipMevBoost: false,
Slot: primitives.Slot(slot),
RandaoReveal: randaoReveal,
Graffiti: graffiti,
SkipMevBoost: false,
BuilderBoostFactor: bbFactor,
}, any)
}

Expand Down
14 changes: 10 additions & 4 deletions beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ import (
var eth1DataNotification bool

const (
eth1dataTimeout = 2 * time.Second
eth1dataTimeout = 2 * time.Second
defaultBuilderBoostFactor = uint64(100)
)

// GetBeaconBlock is called by a proposer during its assigned slot to request a block to sign
Expand Down Expand Up @@ -108,7 +109,12 @@ func (vs *Server) GetBeaconBlock(ctx context.Context, req *ethpb.BlockRequest) (
}
sBlk.SetProposerIndex(idx)

if err = vs.BuildBlockParallel(ctx, sBlk, head, req.SkipMevBoost); err != nil {
builderBoostFactor := defaultBuilderBoostFactor
if req.BuilderBoostFactor != nil {
builderBoostFactor = req.BuilderBoostFactor.Value
}

if err = vs.BuildBlockParallel(ctx, sBlk, head, req.SkipMevBoost, builderBoostFactor); err != nil {
return nil, errors.Wrap(err, "could not build block in parallel")
}

Expand All @@ -128,7 +134,7 @@ func (vs *Server) GetBeaconBlock(ctx context.Context, req *ethpb.BlockRequest) (
return vs.constructGenericBeaconBlock(sBlk, bundleCache.get(req.Slot))
}

func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool) error {
func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.SignedBeaconBlock, head state.BeaconState, skipMevBoost bool, builderBoostFactor uint64) error {
// Build consensus fields in background
var wg sync.WaitGroup
wg.Add(1)
Expand Down Expand Up @@ -186,7 +192,7 @@ func (vs *Server) BuildBlockParallel(ctx context.Context, sBlk interfaces.Signed
}
}

if err := setExecutionData(ctx, sBlk, localPayload, builderPayload, builderKzgCommitments); err != nil {
if err := setExecutionData(ctx, sBlk, localPayload, builderPayload, builderKzgCommitments, builderBoostFactor); err != nil {
return status.Errorf(codes.Internal, "Could not set execution data: %v", err)
}

Expand Down
22 changes: 16 additions & 6 deletions beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var emptyTransactionsRoot = [32]byte{127, 254, 36, 30, 166, 1, 135, 253, 176, 24
const blockBuilderTimeout = 1 * time.Second

// Sets the execution data for the block. Execution data can come from local EL client or remote builder depends on validator registration and circuit breaker conditions.
func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, localPayload, builderPayload interfaces.ExecutionData, builderKzgCommitments [][]byte) error {
func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, localPayload, builderPayload interfaces.ExecutionData, builderKzgCommitments [][]byte, builderBoostFactor uint64) error {
_, span := trace.StartSpan(ctx, "ProposerServer.setExecutionData")
defer span.End()

Expand Down Expand Up @@ -80,9 +80,17 @@ 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)
Copy link
Contributor

@potuz potuz Jan 3, 2024

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

boost := params.BeaconConfig().LocalBlockValueBoost
higherValueBuilder := builderValueGwei*100 > localValueGwei*(100+boost)
higherValueBuilder := builderValueGwei*builderBoostFactor > localValueGwei*(100+boost)
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")
}

// If we can't get the builder value, just use local block.
if higherValueBuilder && withdrawalsMatched { // Builder value is higher and withdrawals match.
Expand All @@ -98,13 +106,15 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc
"localGweiValue": localValueGwei,
"localBoostPercentage": boost,
"builderGweiValue": builderValueGwei,
"builderBoostFactor": builderBoostFactor,
}).Warn("Proposer: using local execution payload because higher value")
}
span.AddAttributes(
trace.BoolAttribute("higherValueBuilder", higherValueBuilder),
trace.Int64Attribute("localGweiValue", int64(localValueGwei)), // lint:ignore uintcast -- This is OK for tracing.
trace.Int64Attribute("localBoostPercentage", int64(boost)), // lint:ignore uintcast -- This is OK for tracing.
trace.Int64Attribute("builderGweiValue", int64(builderValueGwei)), // lint:ignore uintcast -- This is OK for tracing.
trace.Int64Attribute("localGweiValue", int64(localValueGwei)), // lint:ignore uintcast -- This is OK for tracing.
trace.Int64Attribute("localBoostPercentage", int64(boost)), // lint:ignore uintcast -- This is OK for tracing.
trace.Int64Attribute("builderGweiValue", int64(builderValueGwei)), // lint:ignore uintcast -- This is OK for tracing.
trace.Int64Attribute("builderBoostFactor", int64(builderBoostFactor)), // lint:ignore uintcast -- This is OK for tracing.
)
return setLocalExecution(blk, localPayload)
default: // Bellatrix case.
Expand Down
Loading
Loading