From 3e0c240c7c1aabd618f45634589237c3dcd91cd3 Mon Sep 17 00:00:00 2001 From: Nikhil Saraf <1028334+nikhilsaraf@users.noreply.github.com> Date: Thu, 9 Jan 2020 12:20:56 +0530 Subject: [PATCH] new priceFeedFilter, refactor filterOps, remove dedupeRedundantUpdatesFilter, closes #329 (#335) * 1 - priceFeedFilter.go * 2 - better function and var names for priceFeedFilter * 3 - add filterPriceFeed to filterFactory.go * 4 - add sample price feed filter in sample_trader.cfg * 5 - add a log line per op in the priceFeedFilter * 6 - refactor filterFn to remove keep return var returning a nil newOp means we don't want to keep it, and returning a non-nil newOp means we want to keep it. Not keeping is handled differently for ops that have existing offers vs. those that do not * f 6 - need to copy original offer * 7 - remove dedupeRedundantUpdatesFilter * 8 - filterOps to fetch original offer if exists for all ops * 9 - simplify runInnerFilterFn by figuring out filterCounterToIncrement in caller * 10 - rename !isNewOpNil -> keep * 11 - consolidate drop case for volumeFilter and makerModeFilter * 12 - return newOpToPrepend before the newOpToAppend value --- cmd/trade.go | 3 - examples/configs/trader/sample_trader.cfg | 11 ++ plugins/dedupeRedundantUpdatesFilter.go | 52 -------- plugins/filterFactory.go | 28 ++++- plugins/makerModeFilter.go | 29 ++--- plugins/maxPriceFilter.go | 12 +- plugins/minPriceFilter.go | 12 +- plugins/priceFeedFilter.go | 101 +++++++++++++++ plugins/submitFilter.go | 142 +++++++++++----------- plugins/volumeFilter.go | 25 ++-- 10 files changed, 242 insertions(+), 173 deletions(-) delete mode 100644 plugins/dedupeRedundantUpdatesFilter.go create mode 100644 plugins/priceFeedFilter.go diff --git a/cmd/trade.go b/cmd/trade.go index d5cc3f417..107e9d1f4 100644 --- a/cmd/trade.go +++ b/cmd/trade.go @@ -387,9 +387,6 @@ func makeBot( } submitFilters = append(submitFilters, filter) } - // always add the dedupeRedundantUpdatesFilter as the second-last filter - dedupeRedundantUpdatesFilter := plugins.MakeFilterDedupeRedundantUpdates(assetBase, assetQuote) - submitFilters = append(submitFilters, dedupeRedundantUpdatesFilter) // exchange constraints filter is last so we catch any modifications made by previous filters. this ensures that the exchange is // less likely to reject our updates submitFilters = append(submitFilters, diff --git a/examples/configs/trader/sample_trader.cfg b/examples/configs/trader/sample_trader.cfg index 75dfda375..82d561744 100644 --- a/examples/configs/trader/sample_trader.cfg +++ b/examples/configs/trader/sample_trader.cfg @@ -120,6 +120,17 @@ HORIZON_URL="https://horizon-testnet.stellar.org" # # # limit offers based on a maximum price requirement # "price/max/1.00", +# +# # limit offers based on a reference price from any price feed. +# # this "priceFeed" filter uses the format: priceFeed/// +# # and only allows prices "outside" the reference price. +# # i.e. gte/gt for sell offers or lte/lt for buy offers based on the comparisonModes options defined: +# # - "outside-exclude" - keeps offers that are great than the reference price for sell offers and keeps offers that +# # are less than the reference price for buy offers. +# # - "outside-include" - keeps offers that are great than or equal to the reference price for sell offers and keeps +# # offers that are less than or equal to the reference price for buy offers. +# # Note: the feedURL specified at the end of this filter may have its own "/" delimiters which is ok. +# "priceFeed/outside-exclude/exchange/kraken/XXLM/ZUSD/mid", #] # specify parameters for how we compute the operation fee from the /fee_stats endpoint diff --git a/plugins/dedupeRedundantUpdatesFilter.go b/plugins/dedupeRedundantUpdatesFilter.go deleted file mode 100644 index f4acf28d1..000000000 --- a/plugins/dedupeRedundantUpdatesFilter.go +++ /dev/null @@ -1,52 +0,0 @@ -package plugins - -import ( - "fmt" - - hProtocol "github.com/stellar/go/protocols/horizon" - "github.com/stellar/go/txnbuild" -) - -type dedupeRedundantUpdatesFilter struct { - name string - baseAsset hProtocol.Asset - quoteAsset hProtocol.Asset -} - -// MakeFilterDedupeRedundantUpdates makes a submit filter that dedupes offer updates that would result in the same offer being placed on the orderbook -func MakeFilterDedupeRedundantUpdates(baseAsset hProtocol.Asset, quoteAsset hProtocol.Asset) SubmitFilter { - return &dedupeRedundantUpdatesFilter{ - name: "dedupeRedundantUpdatesFilter", - baseAsset: baseAsset, - quoteAsset: quoteAsset, - } -} - -var _ SubmitFilter = &dedupeRedundantUpdatesFilter{} - -func (f *dedupeRedundantUpdatesFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProtocol.Offer, buyingOffers []hProtocol.Offer) ([]txnbuild.Operation, error) { - offersMap := map[int64]hProtocol.Offer{} - for _, offer := range append(sellingOffers, buyingOffers...) { - offersMap[offer.ID] = offer - } - - innerFn := func(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, bool, error) { - existingOffer, hasOffer := offersMap[op.OfferID] - isDupe := hasOffer && existingOffer.Amount == op.Amount && existingOffer.Price == op.Price - - if isDupe { - // do not return an op because this is spawned from an offer (hasOffer = true) and we do not want to drop the offer nor do we want to create - // a new operation to delete the offer and re-update the offer to the same values, so the returned Operation is nil with keep = false - return nil, false, nil - } - - // this an actual update of either the offer or a new operation, we dont want to make any changes to that so we return the original op with keep = true - return op, true, nil - } - ops, e := filterOps(f.name, f.baseAsset, f.quoteAsset, sellingOffers, buyingOffers, ops, innerFn) - if e != nil { - return nil, fmt.Errorf("could not apply filter: %s", e) - } - - return ops, nil -} diff --git a/plugins/filterFactory.go b/plugins/filterFactory.go index f00fed180..c42c26a2e 100644 --- a/plugins/filterFactory.go +++ b/plugins/filterFactory.go @@ -11,8 +11,9 @@ import ( ) var filterMap = map[string]func(f *FilterFactory, configInput string) (SubmitFilter, error){ - "volume": filterVolume, - "price": filterPrice, + "volume": filterVolume, + "price": filterPrice, + "priceFeed": filterPriceFeed, } // FilterFactory is a struct that handles creating all the filters @@ -100,3 +101,26 @@ func filterPrice(f *FilterFactory, configInput string) (SubmitFilter, error) { } return nil, fmt.Errorf("invalid price filter type in second argument (%s)", configInput) } + +func filterPriceFeed(f *FilterFactory, configInput string) (SubmitFilter, error) { + // parts[0] = "priceFeed", parts[1] = comparisonMode, parts[2] = feedDataType, parts[3] = feedURL which can have more "/" chars + parts := strings.Split(configInput, "/") + if len(parts) < 4 { + return nil, fmt.Errorf("\"priceFeed\" filter needs at least 4 parts separated by the '/' delimiter (priceFeed///) but we received %s", configInput) + } + + cmString := parts[1] + feedType := parts[2] + feedURL := strings.Join(parts[3:len(parts)], "/") + pf, e := MakePriceFeed(feedType, feedURL) + if e != nil { + return nil, fmt.Errorf("could not make price feed for config input string '%s': %s", configInput, e) + } + + filter, e := MakeFilterPriceFeed(f.BaseAsset, f.QuoteAsset, cmString, pf) + if e != nil { + return nil, fmt.Errorf("could not make price feed filter for config input string '%s': %s", configInput, e) + } + + return filter, nil +} diff --git a/plugins/makerModeFilter.go b/plugins/makerModeFilter.go index 51a28d287..1a207907f 100644 --- a/plugins/makerModeFilter.go +++ b/plugins/makerModeFilter.go @@ -42,14 +42,14 @@ func (f *makerModeFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProto return nil, fmt.Errorf("could not get assets: %s", e) } - innerFn := func(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, bool, error) { + innerFn := func(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, error) { topBidPrice, e := f.topOrderPriceExcludingTrader(ob.Bids(), buyingOffers, false) if e != nil { - return nil, false, fmt.Errorf("could not get topOrderPriceExcludingTrader for bids: %s", e) + return nil, fmt.Errorf("could not get topOrderPriceExcludingTrader for bids: %s", e) } topAskPrice, e := f.topOrderPriceExcludingTrader(ob.Asks(), sellingOffers, true) if e != nil { - return nil, false, fmt.Errorf("could not get topOrderPriceExcludingTrader for asks: %s", e) + return nil, fmt.Errorf("could not get topOrderPriceExcludingTrader for asks: %s", e) } return f.transformOfferMakerMode(baseAsset, quoteAsset, topBidPrice, topAskPrice, op) @@ -131,20 +131,20 @@ func (f *makerModeFilter) transformOfferMakerMode( topBidPrice *model.Number, topAskPrice *model.Number, op *txnbuild.ManageSellOffer, -) (*txnbuild.ManageSellOffer, bool, error) { +) (*txnbuild.ManageSellOffer, error) { // delete operations should never be dropped if op.Amount == "0" { - return op, true, nil + return op, nil } isSell, e := utils.IsSelling(baseAsset, quoteAsset, op.Selling, op.Buying) if e != nil { - return nil, false, fmt.Errorf("error when running the isSelling check: %s", e) + return nil, fmt.Errorf("error when running the isSelling check: %s", e) } sellPrice, e := strconv.ParseFloat(op.Price, 64) if e != nil { - return nil, false, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) + return nil, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) } var keep bool @@ -167,18 +167,9 @@ func (f *makerModeFilter) transformOfferMakerMode( } if keep { - return op, true, nil + return op, nil } - // figure out how to convert the offer to a dropped state - if op.OfferID == 0 { - // new offers can be dropped - return nil, false, nil - } else if op.Amount != "0" { - // modify offers should be converted to delete offers - opCopy := *op - opCopy.Amount = "0" - return &opCopy, false, nil - } - return nil, keep, fmt.Errorf("unable to transform manageOffer operation: offerID=%d, amount=%s, price=%.7f", op.OfferID, op.Amount, sellPrice) + // we don't want to keep it so return the dropped command + return nil, nil } diff --git a/plugins/maxPriceFilter.go b/plugins/maxPriceFilter.go index 7907d87b6..a0244a4e1 100644 --- a/plugins/maxPriceFilter.go +++ b/plugins/maxPriceFilter.go @@ -54,24 +54,24 @@ func (f *maxPriceFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProtoc return ops, nil } -func (f *maxPriceFilter) maxPriceFilterFn(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, bool, error) { +func (f *maxPriceFilter) maxPriceFilterFn(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, error) { isSell, e := utils.IsSelling(f.baseAsset, f.quoteAsset, op.Selling, op.Buying) if e != nil { - return nil, false, fmt.Errorf("error when running the isSelling check: %s", e) + return nil, fmt.Errorf("error when running the isSelling check: %s", e) } sellPrice, e := strconv.ParseFloat(op.Price, 64) if e != nil { - return nil, false, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) + return nil, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) } if isSell { if sellPrice > *f.config.MaxPrice { - return nil, false, nil + return nil, nil } - return op, true, nil + return op, nil } // TODO for buy side - return op, true, nil + return op, nil } diff --git a/plugins/minPriceFilter.go b/plugins/minPriceFilter.go index 049b1890e..7933861b4 100644 --- a/plugins/minPriceFilter.go +++ b/plugins/minPriceFilter.go @@ -54,24 +54,24 @@ func (f *minPriceFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProtoc return ops, nil } -func (f *minPriceFilter) minPriceFilterFn(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, bool, error) { +func (f *minPriceFilter) minPriceFilterFn(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, error) { isSell, e := utils.IsSelling(f.baseAsset, f.quoteAsset, op.Selling, op.Buying) if e != nil { - return nil, false, fmt.Errorf("error when running the isSelling check: %s", e) + return nil, fmt.Errorf("error when running the isSelling check: %s", e) } sellPrice, e := strconv.ParseFloat(op.Price, 64) if e != nil { - return nil, false, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) + return nil, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) } if isSell { if sellPrice < *f.config.MinPrice { - return nil, false, nil + return nil, nil } - return op, true, nil + return op, nil } // TODO for buy side - return op, true, nil + return op, nil } diff --git a/plugins/priceFeedFilter.go b/plugins/priceFeedFilter.go new file mode 100644 index 000000000..fbb2ccadd --- /dev/null +++ b/plugins/priceFeedFilter.go @@ -0,0 +1,101 @@ +package plugins + +import ( + "fmt" + "log" + "strconv" + + hProtocol "github.com/stellar/go/protocols/horizon" + "github.com/stellar/go/txnbuild" + "github.com/stellar/kelp/api" + "github.com/stellar/kelp/support/utils" +) + +type comparisonMode int8 + +const ( + comparisonModeOutsideExclude comparisonMode = iota // gt for sell, lt for buy + comparisonModeOutsideInclude // gte for sell, lte for buy +) + +func (c comparisonMode) keepSellOp(threshold float64, price float64) bool { + if c == comparisonModeOutsideExclude { + return price > threshold + } else if c == comparisonModeOutsideInclude { + return price >= threshold + } + panic("unidentified comparisonMode") +} + +// TODO implement passesBuy() where we use < and <= operators + +type priceFeedFilter struct { + name string + baseAsset hProtocol.Asset + quoteAsset hProtocol.Asset + pf api.PriceFeed + cm comparisonMode +} + +// MakeFilterPriceFeed makes a submit filter that limits orders placed based on the value of the price feed +func MakeFilterPriceFeed(baseAsset hProtocol.Asset, quoteAsset hProtocol.Asset, comparisonModeString string, pf api.PriceFeed) (SubmitFilter, error) { + var cm comparisonMode + if comparisonModeString == "outside-exclude" { + cm = comparisonModeOutsideExclude + } else if comparisonModeString == "outside-include" { + cm = comparisonModeOutsideInclude + } else { + return nil, fmt.Errorf("invalid comparisonMode ('%s') used for priceFeedFilter", comparisonModeString) + } + + return &priceFeedFilter{ + name: "priceFeedFilter", + baseAsset: baseAsset, + quoteAsset: quoteAsset, + cm: cm, + pf: pf, + }, nil +} + +var _ SubmitFilter = &priceFeedFilter{} + +func (f *priceFeedFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProtocol.Offer, buyingOffers []hProtocol.Offer) ([]txnbuild.Operation, error) { + ops, e := filterOps(f.name, f.baseAsset, f.quoteAsset, sellingOffers, buyingOffers, ops, f.priceFeedFilterFn) + if e != nil { + return nil, fmt.Errorf("could not apply filter: %s", e) + } + return ops, nil +} + +func (f *priceFeedFilter) priceFeedFilterFn(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, error) { + isSell, e := utils.IsSelling(f.baseAsset, f.quoteAsset, op.Selling, op.Buying) + if e != nil { + return nil, fmt.Errorf("error when running the isSelling check: %s", e) + } + + sellPrice, e := strconv.ParseFloat(op.Price, 64) + if e != nil { + return nil, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) + } + + thresholdFeedPrice, e := f.pf.GetPrice() + if e != nil { + return nil, fmt.Errorf("could not get price from priceFeed: %s", e) + } + + var opRet *txnbuild.ManageSellOffer + // for the sell side we keep only those ops that meet the comparison mode using the value from the price feed as the threshold + if isSell { + if f.cm.keepSellOp(thresholdFeedPrice, sellPrice) { + opRet = op + } else { + opRet = nil + } + } else { + // for the buy side we keep only those ops that meet the comparison mode using the value from the price feed as the threshold + // TODO for buy side (after considering whether sellPrice needs to be inverted or not) + opRet = op + } + log.Printf("priceFeedFilter: isSell=%v, sellPrice=%.10f, thresholdFeedPrice=%.10f, keep=%v", isSell, sellPrice, thresholdFeedPrice, opRet != nil) + return opRet, nil +} diff --git a/plugins/submitFilter.go b/plugins/submitFilter.go index 20b899bfb..3776a56d0 100644 --- a/plugins/submitFilter.go +++ b/plugins/submitFilter.go @@ -5,6 +5,7 @@ import ( "log" "strconv" + "github.com/stellar/go/clients/horizon" hProtocol "github.com/stellar/go/protocols/horizon" "github.com/stellar/go/txnbuild" "github.com/stellar/kelp/support/utils" @@ -19,7 +20,15 @@ type SubmitFilter interface { ) ([]txnbuild.Operation, error) } -type filterFn func(op *txnbuild.ManageSellOffer) (newOp *txnbuild.ManageSellOffer, keep bool, e error) +// filterFn returns a non-nil op to indicate the op that we want to append to the update. the newOp can do one of the following: +// - modify an existing offer +// - create a new offer +// - update an offer that was created by a previous filterFn +// filterFn has no knowledge of whether the passed in op is an existing offer or a new op and therefore is not responsible for +// deleting existing offers. +// If the newOp returned is nil and it was spawned from an existingOffer then the filterOps function here will automatically delete +// the existing offer. i.e. if filterFn returns a nil newOp value then we will "drop" that operation or delete the existing offer. +type filterFn func(op *txnbuild.ManageSellOffer) (newOp *txnbuild.ManageSellOffer, e error) type filterCounter struct { idx int @@ -51,6 +60,14 @@ func ignoreOfferIDs(ops []txnbuild.Operation) map[int64]bool { return ignoreOfferIDs } +func makeOfferMap(offers []horizon.Offer) map[int64]horizon.Offer { + offerMap := map[int64]horizon.Offer{} + for _, o := range offers { + offerMap[o.ID] = o + } + return offerMap +} + // TODO - simplify filterOps by separating out logic to convert into a single list of operations from transforming the operations /* What filterOps() does and why: @@ -103,6 +120,7 @@ func filterOps( fn filterFn, ) ([]txnbuild.Operation, error) { ignoreOfferIds := ignoreOfferIDs(ops) + offerMap := makeOfferMap(append(sellingOffers, buyingOffers...)) opCounter := filterCounter{} buyCounter := filterCounter{} sellCounter := filterCounter{} @@ -126,7 +144,7 @@ func filterOps( return nil, fmt.Errorf("unable to pick between whether the op was a buy or sell op: %s", e) } - opToTransform, originalOffer, filterCounterToIncrement, isIgnoredOffer, e := selectOpOrOffer( + opToTransform, filterCounterToIncrement, isIgnoredOffer, e := selectOpOrOffer( offerList, offerCounter, o, @@ -138,17 +156,16 @@ func filterOps( } filterCounterToIncrement.idx++ if isIgnoredOffer { - filterCounterToIncrement.ignored++ + // don't increment anything here becuase it will be addressed with the op that updated the offer continue } + originalOfferAsOp := fetchOfferAsOpByID(opToTransform.OfferID, offerMap) - newOpToAppend, newOpToPrepend, filterCounterToIncrement, incrementValues, e := runInnerFilterFn( + newOpToPrepend, newOpToAppend, incrementValues, e := runInnerFilterFn( *opToTransform, // pass copy fn, - originalOffer, + originalOfferAsOp, *o, // pass copy - offerCounter, - &opCounter, ) if e != nil { return nil, fmt.Errorf("error while running inner filter function: %s", e) @@ -159,8 +176,15 @@ func filterOps( if newOpToPrepend != nil { filteredOps = append([]txnbuild.Operation{newOpToPrepend}, filteredOps...) } - if filterCounterToIncrement != nil && incrementValues != nil { - filterCounterToIncrement.add(*incrementValues) + if originalOfferAsOp != nil { + offerCounter.add(incrementValues) + + // if this was a selection of an operation that had a corresponding offer than increment opCounter's ignored field + if *filterCounterToIncrement == opCounter { + opCounter.ignored++ + } + } else { + opCounter.add(incrementValues) } default: filteredOps = append(filteredOps, o) @@ -173,7 +197,6 @@ func filterOps( filteredOps, e := handleRemainingOffers( &sellCounter, sellingOffers, - &opCounter, ignoreOfferIds, filteredOps, fn, @@ -184,7 +207,6 @@ func filterOps( filteredOps, e = handleRemainingOffers( &buyCounter, buyingOffers, - &opCounter, ignoreOfferIds, filteredOps, fn, @@ -193,9 +215,9 @@ func filterOps( return nil, fmt.Errorf("error when handling remaining buy offers: %s", e) } - log.Printf("filter \"%s\" result A: dropped %d, transformed %d, kept %d ops from the %d ops passed in\n", filterName, opCounter.dropped, opCounter.transformed, opCounter.kept, len(ops)) - log.Printf("filter \"%s\" result B: dropped %d, transformed %d, kept %d, ignored %d sell offers (corresponding op update) from original %d sell offers\n", filterName, sellCounter.dropped, sellCounter.transformed, sellCounter.kept, sellCounter.ignored, len(sellingOffers)) - log.Printf("filter \"%s\" result C: dropped %d, transformed %d, kept %d, ignored %d buy offers (corresponding op update) from original %d buy offers\n", filterName, buyCounter.dropped, buyCounter.transformed, buyCounter.kept, buyCounter.ignored, len(buyingOffers)) + log.Printf("filter \"%s\" result A: dropped %d, transformed %d, kept %d, ignored %d (handled by offer counter) ops from the %d ops passed in\n", filterName, opCounter.dropped, opCounter.transformed, opCounter.kept, opCounter.ignored, len(ops)) + log.Printf("filter \"%s\" result B: dropped %d, transformed %d, kept %d from original %d sell offers\n", filterName, sellCounter.dropped, sellCounter.transformed, sellCounter.kept, len(sellingOffers)) + log.Printf("filter \"%s\" result C: dropped %d, transformed %d, kept %d from original %d buy offers\n", filterName, buyCounter.dropped, buyCounter.transformed, buyCounter.kept, len(buyingOffers)) log.Printf("filter \"%s\" result D: len(filteredOps) = %d\n", filterName, len(filteredOps)) return filteredOps, nil } @@ -228,35 +250,40 @@ func selectOpOrOffer( ignoreOfferIds map[int64]bool, ) ( opToTransform *txnbuild.ManageSellOffer, - originalOfferAsOp *txnbuild.ManageSellOffer, c *filterCounter, isIgnoredOffer bool, err error, ) { if offerCounter.idx >= len(offerList) { - return mso, nil, opCounter, false, nil + return mso, opCounter, false, nil } - existingOffer := offerList[offerCounter.idx] - if _, ignoreOffer := ignoreOfferIds[existingOffer.ID]; ignoreOffer { - // we want to only compare against valid offers so skip this offer by returning ignored = true - return nil, nil, offerCounter, true, nil + nextOffer := offerList[offerCounter.idx] + if _, ignoreOffer := ignoreOfferIds[nextOffer.ID]; ignoreOffer { + // we want to only compare against valid offers so ignore this offer + return nil, offerCounter, true, nil } - offerPrice := float64(existingOffer.PriceR.N) / float64(existingOffer.PriceR.D) + offerPrice := float64(nextOffer.PriceR.N) / float64(nextOffer.PriceR.D) opPrice, e := strconv.ParseFloat(mso.Price, 64) if e != nil { - return nil, nil, nil, false, fmt.Errorf("could not parse price as float64: %s", e) + return nil, nil, false, fmt.Errorf("could not parse price as float64: %s", e) } // use the existing offer if the price is the same so we don't recreate an offer unnecessarily + offerAsOp := convertOffer2MSO(nextOffer) if opPrice < offerPrice { - return mso, nil, opCounter, false, nil + return mso, opCounter, false, nil } + return offerAsOp, offerCounter, false, nil +} - offerAsOp := convertOffer2MSO(existingOffer) - offerAsOpCopy := *offerAsOp - return offerAsOp, &offerAsOpCopy, offerCounter, false, nil +// fetchOfferAsOpByID returns the offer as an op if it exists otherwise nil +func fetchOfferAsOpByID(offerID int64, offerMap map[int64]horizon.Offer) *txnbuild.ManageSellOffer { + if offer, exists := offerMap[offerID]; exists { + return convertOffer2MSO(offer) + } + return nil } func runInnerFilterFn( @@ -264,66 +291,50 @@ func runInnerFilterFn( fn filterFn, originalOfferAsOp *txnbuild.ManageSellOffer, originalMSO txnbuild.ManageSellOffer, // passed by value so it doesn't change - offerCounter *filterCounter, - opCounter *filterCounter, ) ( - newOpToAppend *txnbuild.ManageSellOffer, newOpToPrepend *txnbuild.ManageSellOffer, - filterCounterToIncrement *filterCounter, - incrementValues *filterCounter, + newOpToAppend *txnbuild.ManageSellOffer, + incrementValues filterCounter, err error, ) { - var keep bool var newOp *txnbuild.ManageSellOffer var e error // delete operations should never be dropped if opToTransform.Amount == "0" { - newOp, keep = &opToTransform, true + newOp = &opToTransform } else { - newOp, keep, e = fn(&opToTransform) + newOp, e = fn(&opToTransform) if e != nil { - return nil, nil, nil, nil, fmt.Errorf("error in inner filter fn: %s", e) + return nil, nil, filterCounter{}, fmt.Errorf("error in inner filter fn: %s", e) } } - isNewOpNil := newOp == nil || fmt.Sprintf("%v", newOp) == "" - if keep && isNewOpNil { - return nil, nil, nil, nil, fmt.Errorf("we want to keep op but newOp was nil (programmer error?)") - } else if keep { + keep := newOp != nil && fmt.Sprintf("%v", newOp) != "" + if keep { if originalOfferAsOp != nil && originalOfferAsOp.Price == newOp.Price && originalOfferAsOp.Amount == newOp.Amount { // do not append to filteredOps because this is an existing offer that we want to keep as-is - return nil, nil, offerCounter, &filterCounter{kept: 1}, nil + return nil, nil, filterCounter{kept: 1}, nil } else if originalOfferAsOp != nil { // we were dealing with an existing offer that was modified - return newOp, nil, offerCounter, &filterCounter{transformed: 1}, nil + return nil, newOp, filterCounter{transformed: 1}, nil } else { // we were dealing with an operation opModified := originalMSO.Price != newOp.Price || originalMSO.Amount != newOp.Amount if opModified { - return newOp, nil, opCounter, &filterCounter{transformed: 1}, nil + return nil, newOp, filterCounter{transformed: 1}, nil } - return newOp, nil, opCounter, &filterCounter{kept: 1}, nil - } - } else if isNewOpNil { - if originalOfferAsOp != nil { - // if newOp is nil for an original offer it means we want to keep that offer. - return nil, nil, offerCounter, &filterCounter{kept: 1}, nil - } else { - // if newOp is nil and it is not an original offer it means we want to drop the operation. - return nil, nil, opCounter, &filterCounter{dropped: 1}, nil + return nil, newOp, filterCounter{kept: 1}, nil } } else { - // newOp can be a transformed op to change the op to an effectively "dropped" state - // prepend this so we always have delete commands at the beginning of the operation list if originalOfferAsOp != nil { - // we are dealing with an existing offer that needs dropping - return nil, newOp, offerCounter, &filterCounter{dropped: 1}, nil + // if newOp is nil for an original offer it means we want to explicitly delete that offer + opCopy := *originalOfferAsOp + opCopy.Amount = "0" + return &opCopy, nil, filterCounter{dropped: 1}, nil } else { - // we are dealing with an operation that had updated an offer which now needs dropping - // using the transformed counter here because we are changing the actual intent of the operation - // from an update existing offer to delete existing offer logic - return nil, newOp, opCounter, &filterCounter{transformed: 1}, nil + // if newOp is nil and it is not an original offer it means we want to drop the operation. + return nil, nil, filterCounter{dropped: 1}, nil } } } @@ -331,27 +342,24 @@ func runInnerFilterFn( func handleRemainingOffers( offerCounter *filterCounter, offers []hProtocol.Offer, - opCounter *filterCounter, ignoreOfferIds map[int64]bool, filteredOps []txnbuild.Operation, fn filterFn, ) ([]txnbuild.Operation, error) { for offerCounter.idx < len(offers) { if _, ignoreOffer := ignoreOfferIds[offers[offerCounter.idx].ID]; ignoreOffer { - // we want to only compare against valid offers so ignore this offer - offerCounter.ignored++ + // don't increment anything here becuase it was already addressed with the op that updated the offer + // so just move on to the next one offerCounter.idx++ continue } originalOfferAsOp := convertOffer2MSO(offers[offerCounter.idx]) - newOpToAppend, newOpToPrepend, filterCounterToIncrement, incrementValues, e := runInnerFilterFn( + newOpToPrepend, newOpToAppend, incrementValues, e := runInnerFilterFn( *originalOfferAsOp, // pass copy fn, originalOfferAsOp, *originalOfferAsOp, // pass copy - offerCounter, - opCounter, ) if e != nil { return nil, fmt.Errorf("error while running inner filter function for remaining offers: %s", e) @@ -362,9 +370,7 @@ func handleRemainingOffers( if newOpToPrepend != nil { filteredOps = append([]txnbuild.Operation{newOpToPrepend}, filteredOps...) } - if filterCounterToIncrement != nil && incrementValues != nil { - filterCounterToIncrement.add(*incrementValues) - } + offerCounter.add(incrementValues) offerCounter.idx++ } return filteredOps, nil diff --git a/plugins/volumeFilter.go b/plugins/volumeFilter.go index 9c725ceed..a67b3b4f8 100644 --- a/plugins/volumeFilter.go +++ b/plugins/volumeFilter.go @@ -126,7 +126,7 @@ func (f *volumeFilter) Apply(ops []txnbuild.Operation, sellingOffers []hProtocol SellBaseAssetCapInQuoteUnits: &dailyTbbSellQuote, } - innerFn := func(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, bool, error) { + innerFn := func(op *txnbuild.ManageSellOffer) (*txnbuild.ManageSellOffer, error) { return f.volumeFilterFn(dailyOTB, dailyTBB, op) } ops, e = filterOps(f.name, f.baseAsset, f.quoteAsset, sellingOffers, buyingOffers, ops, innerFn) @@ -136,20 +136,20 @@ 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, bool, error) { +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) if e != nil { - return nil, false, fmt.Errorf("error when running the isSelling check: %s", e) + return nil, fmt.Errorf("error when running the isSelling check: %s", e) } sellPrice, e := strconv.ParseFloat(op.Price, 64) if e != nil { - return nil, false, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) + return nil, fmt.Errorf("could not convert price (%s) to float: %s", op.Price, e) } amountValueUnitsBeingSold, e := strconv.ParseFloat(op.Amount, 64) if e != nil { - return nil, false, fmt.Errorf("could not convert amount (%s) to float: %s", op.Amount, e) + return nil, fmt.Errorf("could not convert amount (%s) to float: %s", op.Amount, e) } if isSell { @@ -197,23 +197,14 @@ func (f *volumeFilter) volumeFilterFn(dailyOTB *VolumeFilterConfig, dailyTBB *Vo // update the dailyTBB to include the additional amounts so they can be used in the calculation of the next operation *dailyTBB.SellBaseAssetCapInBaseUnits += newAmountBeingSold *dailyTBB.SellBaseAssetCapInQuoteUnits += (newAmountBeingSold * sellPrice) - return opToReturn, true, nil + return opToReturn, nil } } else { // TODO buying side } - // convert the offer to a dropped state - if op.OfferID == 0 { - // new offers can be dropped - return nil, false, nil - } else if op.Amount != "0" { - // modify offers should be converted to delete offers - opCopy := *op - opCopy.Amount = "0" - return &opCopy, false, nil - } - return nil, false, fmt.Errorf("unable to transform manageOffer operation: offerID=%d, amount=%s, price=%.7f", op.OfferID, op.Amount, sellPrice) + // we don't want to keep it so return the dropped command + return nil, nil } func (c *VolumeFilterConfig) isEmpty() bool {