builder: only build on top of first payload attributes event as it should be canonical

.github/workflows/go.yml

@@ -26,7 +26,7 @@ jobs:
       uses: actions/checkout@v2
 
     - name: Test
-      run: go test ./core ./miner/... ./internal/ethapi/... ./les/...
+      run: go test ./core ./miner/... ./internal/ethapi/... ./les/... ./builder/... ./eth/block-validation/...
 
     - name: Build
       run: make geth

README.md

@@ -38,8 +38,9 @@ $ geth --help
     --builder                      (default: false)
           Enable the builder
 
-    --builder.beacon_endpoint value (default: "http://127.0.0.1:5052")
-          Beacon endpoint to connect to for beacon chain data [$BUILDER_BEACON_ENDPOINT]
+    --builder.beacon_endpoints value (default: "http://127.0.0.1:5052")
+          Comma separated list of beacon endpoints to connect to for beacon chain data
+          [$BUILDER_BEACON_ENDPOINTS]
 
     --builder.bellatrix_fork_version value (default: "0x02000000")
           Bellatrix fork version. [$BUILDER_BELLATRIX_FORK_VERSION]
@@ -53,6 +54,10 @@ $ geth --help
     --builder.genesis_validators_root value (default: "0x0000000000000000000000000000000000000000000000000000000000000000")
           Genesis validators root of the network. [$BUILDER_GENESIS_VALIDATORS_ROOT]
 
+    --builder.ignore_late_payload_attributes (default: false)
+          Builder will ignore all but the first payload attributes. Use if your CL sends
+          non-canonical head updates.
+
     --builder.listen_addr value    (default: ":28545")
           Listening address for builder endpoint [$BUILDER_LISTEN_ADDR]
 
@@ -74,11 +79,17 @@ $ geth --help
           missing from the primary remote relay, and to push blocks for registrations
           missing from or matching the primary [$BUILDER_SECONDARY_REMOTE_RELAY_ENDPOINTS]
 
+    --builder.seconds_in_slot value (default: 12)
+          Set the number of seconds in a slot in the local relay
+
     --builder.secret_key value     (default: "0x2fc12ae741f29701f8e30f5de6350766c020cb80768a0ff01e6838ffd2431e11")
           Builder key used for signing blocks [$BUILDER_SECRET_KEY]
 
-    --builder.validation_blacklist value (default: "")
-          Path to file containing blacklisted addresses, json-encoded list of strings.
+    --builder.slots_in_epoch value (default: 32)
+          Set the number of slots in an epoch in the local relay
+
+    --builder.validation_blacklist value
+          Path to file containing blacklisted addresses, json-encoded list of strings
 
     --builder.validator_checks     (default: false)
           Enable the validator checks

builder/beacon_client.go

@@ -109,11 +109,10 @@ func (m *MultiBeaconClient) getProposerForNextSlot(requestedSlot uint64) (Pubkey
 
 func payloadAttributesMatch(l types.BuilderPayloadAttributes, r types.BuilderPayloadAttributes) bool {
 	if l.Timestamp != r.Timestamp ||
-		l.Random != r.Random ||
-		l.SuggestedFeeRecipient != r.SuggestedFeeRecipient ||
 		l.Slot != r.Slot ||
-		l.HeadHash != r.HeadHash ||
-		l.GasLimit != r.GasLimit {
+		l.GasLimit != r.GasLimit ||
+		l.Random != r.Random ||
+		l.HeadHash != r.HeadHash {
 		return false
 	}
 

builder/builder.go

@@ -52,46 +52,46 @@ type IBuilder interface {
 }
 
 type Builder struct {
-	ds                   flashbotsextra.IDatabaseService
-	relay                IRelay
-	eth                  IEthereumService
-	dryRun               bool
-	validator            *blockvalidation.BlockValidationAPI
-	beaconClient         IBeaconClient
-	builderSecretKey     *bls.SecretKey
-	builderPublicKey     boostTypes.PublicKey
-	builderSigningDomain boostTypes.Domain
+	ds                          flashbotsextra.IDatabaseService
+	relay                       IRelay
+	eth                         IEthereumService
+	dryRun                      bool
+	ignoreLatePayloadAttributes bool
+	validator                   *blockvalidation.BlockValidationAPI
+	beaconClient                IBeaconClient
+	builderSecretKey            *bls.SecretKey
+	builderPublicKey            boostTypes.PublicKey
+	builderSigningDomain        boostTypes.Domain
 
 	limiter *rate.Limiter
 
 	slotMu        sync.Mutex
-	slot          uint64
-	slotAttrs     []types.BuilderPayloadAttributes
+	slotAttrs     types.BuilderPayloadAttributes
 	slotCtx       context.Context
 	slotCtxCancel context.CancelFunc
 
 	stop chan struct{}
 }
 
-func NewBuilder(sk *bls.SecretKey, ds flashbotsextra.IDatabaseService, relay IRelay, builderSigningDomain boostTypes.Domain, eth IEthereumService, dryRun bool, validator *blockvalidation.BlockValidationAPI, beaconClient IBeaconClient) *Builder {
+func NewBuilder(sk *bls.SecretKey, ds flashbotsextra.IDatabaseService, relay IRelay, builderSigningDomain boostTypes.Domain, eth IEthereumService, dryRun bool, ignoreLatePayloadAttributes bool, validator *blockvalidation.BlockValidationAPI, beaconClient IBeaconClient) *Builder {
 	pkBytes := bls.PublicKeyFromSecretKey(sk).Compress()
 	pk := boostTypes.PublicKey{}
 	pk.FromSlice(pkBytes)
 
 	slotCtx, slotCtxCancel := context.WithCancel(context.Background())
 	return &Builder{
-		ds:                   ds,
-		relay:                relay,
-		eth:                  eth,
-		dryRun:               dryRun,
-		validator:            validator,
-		beaconClient:         beaconClient,
-		builderSecretKey:     sk,
-		builderPublicKey:     pk,
-		builderSigningDomain: builderSigningDomain,
+		ds:                          ds,
+		relay:                       relay,
+		eth:                         eth,
+		dryRun:                      dryRun,
+		ignoreLatePayloadAttributes: ignoreLatePayloadAttributes,
+		validator:                   validator,
+		beaconClient:                beaconClient,
+		builderSecretKey:            sk,
+		builderPublicKey:            pk,
+		builderSigningDomain:        builderSigningDomain,
 
 		limiter:       rate.NewLimiter(rate.Every(time.Millisecond), 510),
-		slot:          0,
 		slotCtx:       slotCtx,
 		slotCtxCancel: slotCtxCancel,
 
@@ -116,7 +116,10 @@ func (b *Builder) Start() error {
 				if payloadAttributes.Slot < currentSlot {
 					continue
 				} else if payloadAttributes.Slot == currentSlot {
-					b.OnPayloadAttribute(&payloadAttributes)
+					// Subsequent sse events should only be canonical!
+					if !b.ignoreLatePayloadAttributes {
+						b.OnPayloadAttribute(&payloadAttributes)
+					}
 				} else if payloadAttributes.Slot > currentSlot {
 					currentSlot = payloadAttributes.Slot
 					b.OnPayloadAttribute(&payloadAttributes)
@@ -302,27 +305,20 @@ func (b *Builder) OnPayloadAttribute(attrs *types.BuilderPayloadAttributes) erro
 	b.slotMu.Lock()
 	defer b.slotMu.Unlock()
 
-	// Forcibly cancel previous building job, build on top of reorgable blocks as this is the behaviour relays expect.
-	// This will change in the future
+	if attrs.Equal(&b.slotAttrs) {
+		log.Debug("ignoring known payload attribute", "slot", attrs.Slot, "hash", attrs.HeadHash)
+		return nil
+	}
 
 	if b.slotCtxCancel != nil {
 		b.slotCtxCancel()
 	}
 
 	slotCtx, slotCtxCancel := context.WithTimeout(context.Background(), 12*time.Second)
-	b.slot = attrs.Slot
-	b.slotAttrs = nil
+	b.slotAttrs = *attrs
 	b.slotCtx = slotCtx
 	b.slotCtxCancel = slotCtxCancel
 
-	for _, currentAttrs := range b.slotAttrs {
-		if attrs.Equal(&currentAttrs) {
-			log.Debug("ignoring known payload attribute", "slot", attrs.Slot, "hash", attrs.HeadHash)
-			return nil
-		}
-	}
-	b.slotAttrs = append(b.slotAttrs, *attrs)
-
 	go b.runBuildingJob(b.slotCtx, proposerPubkey, vd, attrs)
 	return nil
 }

builder/builder_test.go

@@ -73,7 +73,7 @@ func TestOnPayloadAttributes(t *testing.T) {
 
 	testEthService := &testEthereumService{synced: true, testExecutableData: testExecutableData, testBlock: testBlock, testBlockValue: big.NewInt(10)}
 
-	builder := NewBuilder(sk, flashbotsextra.NilDbService{}, &testRelay, bDomain, testEthService, false, nil, &testBeacon)
+	builder := NewBuilder(sk, flashbotsextra.NilDbService{}, &testRelay, bDomain, testEthService, false, false, nil, &testBeacon)
 	builder.Start()
 	defer builder.Stop()
 

builder/config.go

@@ -8,6 +8,7 @@ type Config struct {
 	SecondsInSlot                 uint64   `toml:",omitempty"`
 	DisableBundleFetcher          bool     `toml:",omitempty"`
 	DryRun                        bool     `toml:",omitempty"`
+	IgnoreLatePayloadAttributes   bool     `toml:",omitempty"`
 	BuilderSecretKey              string   `toml:",omitempty"`
 	RelaySecretKey                string   `toml:",omitempty"`
 	ListenAddr                    string   `toml:",omitempty"`
@@ -29,6 +30,7 @@ var DefaultConfig = Config{
 	SecondsInSlot:                 12,
 	DisableBundleFetcher:          false,
 	DryRun:                        false,
+	IgnoreLatePayloadAttributes:   false,
 	BuilderSecretKey:              "0x2fc12ae741f29701f8e30f5de6350766c020cb80768a0ff01e6838ffd2431e11",
 	RelaySecretKey:                "0x2fc12ae741f29701f8e30f5de6350766c020cb80768a0ff01e6838ffd2431e11",
 	ListenAddr:                    ":28545",

builder/local_relay_test.go

@@ -32,7 +32,7 @@ func newTestBackend(t *testing.T, forkchoiceData *engine.ExecutableData, block *
 	beaconClient := &testBeaconClient{validator: validator}
 	localRelay := NewLocalRelay(sk, beaconClient, bDomain, cDomain, ForkData{}, true)
 	ethService := &testEthereumService{synced: true, testExecutableData: forkchoiceData, testBlock: block, testBlockValue: blockValue}
-	backend := NewBuilder(sk, flashbotsextra.NilDbService{}, localRelay, bDomain, ethService, false, nil, beaconClient)
+	backend := NewBuilder(sk, flashbotsextra.NilDbService{}, localRelay, bDomain, ethService, false, false, nil, beaconClient)
 	// service := NewService("127.0.0.1:31545", backend)
 
 	backend.limiter = rate.NewLimiter(rate.Inf, 0)

builder/service.go

@@ -203,7 +203,7 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg *Config) error {
 		return errors.New("incorrect builder API secret key provided")
 	}
 
-	builderBackend := NewBuilder(builderSk, ds, relay, builderSigningDomain, ethereumService, cfg.DryRun, validator, beaconClient)
+	builderBackend := NewBuilder(builderSk, ds, relay, builderSigningDomain, ethereumService, cfg.DryRun, cfg.IgnoreLatePayloadAttributes, validator, beaconClient)
 	builderService := NewService(cfg.ListenAddr, localRelay, builderBackend)
 
 	stack.RegisterAPIs([]rpc.API{

cmd/geth/main.go

@@ -165,6 +165,7 @@ var (
 		utils.BuilderSlotsInEpoch,
 		utils.BuilderDisableBundleFetcher,
 		utils.BuilderDryRun,
+		utils.BuilderIgnoreLatePayloadAttributes,
 		utils.BuilderSecretKey,
 		utils.BuilderRelaySecretKey,
 		utils.BuilderListenAddr,

cmd/utils/flags.go

@@ -736,6 +736,11 @@ var (
 		Usage:    "Builder only validates blocks without submission to the relay",
 		Category: flags.BuilderCategory,
 	}
+	BuilderIgnoreLatePayloadAttributes = &cli.BoolFlag{
+		Name:     "builder.ignore_late_payload_attributes",
+		Usage:    "Builder will ignore all but the first payload attributes. Use if your CL sends non-canonical head updates.",
+		Category: flags.BuilderCategory,
+	}
 	BuilderSecretKey = &cli.StringFlag{
 		Name:     "builder.secret_key",
 		Usage:    "Builder key used for signing blocks",
@@ -1602,6 +1607,7 @@ func SetBuilderConfig(ctx *cli.Context, cfg *builder.Config) {
 	cfg.SecondsInSlot = ctx.Uint64(BuilderSecondsInSlot.Name)
 	cfg.DisableBundleFetcher = ctx.IsSet(BuilderDisableBundleFetcher.Name)
 	cfg.DryRun = ctx.IsSet(BuilderDryRun.Name)
+	cfg.IgnoreLatePayloadAttributes = ctx.IsSet(BuilderIgnoreLatePayloadAttributes.Name)
 	cfg.BuilderSecretKey = ctx.String(BuilderSecretKey.Name)
 	cfg.RelaySecretKey = ctx.String(BuilderRelaySecretKey.Name)
 	cfg.ListenAddr = ctx.String(BuilderListenAddr.Name)

eth/block-validation/api_test.go

@@ -2,6 +2,7 @@ package blockvalidation
 
 import (
 	"encoding/json"
+	"errors"
 	"io/ioutil"
 	"math/big"
 	"os"
@@ -166,6 +167,7 @@ func TestValidateBuilderSubmissionV1(t *testing.T) {
 
 func TestValidateBuilderSubmissionV2(t *testing.T) {
 	genesis, preMergeBlocks := generatePreMergeChain(20)
+	os.Setenv("BUILDER_TX_SIGNING_KEY", "0x28c3cd61b687fdd03488e167a5d84f50269df2a4c29a2cfb1390903aa775c5d0")
 	time := preMergeBlocks[len(preMergeBlocks)-1].Time() + 5
 	genesis.Config.ShanghaiTime = &time
 	n, ethservice := startEthService(t, genesis, preMergeBlocks)
@@ -175,6 +177,8 @@ func TestValidateBuilderSubmissionV2(t *testing.T) {
 	api := NewBlockValidationAPI(ethservice, nil)
 	parent := preMergeBlocks[len(preMergeBlocks)-1]
 
+	api.eth.APIBackend.Miner().SetEtherbase(testValidatorAddr)
+
 	// This EVM code generates a log when the contract is created.
 	logCode := common.Hex2Bytes("60606040525b7f24ec1d3ff24c2f6ff210738839dbc339cd45a5294d85c79361016243157aae7b60405180905060405180910390a15b600a8060416000396000f360606040526008565b00")
 
@@ -224,18 +228,19 @@ func TestValidateBuilderSubmissionV2(t *testing.T) {
 	// require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "withdrawals mismatch")
 
 	execData, err := assembleBlock(api, parent.Hash(), &engine.PayloadAttributes{
-		Timestamp:   parent.Time() + 5,
-		Withdrawals: withdrawals,
+		Timestamp:             parent.Time() + 5,
+		Withdrawals:           withdrawals,
+		SuggestedFeeRecipient: testValidatorAddr,
 	})
 	require.NoError(t, err)
 	require.EqualValues(t, len(execData.Withdrawals), 2)
-	require.EqualValues(t, len(execData.Transactions), 3)
+	require.EqualValues(t, len(execData.Transactions), 4)
 
 	payload, err := ExecutableDataToExecutionPayloadV2(execData)
 	require.NoError(t, err)
 
 	proposerAddr := bellatrix.ExecutionAddress{}
-	copy(proposerAddr[:], testAddr.Bytes())
+	copy(proposerAddr[:], testValidatorAddr.Bytes())
 
 	blockRequest = &BuilderBlockValidationRequestV2{
 		SubmitBlockRequest: capellaapi.SubmitBlockRequest{
@@ -255,7 +260,7 @@ func TestValidateBuilderSubmissionV2(t *testing.T) {
 	}
 
 	require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "inaccurate payment")
-	blockRequest.Message.Value = uint256.NewInt(10)
+	blockRequest.Message.Value = uint256.NewInt(149842511727212)
 	require.NoError(t, api.ValidateBuilderSubmissionV2(blockRequest))
 
 	blockRequest.Message.GasLimit += 1
@@ -288,7 +293,7 @@ func TestValidateBuilderSubmissionV2(t *testing.T) {
 	api.accessVerifier = nil
 
 	blockRequest.Message.GasUsed = 10
-	require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "incorrect GasUsed 10, expected 98996")
+	require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "incorrect GasUsed 10, expected 119996")
 	blockRequest.Message.GasUsed = execData.GasUsed
 
 	newTestKey, _ := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f290")
@@ -306,8 +311,7 @@ func TestValidateBuilderSubmissionV2(t *testing.T) {
 	copy(invalidPayload.ReceiptsRoot[:], hexutil.MustDecode("0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421")[:32])
 	blockRequest.ExecutionPayload = invalidPayload
 	updatePayloadHashV2(t, blockRequest)
-	require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "could not apply tx 3", "insufficient funds for gas * price + value")
-
+	require.ErrorContains(t, api.ValidateBuilderSubmissionV2(blockRequest), "could not apply tx 4", "insufficient funds for gas * price + value")
 }
 
 func updatePayloadHash(t *testing.T, blockRequest *BuilderBlockValidationRequest) {
@@ -399,7 +403,12 @@ func assembleBlock(api *BlockValidationAPI, parentHash common.Hash, params *engi
 	if err != nil {
 		return nil, err
 	}
-	return payload.ResolveFull().ExecutionPayload, nil
+
+	if payload := payload.ResolveFull(); payload != nil {
+		return payload.ExecutionPayload, nil
+	}
+
+	return nil, errors.New("payload did not resolve")
 }
 
 func TestBlacklistLoad(t *testing.T) {

miner/multi_worker.go

@@ -130,6 +130,9 @@ func (w *multiWorker) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 			block, fees, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.GasLimit, args.Random, args.Withdrawals, false, args.BlockHook)
 			if err == nil {
 				workerPayload.update(block, fees, time.Since(start))
+			} else {
+				log.Error("Error while sealing block", "err", err)
+				workerPayload.Cancel()
 			}
 		}(w)
 	}