Add tests for the volume filter (part of #483)

plugins/filterFactory.go

@@ -71,6 +71,22 @@ func filterVolume(f *FilterFactory, configInput string) (SubmitFilter, error) {
 	)
 }
 
+func makeRawVolumeFilterConfig(
+	sellBaseAssetCapInBaseUnits *float64,
+	sellBaseAssetCapInQuoteUnits *float64,
+	mode volumeFilterMode,
+	additionalMarketIDs []string,
+	optionalAccountIDs []string,
+) *VolumeFilterConfig {
+	return &VolumeFilterConfig{
+		SellBaseAssetCapInBaseUnits:  sellBaseAssetCapInBaseUnits,
+		SellBaseAssetCapInQuoteUnits: sellBaseAssetCapInQuoteUnits,
+		mode:                         mode,
+		additionalMarketIDs:          additionalMarketIDs,
+		optionalAccountIDs:           optionalAccountIDs,
+	}
+}
+
 func makeVolumeFilterConfig(configInput string) (*VolumeFilterConfig, error) {
 	parts := strings.Split(configInput, "/")
 	if len(parts) != 6 {

plugins/volumeFilter.go

@@ -44,6 +44,12 @@ type VolumeFilterConfig struct {
 	// buyBaseAssetCapInQuoteUnits  *float64
 }
 
+type limitParameters struct {
+	sellBaseAssetCapInBaseUnits  *float64
+	sellBaseAssetCapInQuoteUnits *float64
+	mode                         volumeFilterMode
+}
+
 type volumeFilter struct {
 	name                   string
 	configValue            string
@@ -73,13 +79,16 @@ func makeFilterVolume(
 	if e != nil {
 		return nil, fmt.Errorf("could not convert quote asset (%s) from trading pair via the passed in assetDisplayFn: %s", string(tradingPair.Quote), e)
 	}
+
 	marketID := MakeMarketID(exchangeName, baseAssetString, quoteAssetString)
 	marketIDs := utils.Dedupe(append([]string{marketID}, config.additionalMarketIDs...))
 	dailyVolumeByDateQuery, e := queries.MakeDailyVolumeByDateForMarketIdsAction(db, marketIDs, "sell", config.optionalAccountIDs)
 	if e != nil {
 		return nil, fmt.Errorf("could not make daily volume by date Query: %s", e)
 	}
 
+	// TODO DS Validate the config, to have exactly one asset cap defined; a valid mode; non-nil market IDs; and non-nil optional account IDs.
+
 	return &volumeFilter{
 		name:                   "volumeFilter",
 		configValue:            configValue,
@@ -135,7 +144,12 @@ func (f *volumeFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProtocol
 	}
 
 	innerFn := func(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, error) {
-		return f.volumeFilterFn(dailyOTB, dailyTBB, op)
+		limitParameters := limitParameters{
+			sellBaseAssetCapInBaseUnits:  f.config.SellBaseAssetCapInBaseUnits,
+			sellBaseAssetCapInQuoteUnits: f.config.SellBaseAssetCapInQuoteUnits,
+			mode:                         f.config.mode,
+		}
+		return volumeFilterFn(dailyOTB, dailyTBB, op, f.baseAsset, f.quoteAsset, limitParameters)
 	}
 	ops, e = filterOps(f.name, f.baseAsset, f.quoteAsset, sellingOffers, buyingOffers, ops, innerFn)
 	if e != nil {
@@ -144,8 +158,8 @@ func (f *volumeFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProtocol
 	return ops, nil
 }
 
-func (f *volumeFilter) volumeFilterFn(dailyOTB *VolumeFilterConfig, dailyTBB *VolumeFilterConfig, op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, error) {
-	isSell, e := utils.IsSelling(f.baseAsset, f.quoteAsset, op.Selling, op.Buying)
+func volumeFilterFn(dailyOTB *VolumeFilterConfig, dailyTBB *VolumeFilterConfig, op *txnbuild.ManageSellOffer, baseAsset hProtocol.Asset, quoteAsset hProtocol.Asset, lp limitParameters) (*txnbuild.ManageSellOffer, error) {
+	isSell, e := utils.IsSelling(baseAsset, quoteAsset, op.Selling, op.Buying)
 	if e != nil {
 		return nil, fmt.Errorf("error when running the isSelling check for offer '%+v': %s", *op, e)
 	}
@@ -165,38 +179,38 @@ func (f *volumeFilter) volumeFilterFn(dailyOTB *VolumeFilterConfig, dailyTBB *Vo
 		newAmountBeingSold := amountValueUnitsBeingSold
 		var keepSellingBase bool
 		var keepSellingQuote bool
-		if f.config.SellBaseAssetCapInBaseUnits != nil {
+		if lp.sellBaseAssetCapInBaseUnits != nil {
 			projectedSoldInBaseUnits := *dailyOTB.SellBaseAssetCapInBaseUnits + *dailyTBB.SellBaseAssetCapInBaseUnits + amountValueUnitsBeingSold
-			keepSellingBase = projectedSoldInBaseUnits <= *f.config.SellBaseAssetCapInBaseUnits
+			keepSellingBase = projectedSoldInBaseUnits <= *lp.sellBaseAssetCapInBaseUnits
 			newAmountString := ""
-			if f.config.mode == volumeFilterModeExact && !keepSellingBase {
-				newAmount := *f.config.SellBaseAssetCapInBaseUnits - *dailyOTB.SellBaseAssetCapInBaseUnits - *dailyTBB.SellBaseAssetCapInBaseUnits
+			if lp.mode == volumeFilterModeExact && !keepSellingBase {
+				newAmount := *lp.sellBaseAssetCapInBaseUnits - *dailyOTB.SellBaseAssetCapInBaseUnits - *dailyTBB.SellBaseAssetCapInBaseUnits
 				if newAmount > 0 {
 					newAmountBeingSold = newAmount
 					opToReturn.Amount = fmt.Sprintf("%.7f", newAmountBeingSold)
 					keepSellingBase = true
 					newAmountString = ", newAmountString = " + opToReturn.Amount
 				}
 			}
-			log.Printf("volumeFilter:  selling (base units), price=%.8f amount=%.8f, keep = (projectedSoldInBaseUnits) %.7f <= %.7f (config.SellBaseAssetCapInBaseUnits): keepSellingBase = %v%s", sellPrice, amountValueUnitsBeingSold, projectedSoldInBaseUnits, *f.config.SellBaseAssetCapInBaseUnits, keepSellingBase, newAmountString)
+			log.Printf("volumeFilter:  selling (base units), price=%.8f amount=%.8f, keep = (projectedSoldInBaseUnits) %.7f <= %.7f (config.SellBaseAssetCapInBaseUnits): keepSellingBase = %v%s", sellPrice, amountValueUnitsBeingSold, projectedSoldInBaseUnits, *lp.sellBaseAssetCapInBaseUnits, keepSellingBase, newAmountString)
 		} else {
 			keepSellingBase = true
 		}
 
-		if f.config.SellBaseAssetCapInQuoteUnits != nil {
+		if lp.sellBaseAssetCapInQuoteUnits != nil {
 			projectedSoldInQuoteUnits := *dailyOTB.SellBaseAssetCapInQuoteUnits + *dailyTBB.SellBaseAssetCapInQuoteUnits + (newAmountBeingSold * sellPrice)
-			keepSellingQuote = projectedSoldInQuoteUnits <= *f.config.SellBaseAssetCapInQuoteUnits
+			keepSellingQuote = projectedSoldInQuoteUnits <= *lp.sellBaseAssetCapInQuoteUnits
 			newAmountString := ""
-			if f.config.mode == volumeFilterModeExact && !keepSellingQuote {
-				newAmount := (*f.config.SellBaseAssetCapInQuoteUnits - *dailyOTB.SellBaseAssetCapInQuoteUnits - *dailyTBB.SellBaseAssetCapInQuoteUnits) / sellPrice
+			if lp.mode == volumeFilterModeExact && !keepSellingQuote {
+				newAmount := (*lp.sellBaseAssetCapInQuoteUnits - *dailyOTB.SellBaseAssetCapInQuoteUnits - *dailyTBB.SellBaseAssetCapInQuoteUnits) / sellPrice
 				if newAmount > 0 {
 					newAmountBeingSold = newAmount
 					opToReturn.Amount = fmt.Sprintf("%.7f", newAmountBeingSold)
 					keepSellingQuote = true
 					newAmountString = ", newAmountString = " + opToReturn.Amount
 				}
 			}
-			log.Printf("volumeFilter: selling (quote units), price=%.8f amount=%.8f, keep = (projectedSoldInQuoteUnits) %.7f <= %.7f (config.SellBaseAssetCapInQuoteUnits): keepSellingQuote = %v%s", sellPrice, amountValueUnitsBeingSold, projectedSoldInQuoteUnits, *f.config.SellBaseAssetCapInQuoteUnits, keepSellingQuote, newAmountString)
+			log.Printf("volumeFilter: selling (quote units), price=%.8f amount=%.8f, keep = (projectedSoldInQuoteUnits) %.7f <= %.7f (config.SellBaseAssetCapInQuoteUnits): keepSellingQuote = %v%s", sellPrice, amountValueUnitsBeingSold, projectedSoldInQuoteUnits, *lp.sellBaseAssetCapInQuoteUnits, keepSellingQuote, newAmountString)
 		} else {
 			keepSellingQuote = true
 		}

plugins/volumeFilter_test.go

@@ -0,0 +1,161 @@
+package plugins
+
+import (
+	"database/sql"
+	"fmt"
+	"testing"
+
+	"github.com/openlyinc/pointy"
+	"github.com/stellar/kelp/queries"
+	"github.com/stellar/kelp/support/utils"
+
+	"github.com/stellar/go/txnbuild"
+
+	hProtocol "github.com/stellar/go/protocols/horizon"
+	"github.com/stellar/kelp/model"
+	"github.com/stretchr/testify/assert"
+)
+
+func makeWantVolumeFilter(config *VolumeFilterConfig, marketIDs []string, accountIDs []string, action string) *volumeFilter {
+	query, e := queries.MakeDailyVolumeByDateForMarketIdsAction(&sql.DB{}, marketIDs, action, accountIDs)
+	if e != nil {
+		panic(e)
+	}
+
+	return &volumeFilter{
+		name:                   "volumeFilter",
+		baseAsset:              utils.NativeAsset,
+		quoteAsset:             utils.NativeAsset,
+		config:                 config,
+		dailyVolumeByDateQuery: query,
+	}
+}
+
+func TestMakeFilterVolume(t *testing.T) {
+	testAssetDisplayFn := model.MakeSdexMappedAssetDisplayFn(map[model.Asset]hProtocol.Asset{model.Asset("XLM"): utils.NativeAsset})
+	configValue := ""
+	tradingPair := &model.TradingPair{Base: "XLM", Quote: "XLM"}
+	modes := []volumeFilterMode{volumeFilterModeExact, volumeFilterModeIgnore}
+
+	testCases := []struct {
+		name          string
+		exchangeName  string
+		marketIDs     []string
+		accountIDs    []string
+		wantMarketIDs []string
+		wantFilter    *volumeFilter
+	}{
+		// TODO DS Confirm the empty config fails once validation is added to the constructor
+		{
+			name:          "0 market id or account id",
+			exchangeName:  "exchange 2",
+			marketIDs:     []string{},
+			accountIDs:    []string{},
+			wantMarketIDs: []string{"9db20cdd56"},
+		},
+		{
+			name:          "1 market id",
+			exchangeName:  "exchange 1",
+			marketIDs:     []string{"marketID"},
+			accountIDs:    []string{},
+			wantMarketIDs: []string{"6d9862b0e2", "marketID"},
+		},
+		{
+			name:          "2 market ids",
+			exchangeName:  "exchange 2",
+			marketIDs:     []string{"marketID1", "marketID2"},
+			accountIDs:    []string{},
+			wantMarketIDs: []string{"9db20cdd56", "marketID1", "marketID2"},
+		},
+		{
+			name:          "2 dupe market ids, 1 distinct",
+			exchangeName:  "exchange 1",
+			marketIDs:     []string{"marketID1", "marketID1", "marketID2"},
+			accountIDs:    []string{},
+			wantMarketIDs: []string{"6d9862b0e2", "marketID1", "marketID2"},
+		},
+		{
+			name:          "1 account id",
+			exchangeName:  "exchange 2",
+			marketIDs:     []string{},
+			accountIDs:    []string{"accountID"},
+			wantMarketIDs: []string{"9db20cdd56"},
+		},
+		{
+			name:          "2 account ids",
+			exchangeName:  "exchange 1",
+			marketIDs:     []string{},
+			accountIDs:    []string{"accountID1", "accountID2"},
+			wantMarketIDs: []string{"6d9862b0e2"},
+		},
+		{
+			name:          "account and market ids",
+			exchangeName:  "exchange 2",
+			marketIDs:     []string{"marketID"},
+			accountIDs:    []string{"accountID"},
+			wantMarketIDs: []string{"9db20cdd56", "marketID"},
+		},
+	}
+
+	for _, k := range testCases {
+		// this lets us test both types of modes when varying the market and account ids
+		for _, m := range modes {
+			// this lets us run the for-loop below for both base and quote units within the config
+			baseCapInBaseConfig := makeRawVolumeFilterConfig(
+				pointy.Float64(1.0),
+				nil,
+				m,
+				k.marketIDs,
+				k.accountIDs,
+			)
+			baseCapInQuoteConfig := makeRawVolumeFilterConfig(
+				nil,
+				pointy.Float64(1.0),
+				m,
+				k.marketIDs,
+				k.accountIDs,
+			)
+			for _, config := range []*VolumeFilterConfig{baseCapInBaseConfig, baseCapInQuoteConfig} {
+				// configType is used to represent the type of config when printing test name
+				configType := "quote"
+				if config.SellBaseAssetCapInBaseUnits != nil {
+					configType = "base"
+				}
+
+				// TODO DS Vary filter action between buy and sell, once buy logic is implemented.
+				wantFilter := makeWantVolumeFilter(config, k.wantMarketIDs, k.accountIDs, "sell")
+				t.Run(fmt.Sprintf("%s/%s/%s", k.name, configType, m), func(t *testing.T) {
+					actual, e := makeFilterVolume(
+						configValue,
+						k.exchangeName,
+						tradingPair,
+						testAssetDisplayFn,
+						utils.NativeAsset,
+						utils.NativeAsset,
+						&sql.DB{},
+						config,
+					)
+
+					if !assert.Nil(t, e) {
+						return
+					}
+
+					assert.Equal(t, wantFilter, actual)
+				})
+			}
+		}
+	}
+}
+
+func makeManageSellOffer(price, amount string) *txnbuild.ManageSellOffer {
+	if amount == "" {
+		return nil
+	}
+
+	return &txnbuild.ManageSellOffer{
+		Buying:  txnbuild.NativeAsset{},
+		Selling: txnbuild.NativeAsset{},
+		Price:   price,
+		Amount:  amount,
+	}
+}

queries/dailyVolumeByDate_test.go

@@ -79,6 +79,14 @@ func TestDailyVolumeByDate_QueryRow(t *testing.T) {
 			wantTodayQuote:            10.0,
 			wantTomorrowBase:          0.0,
 			wantTomorrowQuote:         0.0,
+		}, {
+			queryByOptionalAccountIDs: []string{"accountID2", "accountID2"}, // duplicate accountIDs should return same as previous test case
+			wantYesterdayBase:         0.0,
+			wantYesterdayQuote:        0.0,
+			wantTodayBase:             100.0,
+			wantTodayQuote:            10.0,
+			wantTomorrowBase:          0.0,
+			wantTomorrowQuote:         0.0,
 		}, {
 			queryByOptionalAccountIDs: []string{"accountID3"}, //accountID3 does not exist
 			wantYesterdayBase:         0.0,

support/utils/functions.go

@@ -302,6 +302,7 @@ func assetEqualsExact(hAsset hProtocol.Asset, xAsset txnbuild.Asset) (bool, erro
 }
 
 // IsSelling helper method
+// TODO DS Add tests for the various possible errors.
 func IsSelling(sdexBase hProtocol.Asset, sdexQuote hProtocol.Asset, selling txnbuild.Asset, buying txnbuild.Asset) (bool, error) {
 	sellingBase, e := assetEqualsExact(sdexBase, selling)
 	if e != nil {