[code health] refactor redis update to independent function (#514)

* refactor redis update

* remove stale comment

* rename

Author: michaelneuder

services/api/service.go

@@ -1649,6 +1649,50 @@ func (api *RelayAPI) checkFloorBidValue(opts bidFloorOpts) (*big.Int, *logrus.En
 	return floorBidValue, opts.log, true
 }
 
+type redisUpdateBidOpts struct {
+	w                    http.ResponseWriter
+	tx                   redis.Pipeliner
+	log                  *logrus.Entry
+	cancellationsEnabled bool
+	receivedAt           time.Time
+	floorBidValue        *big.Int
+	payload              *common.BuilderSubmitBlockRequest
+}
+
+func (api *RelayAPI) updateRedisBid(opts redisUpdateBidOpts) (*datastore.SaveBidAndUpdateTopBidResponse, *common.GetPayloadResponse, bool) {
+	// Prepare the response data
+	getHeaderResponse, err := common.BuildGetHeaderResponse(opts.payload, api.blsSk, api.publicKey, api.opts.EthNetDetails.DomainBuilder)
+	if err != nil {
+		opts.log.WithError(err).Error("could not sign builder bid")
+		api.RespondError(opts.w, http.StatusBadRequest, err.Error())
+		return nil, nil, false
+	}
+
+	getPayloadResponse, err := common.BuildGetPayloadResponse(opts.payload)
+	if err != nil {
+		opts.log.WithError(err).Error("could not build getPayload response")
+		api.RespondError(opts.w, http.StatusBadRequest, err.Error())
+		return nil, nil, false
+	}
+
+	bidTrace := common.BidTraceV2{
+		BidTrace:    *opts.payload.Message(),
+		BlockNumber: opts.payload.BlockNumber(),
+		NumTx:       uint64(opts.payload.NumTx()),
+	}
+
+	//
+	// Save to Redis
+	//
+	updateBidResult, err := api.redis.SaveBidAndUpdateTopBid(context.Background(), opts.tx, &bidTrace, opts.payload, getPayloadResponse, getHeaderResponse, opts.receivedAt, opts.cancellationsEnabled, opts.floorBidValue)
+	if err != nil {
+		opts.log.WithError(err).Error("could not save bid and update top bids")
+		api.RespondError(opts.w, http.StatusInternalServerError, "failed saving and updating bid")
+		return nil, nil, false
+	}
+	return &updateBidResult, getPayloadResponse, true
+}
+
 func (api *RelayAPI) handleSubmitNewBlock(w http.ResponseWriter, req *http.Request) {
 	var pf common.Profile
 	var prevTime, nextTime time.Time
@@ -1953,34 +1997,17 @@ func (api *RelayAPI) handleSubmitNewBlock(w http.ResponseWriter, req *http.Reque
 		}
 	}
 
-	// Prepare the response data
-	getHeaderResponse, err := common.BuildGetHeaderResponse(payload, api.blsSk, api.publicKey, api.opts.EthNetDetails.DomainBuilder)
-	if err != nil {
-		log.WithError(err).Error("could not sign builder bid")
-		api.RespondError(w, http.StatusBadRequest, err.Error())
-		return
-	}
-
-	getPayloadResponse, err := common.BuildGetPayloadResponse(payload)
-	if err != nil {
-		log.WithError(err).Error("could not build getPayload response")
-		api.RespondError(w, http.StatusBadRequest, err.Error())
-		return
-	}
-
-	bidTrace := common.BidTraceV2{
-		BidTrace:    *payload.Message(),
-		BlockNumber: payload.BlockNumber(),
-		NumTx:       uint64(payload.NumTx()),
+	redisOpts := redisUpdateBidOpts{
+		w:                    w,
+		tx:                   tx,
+		log:                  log,
+		cancellationsEnabled: isCancellationEnabled,
+		receivedAt:           receivedAt,
+		floorBidValue:        floorBidValue,
+		payload:              payload,
 	}
-
-	//
-	// Save to Redis
-	//
-	updateBidResult, err := api.redis.SaveBidAndUpdateTopBid(context.Background(), tx, &bidTrace, payload, getPayloadResponse, getHeaderResponse, receivedAt, isCancellationEnabled, floorBidValue)
-	if err != nil {
-		log.WithError(err).Error("could not save bid and update top bids")
-		api.RespondError(w, http.StatusInternalServerError, "failed saving and updating bid")
+	updateBidResult, getPayloadResponse, ok := api.updateRedisBid(redisOpts)
+	if !ok {
 		return
 	}
 

services/api/service_test.go

@@ -947,6 +947,80 @@ func TestCheckFloorBidValue(t *testing.T) {
 	}
 }
 
+func TestUpdateRedis(t *testing.T) {
+	cases := []struct {
+		description          string
+		cancellationsEnabled bool
+		floorValue           string
+		payload              *common.BuilderSubmitBlockRequest
+		expectOk             bool
+	}{
+		{
+			description: "success",
+			floorValue:  "10",
+			payload: &common.BuilderSubmitBlockRequest{
+				Capella: &builderCapella.SubmitBlockRequest{
+					Message: &v1.BidTrace{
+						Slot:  testSlot,
+						Value: uint256.NewInt(1),
+					},
+					ExecutionPayload: &capella.ExecutionPayload{},
+				},
+			},
+			expectOk: true,
+		},
+		{
+			description: "failure_no_payload",
+			floorValue:  "10",
+			payload:     nil,
+			expectOk:    false,
+		},
+		{
+			description: "failure_encode_failure_too_long_extra_data",
+			floorValue:  "10",
+			payload: &common.BuilderSubmitBlockRequest{
+				Capella: &builderCapella.SubmitBlockRequest{
+					Message: &v1.BidTrace{
+						Slot:  testSlot,
+						Value: uint256.NewInt(1),
+					},
+					ExecutionPayload: &capella.ExecutionPayload{
+						ExtraData: make([]byte, 33), // Max extra data length is 32.
+					},
+				},
+			},
+			expectOk: false,
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.description, func(t *testing.T) {
+			_, _, backend := startTestBackend(t)
+			w := httptest.NewRecorder()
+			logger := logrus.New()
+			log := logrus.NewEntry(logger)
+			tx := backend.redis.NewTxPipeline()
+
+			floorValue := new(big.Int)
+			floorValue, ok := floorValue.SetString(tc.floorValue, 10)
+			require.True(t, ok)
+			rOpts := redisUpdateBidOpts{
+				w:                    w,
+				tx:                   tx,
+				log:                  log,
+				cancellationsEnabled: tc.cancellationsEnabled,
+				floorBidValue:        floorValue,
+				payload:              tc.payload,
+			}
+			updateResp, getPayloadResp, ok := backend.relay.updateRedisBid(rOpts)
+			require.Equal(t, tc.expectOk, ok)
+			if ok {
+				require.NotNil(t, updateResp)
+				require.NotNil(t, getPayloadResp)
+			}
+		})
+	}
+}
+
 func gzipBytes(t *testing.T, b []byte) []byte {
 	t.Helper()
 	var buf bytes.Buffer