Custom strategies could be more complex if UpdateWithOps was strategy specific

[Reidmcc]

Desired Behavior

I would like to be able to customize more of Kelp's logic than what is currently possible. Currently custom strategy logic is confined mostly to the level providers. However, strategic logic also resides in parts of sellSideStrategy, namely UpdateWithOps and modifySellLevel. Having that logic in a static part of the code limits customization possibilities.

Impact

This would enable users to create more complex customized strategies without messing with sellSideStrategy and changing things for all strategies. I think it also makes sense to have functions that contain level-setting logic be part of the strategy code in general.

Feature Suggestion

We could do this by converting UpdateWithOps and modifySellLevel to work the way level providers do. They would be constructed as part of strategy generation

Additional context

The level providers do allow quite a bit of customization, including the idea I was thinking through when I thought of this, #52. However, the proposed change would enable much more complex logic than that issue.

Specification

We would have the functions in strategy specific files, similar to the files that define level providers. The current static version would be kept for use by strategies that don't need to customize these functions.

The part of the strategy factory methods that generates the sellSideStrategy would change to be something like this:

sellSideStrategy := makeSellSideStrategy(
		sdex,
		assetBase,
		assetQuote,
		makeBalancedLevelProvider(
			config.Spread,
			false,
			config.MinAmountSpread,
			config.MaxAmountSpread,
			config.MaxLevels,
			config.LevelDensity,
			config.EnsureFirstNLevels,
			config.MinAmountCarryoverSpread,
			config.MaxAmountCarryoverSpread,
			config.CarryoverInclusionProbability,
			config.VirtualBalanceBase,
			config.VirtualBalanceQuote),
                makeBalancedUpdater(
                        ParamOne,
                        ParamTwo,)
		config.PriceTolerance,
		config.AmountTolerance,
		false,
	)

A new type would be needed in api/level:

type Updater interface {
	UpdateWithOps(offers []horizon.Offer) (ops []build.TransactionMutator, newTopOffer *model.Number, e error) ,
	modifySellLevel(offers []horizon.Offer, index int, targetPrice model.Number, targetAmount model.Number) (*model.Number, bool, *build.ManageOfferBuilder, error)
}

A additional parameter would be added to sellSideStrategy and makeSellSideStrategy

type sellSideStrategy struct {
	sdex                *SDEX
	assetBase           *horizon.Asset
	assetQuote          *horizon.Asset
	levelsProvider      api.LevelProvider
        updater              api.Updater
	priceTolerance      float64
	amountTolerance     float64
	divideAmountByPrice bool
	action              string

	// uninitialized
	currentLevels []api.Level // levels for current iteration
	maxAssetBase  float64
	maxAssetQuote float64
}

// ensure it implements SideStrategy
var _ api.SideStrategy = &sellSideStrategy{}

// makeSellSideStrategy is a factory method for sellSideStrategy
func makeSellSideStrategy(
	sdex *SDEX,
	assetBase *horizon.Asset,
	assetQuote *horizon.Asset,
	levelsProvider api.LevelProvider,
        updater api.Updater,
	priceTolerance float64,
	amountTolerance float64,
	divideAmountByPrice bool,
) api.SideStrategy {
	action := actionSell
	if divideAmountByPrice {
		action = actionBuy
	}
	return &sellSideStrategy{
		sdex:                sdex,
		assetBase:           assetBase,
		assetQuote:          assetQuote,
		levelsProvider:      levelsProvider,
		priceTolerance:      priceTolerance,
		amountTolerance:     amountTolerance,
		divideAmountByPrice: divideAmountByPrice,
		action:              action,
	}
}

I may well be missing somewhere else a change would be needed. This is obviously a big change, so I'm not going to code it up for a pull request unless approved.

[nikhilsaraf]

@Reidmcc I think that most (if not all) strategies can be coded using a level provider.
Can you provide an example of a strategy that would not fit into the levelProvider interface?

[Reidmcc]

An example would be if you wanted to implement criteria in modifySellLevel that aren't exactly the existing price and amount triggers. If you want to know how far out of tolerance the level is, the current modifySellLevel doesn't know that. You might want Kelp to change what it does to each level based on how much the price has changed. The level provider can implement logic based on what the price is, but not what the price was. This would apply for anything that requires comparing the current generated level with the previous offer, and wouldn't have to use the existing tolerance parameters.

Using UpdateWithOps you could implement logic based on what happened to the previous levels. For example, if you wanted to space your levels more widely if the price was moving more quickly, you could count the number of levels that needed modification and if it reaches a threshold regenerate the levels with a passed increased spread parameter.

[Reidmcc]

After working on #60 I now know that the level providers can store data across cycles, which would allow the examples I gave above to be done in the level provider.

The remaining case for this issue's suggestion is for customized strategies that use a completely different concept than a strategy that wants to replace offers that were filled. If you weren't doing market making that could well be the case. Metrics based swing trading would be an example. That may be out-of-scope for Kelp if it's intended to always be a market maker.

[Reidmcc]

I'm going to go ahead and close this. If a user requests something that requires this we can keep it in mind.