Protorev: Use hooks instead of message parsing to trigger backruns (#…

…5045) * Add input and output - Follows gamm module patterns such as 1) naming convention and 2) providing tokens (sdk.Coins) even though it's currently expected to be a single token in or out (sdk.Coin). * Update modules that use hook * Create new kvstore and methods for swpsToBackrun - With new hook based logic, need to persist the swaps to the posthandler. Doing so via storing in kvstored * Connect protorev to cfmm and cl hooks * Implement CFMMHook logic in protorev * Update comments, restrict join/exit pool logic to single denom in/out * Remove old placeholder print * Change init function to reset * add input and output coins in protorev cl hook * add necessary poolmanager expected keepers * add hook logic for cl pools and refactor * implement new hook-based logic in the posthandler * no-op unused hooks * Remove ResetSwapToBackrun, only use DeleteSwapsToBackrun * Add swap tests for hooks - Also creates a position in the cl pool defined in setupPools * Add join/exit pool hook tests - Limit len(coins) = 1 due to an exit pool message giving all denoms as exitdenoms (and potentially future behavior of other modules) * Add pool creation hook test * Add pool creation tests, handle CL pools differently due to 0 liquidity creation * Update ExtractSwappedPools to no longer need tx * Update TestExtractSwappedPools for new hook-based logic * Delete swaps without using user gas * Update TestAnteHandle with new hook-based logic * Update expected liquidity value for CL pool - CL pool now is created with pre-defined default values, so I remove sending coins to the pool in keeper_test.go - Updates the expected value in epoch_hook_test to match the output of the default PrepareConcentratedPoolWithCoinsAndFullRangePosition coins output * Add changelog entry * Remove unused messages in TestAnteHandle Since we now have hook-based logic, can remove msgs from inputs into TestAnteHandle() * Remove no longer used functions * Check len on all hooks to avoid unintended behavior * lint * Move all mul logic into same fn in hook logic afterpoolCreated, add panic catching to function * Separate getCoins logic into separate function * fix lint * return sdk.Coins{} instead of nil * remove unused import * Remove unnecessary code * clarify comment Co-authored-by: Sishir Giri <[email protected]> * use NewCoins instead of Coins Co-authored-by: Sishir Giri <[email protected]> * Revert "use NewCoins instead of Coins" This reverts commit 6c744ac. * add inline comment for why no return in err * use SetupTest instead of custom logic * Restructure protorev hook updating to only process pools with coins - Previously, protorev was triggered by AfterConcentratedPoolCreated, but this had an issue where the pool is created without any liquidity, returning empty coins object when getting total liquidity - Instead, this commit makes AfterConcentratedPoolCreated a no-op and moves the core logic into AfterInitialPoolPositionCreated, so that we know we have coins - This allows us to not handle this case in any custom manner and can expect there to be coins - Added more test cases to ensure this works as expected * Use denoms first, then coins in pool updating - Does all logic it can just using denoms first, and only gets coins object if necessary * remove comment * use poolmanager to get liquidity to avoid issues with CL - Previously used gamm to get liquidity which would have error'd, so uses poolmanager instead - Separate specific logic into own function for re-use and testing * Add CL first, then balancer (and vice-versa) pool creation tests * v16 version bump * implement ok checking pattern for map check * fix typo * Use expectPass bool in test * reuse k.storeSwap in join/exit logic * Add direct unit test for StoreSwap helper method * Add direct unit test for GetComparablePoolLiquidity helper method * Add direct unit test for StoreJoinExitPoolSwaps helper method * Remove old function * Add direct unit test for CompareAndStorePool helper method * Actually test CompareAndStorePool * export AfterPoolCreatedWithCoins * Clarify why the test calls DeleteAllPoolsForBaseDenom * Move panic catching into GetComparablePoolLiquidity - Panic catching was previously in CompareAndStorePool because that function did the multiplication. - Since then, the multiplication logic that this panic catches overflows on has been separated into it's own function GetComparablePoolLiquidity - This commit moves the panic catching logic to GetComparablePoolLiquidity * Add int overflow panic catching test * Revert "Add int overflow panic catching test" This reverts commit ed66bce. * Add int overflow panic catch test - try 2 * Add custom error message * Add overflow catching logical branch tests for CompareAndStorePool * Add non-gamm pool error test for StoreJoinExitPoolSwaps() * Use s.SetupTest() and give testAcc pool shares * Add clarifying comment about gas meter * Add comment explaining the base denoms check -> CompareAndStorePool logic --------- Co-authored-by: Sishir Giri <[email protected]>
osmosis-labs · Jun 6, 2023 · c6697a5 · c6697a5
1 parent 314c771
commit c6697a5
Show file tree

Hide file tree

Showing 11 changed files with 1,231 additions and 450 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   * [#4830](https://github.com/osmosis-labs/osmosis/pull/4830) Add gas cost when we AddToGaugeRewards, linearly increase with coins to add
   * [#4886](https://github.com/osmosis-labs/osmosis/pull/4886) Implement MsgSplitRouteSwapExactAmountIn and MsgSplitRouteSwapExactAmountOut that supports route splitting.
   * [#5000](https://github.com/osmosis-labs/osmosis/pull/5000) osmomath.Power panics for base < 1 to temporarily restrict broken logic for such base.
+  * [#5045] (https://github.com/osmosis-labs/osmosis/pull/5045) Implement hook-based backrunning logic for ProtoRev
   * [#5281](https://github.com/osmosis-labs/osmosis/pull/5281) Add option to designate Reward Recipient to Lock and Incentives.
   * [#4827] (https://github.com/osmosis-labs/osmosis/pull/4827) Protorev: Change highest liquidity pool updating from weekly to daily and change dev fee payout from weekly to after every trade.
 

diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go
@@ -690,13 +690,15 @@ func (appKeepers *AppKeepers) SetupHooks() {
 			// insert gamm hooks receivers here
 			appKeepers.PoolIncentivesKeeper.Hooks(),
 			appKeepers.TwapKeeper.GammHooks(),
+			appKeepers.ProtoRevKeeper.Hooks(),
 		),
 	)
 
 	appKeepers.ConcentratedLiquidityKeeper.SetListeners(
 		concentratedliquiditytypes.NewConcentratedLiquidityListeners(
 			appKeepers.TwapKeeper.ConcentratedLiquidityListener(),
 			appKeepers.PoolIncentivesKeeper.Hooks(),
+			appKeepers.ProtoRevKeeper.Hooks(),
 		),
 	)
 

diff --git a/x/protorev/keeper/epoch_hook_test.go b/x/protorev/keeper/epoch_hook_test.go
@@ -162,7 +162,7 @@ func (s *KeeperTestSuite) TestUpdateHighestLiquidityPools() {
 			},
 			expectedBaseDenomPools: map[string]map[string]keeper.LiquidityPoolStruct{
 				"epochTwo": {
-					"uosmo": {Liquidity: sdk.NewInt(2000000), PoolId: 49},
+					"uosmo": {Liquidity: sdk.Int(sdk.NewUintFromString("999999999000000001000000000000000000")), PoolId: 49},
 				},
 			},
 		},

diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go
@@ -0,0 +1,222 @@
+package keeper
+
+import (
+	"errors"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	gammtypes "github.com/osmosis-labs/osmosis/v16/x/gamm/types"
+	"github.com/osmosis-labs/osmosis/v16/x/protorev/types"
+)
+
+type Hooks struct {
+	k Keeper
+}
+
+var (
+	_ gammtypes.GammHooks = Hooks{}
+)
+
+// Create new ProtoRev hooks.
+func (k Keeper) Hooks() Hooks { return Hooks{k} }
+
+// ----------------------------------------------------------------------------
+// GAMM HOOKS
+// ----------------------------------------------------------------------------
+
+// AfterCFMMPoolCreated hook checks and potentially stores the pool via the highest liquidity method.
+func (h Hooks) AfterCFMMPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) {
+	h.k.AfterPoolCreatedWithCoins(ctx, poolId)
+}
+
+// AfterJoinPool stores swaps to be checked by protorev given the coins entered into the pool.
+func (h Hooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) {
+	// Checked to avoid future unintended behavior based on how the hook is called
+	if len(enterCoins) != 1 {
+		return
+	}
+
+	h.k.StoreJoinExitPoolSwaps(ctx, sender, poolId, enterCoins[0].Denom, true)
+}
+
+// AfterExitPool stores swaps to be checked by protorev given the coins exited from the pool.
+func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) {
+	// Added due to ExitSwapShareAmountIn both calling
+	// ExitPoolHook with all denoms of the pool and then also
+	// Swapping which triggers the after swap hook.
+	// So this filters out the exit pool hook call with all denoms
+	if len(exitCoins) != 1 {
+		return
+	}
+
+	h.k.StoreJoinExitPoolSwaps(ctx, sender, poolId, exitCoins[0].Denom, false)
+}
+
+// AfterCFMMSwap stores swaps to be checked by protorev given the coins swapped in the pool.
+func (h Hooks) AfterCFMMSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) {
+	// Checked to avoid future unintended behavior based on how the hook is called
+	if len(input) != 1 || len(output) != 1 {
+		return
+	}
+
+	h.k.StoreSwap(ctx, poolId, input[0].Denom, output[0].Denom)
+}
+
+// ----------------------------------------------------------------------------
+// CONCENTRATED LIQUIDITY HOOKS
+// ----------------------------------------------------------------------------
+
+// AfterConcentratedPoolCreated is a noop.
+func (h Hooks) AfterConcentratedPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) {
+}
+
+// AfterInitialPoolPositionCreated checks and potentially stores the pool via the highest liquidity method.
+func (h Hooks) AfterInitialPoolPositionCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) {
+	h.k.AfterPoolCreatedWithCoins(ctx, poolId)
+}
+
+// AfterLastPoolPositionRemoved is a noop.
+func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) {
+}
+
+// AfterConcentratedPoolSwap stores swaps to be checked by protorev given the coins swapped in the pool.
+func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) {
+	// Checked to avoid future unintended behavior based on how the hook is called
+	if len(input) != 1 || len(output) != 1 {
+		return
+	}
+
+	h.k.StoreSwap(ctx, poolId, input[0].Denom, output[0].Denom)
+}
+
+// ----------------------------------------------------------------------------
+// HELPER METHODS
+// ----------------------------------------------------------------------------
+
+// StoreSwap stores a swap to be checked by protorev when attempting backruns.
+func (k Keeper) StoreSwap(ctx sdk.Context, poolId uint64, tokenIn, tokenOut string) {
+	swapToBackrun := types.Trade{
+		Pool:     poolId,
+		TokenIn:  tokenIn,
+		TokenOut: tokenOut,
+	}
+
+	if err := k.AddSwapsToSwapsToBackrun(ctx, []types.Trade{swapToBackrun}); err != nil {
+		ctx.Logger().Error("Protorev error adding swap to backrun from storeSwap", err) // Does not return since logging is last thing in the function
+	}
+}
+
+// GetComparablePoolLiquidity gets the comparable liquidity of a pool by multiplying the amounts of the pool coins.
+func (k Keeper) GetComparablePoolLiquidity(ctx sdk.Context, poolId uint64) (comparableLiquidity sdk.Int, err error) {
+	coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId)
+	if err != nil {
+		return sdk.Int{}, err
+	}
+
+	// Recover from overflow panic
+	defer func() {
+		if r := recover(); r != nil {
+			comparableLiquidity = sdk.Int{}
+			err = errors.New("Int overflow in GetComparablePoolLiquidity")
+		}
+	}()
+
+	comparableLiquidity = coins[0].Amount.Mul(coins[1].Amount)
+
+	return comparableLiquidity, nil
+}
+
+// StoreJoinExitPoolSwaps stores the swaps associated with GAMM join/exit pool messages in the store, depending on if it is a join or exit.
+func (k Keeper) StoreJoinExitPoolSwaps(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, denom string, isJoin bool) {
+	pool, err := k.gammKeeper.GetPoolAndPoke(ctx, poolId)
+	if err != nil {
+		return
+	}
+
+	// Get all the pool coins and iterate to get the denoms that make up the swap
+	coins := pool.GetTotalPoolLiquidity(ctx)
+
+	// Create swaps to backrun being the join coin swapped against all other pool coins
+	for _, coin := range coins {
+		if coin.Denom == denom {
+			continue
+		}
+		// Join messages swap in the denom, exit messages swap out the denom
+		if isJoin {
+			k.StoreSwap(ctx, poolId, denom, coin.Denom)
+		} else {
+			k.StoreSwap(ctx, poolId, coin.Denom, denom)
+		}
+	}
+}
+
+// AfterPoolCreatedWithCoins checks if the new pool should be stored as the highest liquidity pool
+// for any of the base denoms, and stores it if so.
+func (k Keeper) AfterPoolCreatedWithCoins(ctx sdk.Context, poolId uint64) {
+	baseDenoms, err := k.GetAllBaseDenoms(ctx)
+	if err != nil {
+		ctx.Logger().Error("Protorev error getting base denoms in AfterCFMMPoolCreated hook", err)
+		return
+	}
+
+	baseDenomMap := make(map[string]bool)
+	for _, baseDenom := range baseDenoms {
+		baseDenomMap[baseDenom.Denom] = true
+	}
+
+	pool, err := k.poolmanagerKeeper.GetPool(ctx, poolId)
+	if err != nil {
+		ctx.Logger().Error("Protorev error getting pool in AfterCFMMPoolCreated hook", err)
+		return
+	}
+
+	denoms, err := k.poolmanagerKeeper.RouteGetPoolDenoms(ctx, poolId)
+	if err != nil {
+		ctx.Logger().Error("Protorev error getting pool liquidity in afterPoolCreated", err)
+		return
+	}
+
+	// Pool must be active and the number of denoms must be 2
+	if pool.IsActive(ctx) && len(denoms) == 2 {
+		// Check if either of the denoms are base denoms (denoms in which we store highest liquidity
+		// pools for to create backrun routes). If so, we call CompareAndStorePool which will check
+		// if a pool already exists for the base denom pair, and if not, stores the new pool.
+		// If a pool does already exist for the base denom pair, it will compare the liquidity
+		// of the new pool with the stored pool, and store the new pool if it has more liquidity.
+		if _, ok := baseDenomMap[denoms[0]]; ok {
+			k.CompareAndStorePool(ctx, poolId, denoms[0], denoms[1])
+		}
+		if _, ok := baseDenomMap[denoms[1]]; ok {
+			k.CompareAndStorePool(ctx, poolId, denoms[1], denoms[0])
+		}
+	}
+}
+
+// CompareAndStorePool compares the liquidity of the new pool with the liquidity of the stored pool, and stores the new pool if it has more liquidity.
+func (k Keeper) CompareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, otherDenom string) {
+	storedPoolId, err := k.GetPoolForDenomPair(ctx, baseDenom, otherDenom)
+	if err != nil {
+		// Error means no pool exists for this pair, so we set it
+		k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId)
+		return
+	}
+
+	// Get comparable liquidity for the new pool
+	newPoolLiquidity, err := k.GetComparablePoolLiquidity(ctx, poolId)
+	if err != nil {
+		ctx.Logger().Error("Protorev error getting newPoolLiquidity in compareAndStorePool", err)
+		return
+	}
+
+	// Get comparable liquidity for the stored pool
+	storedPoolLiquidity, err := k.GetComparablePoolLiquidity(ctx, storedPoolId)
+	if err != nil {
+		ctx.Logger().Error("Protorev error getting storedPoolLiquidity in compareAndStorePool", err)
+		return
+	}
+
+	// If the new pool has more liquidity, we set it
+	if newPoolLiquidity.GT(storedPoolLiquidity) {
+		k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId)
+	}
+}