Code refactoring and minor improvement

server/url_params.go

@@ -11,17 +11,49 @@ import (
 	"github.com/pkg/errors"
 )
 
+const (
+	// Query parameters
+	ParamHint     = "hint"
+	ParamOriginId = "originid"
+	ParamBuilder  = "builder"
+	ParamRefund   = "refund"
+
+	// Maximum length of the originId query parameter
+	MaxOriginIdLength = 255
+
+	// Maximum and minimum values for the refund percentage
+	MaxRefundPercent = 100
+	MinRefundPercent = 0
+)
+
 var (
 	DefaultAuctionHint = []string{"hash", "special_logs"}
 
-	ErrEmptyHintQuery                      = errors.New("Hint query must be non-empty if set.")
-	ErrEmptyTargetBuilderQuery             = errors.New("Target builder query must be non-empty if set.")
-	ErrIncorrectAuctionHints               = errors.New("Incorrect auction hint, must be one of: contract_address, function_selector, logs, calldata, default_logs.")
-	ErrIncorrectOriginId                   = errors.New("Incorrect origin id, must be less then 255 char.")
-	ErrIncorrectRefundQuery                = errors.New("Incorrect refund query, must be 0xaddress:percentage.")
-	ErrIncorrectRefundAddressQuery         = errors.New("Incorrect refund address.")
-	ErrIncorrectRefundPercentageQuery      = errors.New("Incorrect refund percentage.")
-	ErrIncorrectRefundTotalPercentageQuery = errors.New("Incorrect refund total percentage, must be bellow 100%.")
+	validHints = map[string]struct{}{
+		"hash":              {},
+		"contract_address":  {},
+		"function_selector": {},
+		"logs":              {},
+		"calldata":          {},
+		"default_logs":      {},
+	}
+
+	ErrEmptyHintQuery          = errors.New("Hint query must be non-empty if set.")
+	ErrEmptyTargetBuilderQuery = errors.New("Target builder query must be non-empty if set.")
+	ErrIncorrectAuctionHints   = func(hint string) error {
+		return fmt.Errorf("incorrect auction hint: %s, must be one of: contract_address, function_selector, logs, calldata, default_logs", hint)
+	}
+	ErrIncorrectOriginId = func(originId string) error {
+		return fmt.Errorf("incorrect origin id: %s, must be non-empty and less than 255 characters", originId)
+	}
+	ErrIncorrectRefundQuery = func(refund string) error {
+		return fmt.Errorf("incorrect refund query: %s, must be 0xaddress:percentage", refund)
+	}
+	ErrIncorrectRefundAddressQuery         = func(address string) error { return fmt.Errorf("incorrect refund address: %s", address) }
+	ErrIncorrectRefundPercentageQuery      = func(percent string) error { return fmt.Errorf("incorrect refund percentage: %s", percent) }
+	ErrIncorrectRefundTotalPercentageQuery = func(total string) error {
+		return fmt.Errorf("incorrect refund total percentage: %s, must be below 100%%", total)
+	}
 )
 
 type URLParameters struct {
@@ -42,111 +74,145 @@ func normalizeQueryParams(url *url.URL) map[string][]string {
 	return normalizedQuery
 }
 
-// ExtractParametersFromUrl extracts the auction preference from the url query
-// Allowed query params:
-//   - hint: mev share hints, can be set multiple times, default: hash, special_logs
-//   - originId: origin id, default: ""
-//   - builder: target builder, can be set multiple times, default: empty (only send to flashbots builders)
-//   - refund: refund in the form of 0xaddress:percentage, default: empty (will be set by default when backrun is produced)
-//     example: 0x123:80 - will refund 80% of the backrun profit to 0x123
+// ExtractParametersFromUrl parses the URL query parameters to extract auction preferences.
+//
+// Supported query parameters include:
+//   - hint: Specifies MEV share hints. Accepts multiple values. Valid options are:
+//     "hash", "contract_address", "function_selector", "logs", "calldata", "default_logs".
+//     Default: ["hash", "special_logs"].
+//   - originId: Identifies the origin ID. Default: "" (empty string).
+//   - builder: Targets specific builders. Accepts multiple values.
+//     Default: [] (targets only Flashbots builders).
+//   - refund: Specifies refund details in the format '0xaddress:percentage'.
+//     Default: "" (set automatically to `tx.origin` when a backrun is produced).
+//     Example: "0x123:80" - Refunds 80% of the backrun profit to address 0x123.
+//
+// Errors returned:
+//   - ErrEmptyHintQuery if the hint query parameter is empty
+//   - ErrIncorrectAuctionHints if the hint is not one of the allowed values
+//   - ErrIncorrectOriginId if the originId is empty
+//   - ErrEmptyTargetBuilderQuery if the builder query parameter is empty
+//   - ErrIncorrectRefundQuery if the refund query parameter is malformed
+//   - ErrIncorrectRefundAddressQuery if the refund address is not a valid hex address
+//   - ErrIncorrectRefundPercentageQuery if the refund percentage is not a valid integer or out of allowed range
+//   - ErrIncorrectRefundTotalPercentageQuery if the total refund percentage exceeds 100%
 func ExtractParametersFromUrl(url *url.URL, allBuilders []string) (params URLParameters, err error) {
 	if strings.HasPrefix(url.Path, "/fast") {
 		params.fast = true
 	}
-	// Normalize all query parameters to lowercase keys
 	normalizedQuery := normalizeQueryParams(url)
+	params.pref.Privacy.Hints = DefaultAuctionHint
+
+	if err := extractHints(&params, normalizedQuery); err != nil {
+		return params, err
+	}
+	if err := extractOriginId(&params, normalizedQuery); err != nil {
+		return params, err
+	}
+	if err := extractBuilders(&params, normalizedQuery, allBuilders); err != nil {
+		return params, err
+	}
+	if err := extractRefund(&params, normalizedQuery); err != nil {
+		return params, err
+	}
 
-	var hint []string
-	hintQuery, ok := normalizedQuery["hint"]
-	if ok {
+	return params, nil
+}
+
+// extractHints sets the auction hints from the query parameters.
+func extractHints(params *URLParameters, query map[string][]string) error {
+	if hintQuery, ok := query[ParamHint]; ok {
 		if len(hintQuery) == 0 {
-			return params, ErrEmptyHintQuery
+			return ErrEmptyHintQuery
 		}
 		for _, hint := range hintQuery {
-			// valid hints are: "hash", "contract_address", "function_selector", "logs", "calldata"
-			if hint != "hash" && hint != "contract_address" && hint != "function_selector" && hint != "logs" && hint != "calldata" && hint != "default_logs" {
-				return params, ErrIncorrectAuctionHints
+			if _, ok := validHints[hint]; !ok {
+				return ErrIncorrectAuctionHints(hint)
 			}
 		}
-		hint = hintQuery
+		params.pref.Privacy.Hints = hintQuery
 		params.prefWasSet = true
-	} else {
-		hint = DefaultAuctionHint
 	}
-	params.pref.Privacy.Hints = hint
+	return nil
+}
 
-	originIdQuery, ok := normalizedQuery["originid"]
-	if ok {
-		if len(originIdQuery) == 0 {
-			return params, ErrIncorrectOriginId
+// extractOriginId sets the origin ID from the query parameters.
+func extractOriginId(params *URLParameters, query map[string][]string) error {
+	if originId, ok := query[ParamOriginId]; ok {
+		if len(originId) == 0 || len(originId[0]) > MaxOriginIdLength {
+			return ErrIncorrectOriginId(originId[0])
 		}
-		params.originId = originIdQuery[0]
+		params.originId = originId[0]
 	}
+	return nil
+}
 
-	targetBuildersQuery, ok := normalizedQuery["builder"]
-	if ok {
-		if len(targetBuildersQuery) == 0 {
-			return params, ErrEmptyTargetBuilderQuery
+// extractBuilders sets the target builders from the query parameters.
+func extractBuilders(params *URLParameters, query map[string][]string, allBuilders []string) error {
+	if builders, ok := query[ParamBuilder]; ok {
+		if len(builders) == 0 {
+			return ErrEmptyTargetBuilderQuery
 		}
-		params.pref.Privacy.Builders = targetBuildersQuery
+		params.pref.Privacy.Builders = builders
 	}
 	if params.fast {
-		// set all builders no matter what's in the url
 		params.pref.Privacy.Builders = allBuilders
 	}
+	return nil
+}
 
-	refundAddressQuery, ok := normalizedQuery["refund"]
-	if ok {
-		if len(refundAddressQuery) == 0 {
-			return params, ErrIncorrectRefundQuery
+// extractRefund sets the refund configuration from the query parameters.
+func extractRefund(params *URLParameters, query map[string][]string) error {
+	if refundAddressQuery, ok := query[ParamRefund]; ok {
+		refundConfig, err := processRefundAddressQuery(refundAddressQuery)
+		if err != nil {
+			return err
 		}
+		params.pref.Validity.Refund = refundConfig
+	}
+	return nil
+}
 
-		var (
-			addresses = make([]common.Address, len(refundAddressQuery))
-			percents  = make([]int, len(refundAddressQuery))
-		)
+// processRefundAddressQuery processes the refund address query and returns a list of RefundConfig objects.
+func processRefundAddressQuery(refundAddressQuery []string) ([]types.RefundConfig, error) {
+	if len(refundAddressQuery) == 0 {
+		return nil, ErrIncorrectRefundQuery(refundAddressQuery[0])
+	}
 
-		for i, refundAddress := range refundAddressQuery {
-			split := strings.Split(refundAddress, ":")
-			if len(split) != 2 {
-				return params, ErrIncorrectRefundQuery
-			}
-			if !common.IsHexAddress(split[0]) {
-				return params, ErrIncorrectRefundAddressQuery
-			}
-			address := common.HexToAddress(split[0])
-			percent, err := strconv.Atoi(split[1])
-			if err != nil {
-				return params, ErrIncorrectRefundPercentageQuery
-			}
-			if percent <= 0 || percent >= 100 {
-				return params, ErrIncorrectRefundPercentageQuery
-			}
-			addresses[i] = address
-			percents[i] = percent
-		}
+	var refundConfig []types.RefundConfig
 
-		// should not exceed 100%
-		var totalRefund int
-		for _, percent := range percents {
-			totalRefund += percent
+	for _, refundAddress := range refundAddressQuery {
+		split := strings.Split(refundAddress, ":")
+		if len(split) != 2 {
+			return nil, ErrIncorrectRefundQuery(refundAddress)
 		}
-		if totalRefund > 100 {
-			return params, ErrIncorrectRefundTotalPercentageQuery
+		if !common.IsHexAddress(split[0]) {
+			return nil, ErrIncorrectRefundAddressQuery(split[0])
 		}
-
-		refundConfig := make([]types.RefundConfig, len(percents))
-		for i, percent := range percents {
-			refundConfig[i] = types.RefundConfig{
-				Address: addresses[i],
-				Percent: percent,
-			}
+		address := common.HexToAddress(split[0])
+		percent, err := strconv.Atoi(split[1])
+		if err != nil {
+			return nil, errors.Wrap(err, "invalid refund percentage")
+		}
+		if percent < MinRefundPercent || percent > MaxRefundPercent {
+			return nil, ErrIncorrectRefundPercentageQuery(split[1])
 		}
+		refundConfig = append(refundConfig, types.RefundConfig{
+			Address: address,
+			Percent: percent,
+		})
+	}
 
-		params.pref.Validity.Refund = refundConfig
+	// should not exceed 100%
+	var totalRefund int
+	for _, percent := range refundConfig {
+		totalRefund += percent.Percent
+	}
+	if totalRefund > 100 {
+		return nil, ErrIncorrectRefundTotalPercentageQuery(strconv.Itoa(totalRefund))
 	}
 
-	return params, nil
+	return refundConfig, nil
 }
 
 func AuctionPreferenceErrorToJSONRPCResponse(jsonReq *types.JsonRpcRequest, err error) *types.JsonRpcResponse {

server/url_params_test.go

@@ -54,7 +54,7 @@ func TestExtractAuctionPreferenceFromUrl(t *testing.T) {
 		"incorrect hint preference": {
 			url:  "https://rpc.flashbots.net?hint=contract_address&hint=function_selector&hint=logs&hint=incorrect",
 			want: URLParameters{},
-			err:  ErrIncorrectAuctionHints,
+			err:  ErrIncorrectAuctionHints("incorrect"),
 		},
 		"rpc endpoint set": {
 			url: "https://rpc.flashbots.net?rpc=https://mainnet.infura.io/v3/123",
@@ -158,7 +158,7 @@ func TestExtractAuctionPreferenceFromUrl(t *testing.T) {
 				prefWasSet: false,
 				originId:   "",
 			},
-			err: ErrIncorrectRefundQuery,
+			err: ErrIncorrectRefundQuery(""),
 		},
 		"set refund, incorrect 110": {
 			url: "https://rpc.flashbots.net?refund=0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:110",
@@ -167,7 +167,7 @@ func TestExtractAuctionPreferenceFromUrl(t *testing.T) {
 				prefWasSet: false,
 				originId:   "",
 			},
-			err: ErrIncorrectRefundPercentageQuery,
+			err: ErrIncorrectRefundPercentageQuery("110"),
 		},
 		"set refund, incorrect address": {
 			url: "https://rpc.flashbots.net?refund=0xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:80",
@@ -176,7 +176,7 @@ func TestExtractAuctionPreferenceFromUrl(t *testing.T) {
 				prefWasSet: false,
 				originId:   "",
 			},
-			err: ErrIncorrectRefundAddressQuery,
+			err: ErrIncorrectRefundAddressQuery("0xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"),
 		},
 		"set refund, incorrect 50 + 60": {
 			url: "https://rpc.flashbots.net?refund=0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:50&refund=0xbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:60",
@@ -185,7 +185,7 @@ func TestExtractAuctionPreferenceFromUrl(t *testing.T) {
 				prefWasSet: false,
 				originId:   "",
 			},
-			err: ErrIncorrectRefundTotalPercentageQuery,
+			err: ErrIncorrectRefundTotalPercentageQuery("110"),
 		},
 		"fast": {
 			url: "https://rpc.flashbots.net/fast",
@@ -234,7 +234,7 @@ func TestExtractAuctionPreferenceFromUrl(t *testing.T) {
 
 			got, err := ExtractParametersFromUrl(url, []string{"builder1", "builder2"})
 			if tt.err != nil {
-				require.ErrorIs(t, err, tt.err)
+				require.EqualError(t, err, tt.err.Error())
 			} else {
 				require.NoError(t, err)
 			}