removed sleep before getPayload by removing goroutine in buildPayload

eth/catalyst/api.go

@@ -169,6 +169,7 @@ func NewConsensusAPI(eth *eth.Ethereum) *ConsensusAPI {
 // If there are payloadAttributes: we try to assemble a block with the payloadAttributes
 // and return its payloadID.
 func (api *ConsensusAPI) ForkchoiceUpdatedV1(update engine.ForkchoiceStateV1, payloadAttributes *engine.PayloadAttributes) (engine.ForkChoiceResponse, error) {
+	log.Info("ForkchoiceUpdatedV1 called")
 	if payloadAttributes != nil {
 		if payloadAttributes.Withdrawals != nil {
 			return engine.STATUS_INVALID, engine.InvalidParams.With(fmt.Errorf("withdrawals not supported in V1"))
@@ -399,6 +400,7 @@ func (api *ConsensusAPI) ExchangeTransitionConfigurationV1(config engine.Transit
 
 // GetPayloadV1 returns a cached payload by id.
 func (api *ConsensusAPI) GetPayloadV1(payloadID engine.PayloadID) (*engine.ExecutableData, error) {
+	log.Info("GetPayloadV1 called")
 	data, err := api.getPayload(payloadID)
 	if err != nil {
 		return nil, err

grpc/execution/server.go

@@ -44,7 +44,7 @@ func NewExecutionServiceServer(eth *eth.Ethereum) *ExecutionServiceServer {
 }
 
 func (s *ExecutionServiceServer) DoBlock(ctx context.Context, req *executionv1.DoBlockRequest) (*executionv1.DoBlockResponse, error) {
-	log.Info("DoBlock called request [sam version]", "request", req)
+	log.Info("DoBlock called request", "request", req)
 	prevHeadHash := common.BytesToHash(req.PrevBlockHash)
 
 	// The Engine API has been modified to use transactions from this mempool and abide by it's ordering.
@@ -61,17 +61,14 @@ func (s *ExecutionServiceServer) DoBlock(ctx context.Context, req *executionv1.D
 		Random:                common.Hash{},
 		SuggestedFeeRecipient: common.Address{},
 	}
-	// NOTE: ForkchoiceUpdatedV1 calls forkchoiceUpdated. forkchoiceUpdated calls api.forkchoiceLock.Lock()
+
-	// what is this doing that requires us to wait?
 	fcStartResp, err := s.consensus.ForkchoiceUpdatedV1(*startForkChoice, payloadAttributes)
 	if err != nil {
 		return nil, err
 	}
 
-	// super janky but this is what the payload builder requires :/ (miner.worker.buildPayload())
+	// TODO: we should probably just execute + store the block directly instead of using the engine api.
-	// we should probably just execute + store the block directly instead of using the engine api.
+	payloadResp, err := s.consensus.GetPayloadV1(*fcStartResp.PayloadID)
-	// time.Sleep(time.Second)
-	payloadResp, err := s.consensus.GetPayloadV1(*fcStartResp.PayloadID) // this looks like it is totally fine
 	if err != nil {
 		log.Error("failed to call GetPayloadV1", "err", err)
 		return nil, err

miner/payload_building.go

@@ -163,36 +163,31 @@ func (w *worker) buildPayload(args *BuildPayloadArgs) (*Payload, error) {
 	// Construct a payload object for return.
 	payload := newPayload(empty, args.Id())
 
-	// Spin up a routine for updating the payload in background. This strategy
+	// Setup the timer for re-building the payload. The initial clock is kept
-	// can maximum the revenue for including transactions with highest fee.
+	// for triggering process immediately.
-	go func() {
+	timer := time.NewTimer(0)
-		// Setup the timer for re-building the payload. The initial clock is kept
+	defer timer.Stop()
-		// for triggering process immediately.
-		timer := time.NewTimer(0)
-		defer timer.Stop()
 
-		// Setup the timer for terminating the process if SECONDS_PER_SLOT (12s in
+	// Setup the timer for terminating the process if payload building exceeds 5 seconds.
-		// the Mainnet configuration) have passed since the point in time identified
+	// This should be much longer than normal payload building should take.
-		// by the timestamp parameter.
+	endTimer := time.NewTimer(time.Second * 5)
-		endTimer := time.NewTimer(time.Second * 12)
 
-		for {
+	// TODO: figure out if timeout case can be removed
-			select {
+	for {
-			case <-timer.C:
+		select {
-				start := time.Now()
+		case <-timer.C:
-				block, fees, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, false)
+			start := time.Now()
-				if err == nil {
+			block, fees, err := w.getSealingBlock(args.Parent, args.Timestamp, args.FeeRecipient, args.Random, args.Withdrawals, false)
-					payload.update(block, fees, time.Since(start))
+			if err == nil {
-				}
+				payload.update(block, fees, time.Since(start))
-				timer.Reset(w.recommit)
-			case <-payload.stop:
-				log.Info("Stopping work on payload", "id", payload.id, "reason", "delivery")
-				return
-			case <-endTimer.C:
-				log.Info("Stopping work on payload", "id", payload.id, "reason", "timeout")
-				return
 			}
+			timer.Reset(w.recommit)
+		case <-payload.stop:
+			log.Info("Stopping work on payload", "id", payload.id, "reason", "delivery")
+			return payload, nil
+		case <-endTimer.C:
+			log.Info("Stopping work on payload", "id", payload.id, "reason", "timeout")
+			return payload, nil
 		}
-	}()
+	}
-	return payload, nil
 }