Code review fixes

services/horizon/internal/actions/path.go

@@ -167,7 +167,7 @@ func (handler FindPathsHandler) GetResource(w HeaderWriter, r *http.Request) (in
 		switch err {
 		case simplepath.ErrEmptyInMemoryOrderBook:
 			return nil, horizonProblem.StillIngesting
-		case paths.ErrLimitExceeded:
+		case paths.ErrRateLimitExceeded:
 			return nil, horizonProblem.ServerOverCapacity
 		default:
 			if err != nil {
@@ -345,7 +345,7 @@ func (handler FindFixedPathsHandler) GetResource(w HeaderWriter, r *http.Request
 		switch err {
 		case simplepath.ErrEmptyInMemoryOrderBook:
 			return nil, horizonProblem.StillIngesting
-		case paths.ErrLimitExceeded:
+		case paths.ErrRateLimitExceeded:
 			return nil, horizonProblem.ServerOverCapacity
 		default:
 			if err != nil {

services/horizon/internal/actions_path_test.go

@@ -84,9 +84,9 @@ func TestPathActionsLimitExceeded(t *testing.T) {
 	assertions := &test.Assertions{tt.Assert}
 	finder := paths.MockFinder{}
 	finder.On("Find", mock.Anything, mock.Anything, uint(3)).
-		Return([]paths.Path{}, uint32(0), paths.ErrLimitExceeded).Times(2)
+		Return([]paths.Path{}, uint32(0), paths.ErrRateLimitExceeded).Times(2)
 	finder.On("FindFixedPaths", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
-		Return([]paths.Path{}, uint32(0), paths.ErrLimitExceeded).Times(1)
+		Return([]paths.Path{}, uint32(0), paths.ErrRateLimitExceeded).Times(1)
 
 	rh := mockPathFindingClient(
 		tt,

services/horizon/internal/config.go

@@ -49,11 +49,12 @@ type Config struct {
 	MaxPathLength uint
 	// MaxAssetsPerPathRequest is the maximum number of assets considered for `/paths/strict-send` and `/paths/strict-recieve`
 	MaxAssetsPerPathRequest int
-	// DisablePoolPathFinding configures horizon to run without the path finding endpoint.
+	// DisablePoolPathFinding configures horizon to run path finding without including liquidity pools
+	// in the path finding search.
 	DisablePoolPathFinding bool
 	// MaxPathFindingRequests is the maximum number of path finding requests horizon will allow
 	// in a 1-second period. A value of 0 disables the limit.
-	MaxPathFindingRequests int
+	MaxPathFindingRequests uint
 
 	NetworkPassphrase string
 	SentryDSN         string

services/horizon/internal/flags.go

@@ -395,8 +395,8 @@ func Flags() (*Config, support.ConfigOptions) {
 		&support.ConfigOption{
 			Name:        "max-path-finding-requests",
 			ConfigKey:   &config.MaxPathFindingRequests,
-			OptType:     types.Int,
-			FlagDefault: 0,
+			OptType:     types.Uint,
+			FlagDefault: uint(0),
 			Required:    false,
 			Usage: "The maximum number of path finding requests per second horizon will allow." +
 				" A value of zero (the default) disables the limit.",
@@ -728,9 +728,5 @@ func ApplyFlags(config *Config, flags support.ConfigOptions, options ApplyOption
 		return fmt.Errorf("Invalid config: Only one option of --behind-cloudflare and --behind-aws-load-balancer is allowed. If Horizon is behind both, use --behind-cloudflare only.")
 	}
 
-	if config.MaxPathFindingRequests < 0 {
-		return fmt.Errorf("Invalid config: --max-path-finxing-requests must not be negative")
-	}
-
 	return nil
 }

services/horizon/internal/integration/parameters_test.go

@@ -172,17 +172,17 @@ func TestMaxPathFindingRequests(t *testing.T) {
 		err := test.StartHorizon()
 		assert.NoError(t, err)
 		test.WaitForHorizon()
-		assert.Equal(t, test.Horizon().Config().MaxPathFindingRequests, 0)
+		assert.Equal(t, test.Horizon().Config().MaxPathFindingRequests, uint(0))
 		_, ok := test.Horizon().Paths().(simplepath.InMemoryFinder)
 		assert.True(t, ok)
 		test.Shutdown()
 	})
 	t.Run("set to 5", func(t *testing.T) {
-		test := NewParameterTest(t, map[string]string{"max-max-path-finding-requests": "5"})
+		test := NewParameterTest(t, map[string]string{"max-path-finding-requests": "5"})
 		err := test.StartHorizon()
 		assert.NoError(t, err)
 		test.WaitForHorizon()
-		assert.Equal(t, test.Horizon().Config().MaxPathFindingRequests, 5)
+		assert.Equal(t, test.Horizon().Config().MaxPathFindingRequests, uint(5))
 		finder, ok := test.Horizon().Paths().(*paths.RateLimitedFinder)
 		assert.True(t, ok)
 		assert.Equal(t, finder.Limit(), 5)

services/horizon/internal/paths/ratelimit.go

@@ -10,8 +10,8 @@ import (
 )
 
 var (
-	// ErrLimitExceeded indicates that the in memory order book is not yet populated
-	ErrLimitExceeded = errors.New("Empty orderbook")
+	// ErrRateLimitExceeded indicates that the Finder is not able to fulfill the request due to rate limits.
+	ErrRateLimitExceeded = errors.New("Rate limit exceeded")
 )
 
 // RateLimitedFinder is a Finder implementation which limits the number of path finding requests.
@@ -22,10 +22,10 @@ type RateLimitedFinder struct {
 
 // NewRateLimitedFinder constructs a new RateLimitedFinder which enforces a per
 // second limit on path finding requests.
-func NewRateLimitedFinder(finder Finder, limit int) *RateLimitedFinder {
+func NewRateLimitedFinder(finder Finder, limit uint) *RateLimitedFinder {
 	return &RateLimitedFinder{
 		finder:  finder,
-		limiter: rate.NewLimiter(rate.Limit(limit), limit),
+		limiter: rate.NewLimiter(rate.Limit(limit), int(limit)),
 	}
 }
 
@@ -34,16 +34,16 @@ func (f *RateLimitedFinder) Limit() int {
 	return f.limiter.Burst()
 }
 
-// Find implements the Finder interface and returns ErrLimitExceeded if the
+// Find implements the Finder interface and returns ErrRateLimitExceeded if the
 // RateLimitedFinder is unable to complete the request due to rate limits.
 func (f *RateLimitedFinder) Find(ctx context.Context, q Query, maxLength uint) ([]Path, uint32, error) {
 	if !f.limiter.Allow() {
-		return nil, 0, ErrLimitExceeded
+		return nil, 0, ErrRateLimitExceeded
 	}
 	return f.finder.Find(ctx, q, maxLength)
 }
 
-// FindFixedPaths implements the Finder interface and returns ErrLimitExceeded if the
+// FindFixedPaths implements the Finder interface and returns ErrRateLimitExceeded if the
 // RateLimitedFinder is unable to complete the request due to rate limits.
 func (f *RateLimitedFinder) FindFixedPaths(
 	ctx context.Context,
@@ -53,7 +53,7 @@ func (f *RateLimitedFinder) FindFixedPaths(
 	maxLength uint,
 ) ([]Path, uint32, error) {
 	if !f.limiter.Allow() {
-		return nil, 0, ErrLimitExceeded
+		return nil, 0, ErrRateLimitExceeded
 	}
 	return f.finder.FindFixedPaths(ctx, sourceAsset, amountToSpend, destinationAssets, maxLength)
 }

services/horizon/internal/paths/ratelimit_test.go

@@ -48,7 +48,7 @@ func TestRateLimitedFinder(t *testing.T) {
 
 			for _, f := range []func(Finder){find, findFixedPaths} {
 				wg.Add(totalCalls)
-				rateLimitedFinder := NewRateLimitedFinder(mockFinder, limit)
+				rateLimitedFinder := NewRateLimitedFinder(mockFinder, uint(limit))
 				assert.Equal(t, limit, rateLimitedFinder.Limit())
 				for i := 0; i < totalCalls; i++ {
 					go f(rateLimitedFinder)
@@ -57,7 +57,7 @@ func TestRateLimitedFinder(t *testing.T) {
 				requestsExceedingLimit := totalCalls - limit
 				for i := 0; i < requestsExceedingLimit; i++ {
 					err := <-errorChan
-					assert.Equal(t, ErrLimitExceeded, err)
+					assert.Equal(t, ErrRateLimitExceeded, err)
 				}
 
 				wg.Add(-requestsExceedingLimit)