Merge remote-tracking branch 'flashbots/flashbots' into extra-data

builder/builder.go

@@ -8,7 +8,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"math/big"
 	"net/http"
 	_ "os"
 	"strings"
@@ -41,15 +40,18 @@ type IBuilder interface {
 	handleGetPayload(w http.ResponseWriter, req *http.Request)
 }
 
+type IPayload interface {
+	ResolveFull() *engine.ExecutionPayloadEnvelope
+}
+
 type Builder struct {
 	eth                         IEthereumService
 	ignoreLatePayloadAttributes bool
 	beaconClient                IBeaconClient
 	builderPrivateKey           *ecdsa.PrivateKey
 	builderAddress              common.Address
 
-	builderRetryInterval time.Duration
-	builderBlockTime     time.Duration
+	builderBlockTime time.Duration
 
 	proposerAddress common.Address
 
@@ -58,8 +60,8 @@ type Builder struct {
 	slotCtx       context.Context
 	slotCtxCancel context.CancelFunc
 
-	bestBlockMu sync.Mutex
-	bestBlock   *builderTypes.VersionedBuilderPayloadResponse
+	payloadMu sync.Mutex
+	payload   IPayload
 
 	extraData []byte
 
@@ -71,24 +73,13 @@ type BuilderArgs struct {
 	builderPrivateKey           *ecdsa.PrivateKey
 	builderAddress              common.Address
 	proposerAddress             common.Address
-	builderRetryInterval        time.Duration
 	blockTime                   time.Duration
 	eth                         IEthereumService
 	ignoreLatePayloadAttributes bool
 	beaconClient                IBeaconClient
 	extraData                   []byte
 }
 
-// SubmitBlockOpts is a struct that contains all the arguments needed to submit a block to the relay
-type SubmitBlockOpts struct {
-	// ExecutablePayloadEnvelope is the payload envelope that was executed
-	ExecutionPayloadEnvelope *engine.ExecutionPayloadEnvelope
-	// SealedAt is the time at which the block was sealed
-	SealedAt time.Time
-	// PayloadAttributes are the payload attributes used for block building
-	PayloadAttributes *builderTypes.PayloadAttributes
-}
-
 func NewBuilder(args BuilderArgs) (*Builder, error) {
 	slotCtx, slotCtxCancel := context.WithCancel(context.Background())
 	return &Builder{
@@ -98,7 +89,6 @@ func NewBuilder(args BuilderArgs) (*Builder, error) {
 		builderPrivateKey:           args.builderPrivateKey,
 		builderAddress:              args.builderAddress,
 		proposerAddress:             args.proposerAddress,
-		builderRetryInterval:        args.builderRetryInterval,
 		builderBlockTime:            args.blockTime,
 
 		slotCtx:       slotCtx,
@@ -179,13 +169,10 @@ func (b *Builder) GetPayload(request *builderTypes.BuilderPayloadRequest) (*buil
 		}
 	}
 
-	b.bestBlockMu.Lock()
-	bestBlock := b.bestBlock
-	b.bestBlockMu.Unlock()
-
-	if bestBlock == nil {
-		log.Warn("no builder submissions")
-		return nil, ErrNoPayloads
+	bestBlock, err := b.getVersionedBlockSubmission(request.Message.Slot, request.Message.ParentHash)
+	if err != nil {
+		log.Warn("error getting versioned block submission", "err", err)
+		return nil, fmt.Errorf("error getting builder block: %w", err)
 	}
 
 	if bestBlock.Message.Slot != request.Message.Slot {
@@ -274,12 +261,24 @@ func (b *Builder) handleGetPayload(w http.ResponseWriter, req *http.Request) {
 	updateServeTimeHistogram("getPayload", true, time.Since(start))
 }
 
-func (b *Builder) saveBlockSubmission(opts SubmitBlockOpts) error {
-	executionPayload := opts.ExecutionPayloadEnvelope.ExecutionPayload
+func (b *Builder) getVersionedBlockSubmission(slot uint64, parentHash common.Hash) (*builderTypes.VersionedBuilderPayloadResponse, error) {
+	var executionPayloadEnvelope *engine.ExecutionPayloadEnvelope
+	b.payloadMu.Lock()
+	if b.payload != nil {
+		executionPayloadEnvelope = b.payload.ResolveFull()
+	}
+	b.payloadMu.Unlock()
+
+	if executionPayloadEnvelope == nil {
+		return nil, fmt.Errorf("no builder block found")
+	}
+
+	executionPayload := executionPayloadEnvelope.ExecutionPayload
+
 	log.Info(
 		"saveBlockSubmission",
-		"slot", opts.PayloadAttributes.Slot,
-		"parent", opts.PayloadAttributes.HeadHash.String(),
+		"slot", slot,
+		"parent", parentHash.String(),
 		"hash", executionPayload.BlockHash.String(),
 	)
 
@@ -292,26 +291,26 @@ func (b *Builder) saveBlockSubmission(opts SubmitBlockOpts) error {
 		dataVersion = builderTypes.SpecVersionEcotone
 	}
 
-	value, overflow := uint256.FromBig(opts.ExecutionPayloadEnvelope.BlockValue)
+	value, overflow := uint256.FromBig(executionPayloadEnvelope.BlockValue)
 	if overflow {
-		return fmt.Errorf("could not set block value due to value overflow")
+		return nil, fmt.Errorf("could not set block value due to value overflow")
 	}
 
 	blockBidMsg := builderTypes.BidTrace{
-		Slot:                 opts.PayloadAttributes.Slot,
+		Slot:                 slot,
 		ParentHash:           executionPayload.ParentHash,
 		BlockHash:            executionPayload.BlockHash,
 		BuilderAddress:       b.builderAddress,
 		ProposerAddress:      b.proposerAddress,
-		ProposerFeeRecipient: opts.PayloadAttributes.SuggestedFeeRecipient,
+		ProposerFeeRecipient: executionPayload.FeeRecipient,
 		GasLimit:             executionPayload.GasLimit,
 		GasUsed:              executionPayload.GasUsed,
 		Value:                value.ToBig(),
 	}
 
 	signature, err := b.signBuilderBid(&blockBidMsg)
 	if err != nil {
-		return fmt.Errorf("could not sign block bid message, %w", err)
+		return nil, fmt.Errorf("could not sign block bid message, %w", err)
 	}
 
 	versionedBlockRequest := &builderTypes.VersionedBuilderPayloadResponse{
@@ -321,14 +320,10 @@ func (b *Builder) saveBlockSubmission(opts SubmitBlockOpts) error {
 		Signature:        signature,
 	}
 
-	b.bestBlockMu.Lock()
-	b.bestBlock = versionedBlockRequest
-	b.bestBlockMu.Unlock()
-
-	log.Info("saved block", "version", dataVersion.String(), "slot", opts.PayloadAttributes.Slot, "value", opts.ExecutionPayloadEnvelope.BlockValue.String(),
+	log.Info("resolved block", "version", dataVersion.String(), "slot", slot, "value", executionPayloadEnvelope.BlockValue,
 		"parent", executionPayload.ParentHash.String(), "hash", executionPayload.BlockHash)
 
-	return nil
+	return versionedBlockRequest, nil
 }
 
 func (b *Builder) signBuilderBid(bid *builderTypes.BidTrace) ([]byte, error) {
@@ -386,49 +381,14 @@ func (b *Builder) handlePayloadAttributes(attrs *builderTypes.PayloadAttributes)
 }
 
 func (b *Builder) runBuildingJob(slotCtx context.Context, attrs *builderTypes.PayloadAttributes) {
-	ctx, cancel := context.WithTimeout(slotCtx, b.builderBlockTime)
-	defer cancel()
-
-	// Submission queue for the given payload attributes
-	// multiple jobs can run for different attributes fot the given slot
-	// 1. When new block is ready we check if its profit is higher than profit of last best block
-	//    if it is we set queueBest* to values of the new block and notify queueSignal channel.
-	var (
-		queueMu                sync.Mutex
-		queueLastSubmittedHash common.Hash
-		queueBestBlockValue    *big.Int = big.NewInt(0)
-	)
-
 	log.Info("runBuildingJob", "slot", attrs.Slot, "parent", attrs.HeadHash, "payloadTimestamp", uint64(attrs.Timestamp), "txs", attrs.Transactions)
 
-	// retry build block every builderBlockRetryInterval
-	runRetryLoop(ctx, b.builderRetryInterval, func() {
-		log.Info("retrying BuildBlock",
-			"slot", attrs.Slot,
-			"parent", attrs.HeadHash,
-			"retryInterval", b.builderRetryInterval.String())
-		payload, err := b.eth.BuildBlock(attrs)
-		if err != nil {
-			log.Warn("Failed to build block", "err", err)
-			return
-		}
-
-		sealedAt := time.Now()
-		queueMu.Lock()
-		defer queueMu.Unlock()
-		if payload.ExecutionPayload.BlockHash != queueLastSubmittedHash && payload.BlockValue.Cmp(queueBestBlockValue) >= 0 {
-			queueLastSubmittedHash = payload.ExecutionPayload.BlockHash
-			queueBestBlockValue = payload.BlockValue
-
-			submitBlockOpts := SubmitBlockOpts{
-				ExecutionPayloadEnvelope: payload,
-				SealedAt:                 sealedAt,
-				PayloadAttributes:        attrs,
-			}
-			err := b.saveBlockSubmission(submitBlockOpts)
-			if err != nil {
-				log.Error("could not save block submission", "err", err)
-			}
-		}
-	})
+	payload, err := b.eth.BuildBlock(attrs)
+	if err != nil {
+		log.Warn("Failed to build block", "err", err)
+		return
+	}
+	b.payloadMu.Lock()
+	b.payload = payload
+	b.payloadMu.Unlock()
 }

builder/builder_test.go

@@ -17,14 +17,24 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+type testPayloadService struct {
+	testExecutableData *engine.ExecutionPayloadEnvelope
+}
+
+func (t *testPayloadService) ResolveFull() *engine.ExecutionPayloadEnvelope {
+	return t.testExecutableData
+}
+
 type testEthereumService struct {
 	synced             bool
 	testExecutableData *engine.ExecutionPayloadEnvelope
 	testBlock          *types.Block
 }
 
-func (t *testEthereumService) BuildBlock(attrs *builderTypes.PayloadAttributes) (*engine.ExecutionPayloadEnvelope, error) {
-	return t.testExecutableData, nil
+func (t *testEthereumService) BuildBlock(attrs *builderTypes.PayloadAttributes) (IPayload, error) {
+	return &testPayloadService{
+		testExecutableData: t.testExecutableData,
+	}, nil
 }
 
 func (t *testEthereumService) GetBlockByHash(hash common.Hash) *types.Block { return t.testBlock }
@@ -104,7 +114,6 @@ func TestGetPayloadV1(t *testing.T) {
 		builderPrivateKey:           testPrivateKey,
 		builderAddress:              crypto.PubkeyToAddress(testPrivateKey.PublicKey),
 		proposerAddress:             crypto.PubkeyToAddress(testPrivateKey.PublicKey),
-		builderRetryInterval:        200 * time.Millisecond,
 		blockTime:                   2 * time.Second,
 		eth:                         testEthService,
 		ignoreLatePayloadAttributes: false,

builder/eth_service.go

@@ -1,21 +1,16 @@
 package builder
 
 import (
-	"errors"
-	"time"
-
-	"github.com/ethereum/go-ethereum/beacon/engine"
 	builderTypes "github.com/ethereum/go-ethereum/builder/types"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/eth"
-	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/miner"
 	"github.com/ethereum/go-ethereum/params"
 )
 
 type IEthereumService interface {
-	BuildBlock(attrs *builderTypes.PayloadAttributes) (*engine.ExecutionPayloadEnvelope, error)
+	BuildBlock(attrs *builderTypes.PayloadAttributes) (IPayload, error)
 	GetBlockByHash(hash common.Hash) *types.Block
 	Config() *params.ChainConfig
 	Synced() bool
@@ -33,7 +28,7 @@ func NewEthereumService(eth *eth.Ethereum, config *Config) *EthereumService {
 	}
 }
 
-func (s *EthereumService) BuildBlock(attrs *builderTypes.PayloadAttributes) (*engine.ExecutionPayloadEnvelope, error) {
+func (s *EthereumService) BuildBlock(attrs *builderTypes.PayloadAttributes) (IPayload, error) {
 	// Send a request to generate a full block in the background.
 	// The result can be obtained via the returned channel.
 	args := &miner.BuildPayloadArgs{
@@ -48,31 +43,7 @@ func (s *EthereumService) BuildBlock(attrs *builderTypes.PayloadAttributes) (*en
 		NoTxPool:     attrs.NoTxPool,
 	}
 
-	payload, err := s.eth.Miner().BuildPayloadWithExtraData(args, attrs.ExtraData)
-	if err != nil {
-		log.Error("Failed to build payload", "err", err)
-		return nil, err
-	}
-
-	resCh := make(chan *engine.ExecutionPayloadEnvelope, 1)
-	go func() {
-		resCh <- payload.ResolveFull()
-	}()
-
-	timer := time.NewTimer(s.cfg.BlockTime)
-	defer timer.Stop()
-
-	select {
-	case payload := <-resCh:
-		if payload == nil {
-			return nil, errors.New("received nil payload from sealing work")
-		}
-		return payload, nil
-	case <-timer.C:
-		payload.Cancel()
-		log.Error("timeout waiting for block", "parent hash", attrs.HeadHash, "slot", attrs.Slot)
-		return nil, errors.New("timeout waiting for block result")
-	}
+	return s.eth.Miner().BuildPayloadWithExtraData(args, attrs.ExtraData)
 }
 
 func (s *EthereumService) GetBlockByHash(hash common.Hash) *types.Block {

builder/eth_service_test.go

@@ -93,7 +93,9 @@ func TestBuildBlock(t *testing.T) {
 
 	service := NewEthereumService(ethservice, &DefaultConfig)
 
-	executableData, err := service.BuildBlock(testPayloadAttributes)
+	payload, err := service.BuildBlock(testPayloadAttributes)
+
+	executableData := payload.ResolveFull()
 
 	require.Equal(t, common.Address{0x04, 0x10}, executableData.ExecutionPayload.FeeRecipient)
 	require.Equal(t, common.Hash{0x05, 0x10}, executableData.ExecutionPayload.Random)

builder/service.go

@@ -4,7 +4,6 @@ import (
 	"crypto/ecdsa"
 	"fmt"
 	"net/http"
-	"time"
 
 	builderTypes "github.com/ethereum/go-ethereum/builder/types"
 	"github.com/ethereum/go-ethereum/common"
@@ -75,17 +74,6 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg *Config) error {
 
 	ethereumService := NewEthereumService(backend, cfg)
 
-	var builderRetryInterval time.Duration
-	if cfg.RetryInterval != "" {
-		d, err := time.ParseDuration(cfg.RetryInterval)
-		if err != nil {
-			return fmt.Errorf("error parsing builder retry interval - %v", err)
-		}
-		builderRetryInterval = d
-	} else {
-		builderRetryInterval = RetryIntervalDefault
-	}
-
 	builderPrivateKey, err := crypto.HexToECDSA(cfg.BuilderSigningKey)
 	if err != nil {
 		return fmt.Errorf("invalid builder private key: %w", err)
@@ -109,7 +97,6 @@ func Register(stack *node.Node, backend *eth.Ethereum, cfg *Config) error {
 		builderAddress:              builderAddress,
 		proposerAddress:             proposerAddress,
 		eth:                         ethereumService,
-		builderRetryInterval:        builderRetryInterval,
 		ignoreLatePayloadAttributes: cfg.IgnoreLatePayloadAttributes,
 		beaconClient:                beaconClient,
 		blockTime:                   cfg.BlockTime,