From 02ddd55d7550f07f513d6ff87feb0a984f35d90c Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 18 Apr 2023 02:22:49 -0400 Subject: [PATCH 01/67] 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). --- x/concentrated-liquidity/clmocks/listeners.go | 2 +- x/concentrated-liquidity/swaps.go | 2 +- x/concentrated-liquidity/types/listeners.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/concentrated-liquidity/clmocks/listeners.go b/x/concentrated-liquidity/clmocks/listeners.go index 45d8aa19cf6..82af37a6db5 100644 --- a/x/concentrated-liquidity/clmocks/listeners.go +++ b/x/concentrated-liquidity/clmocks/listeners.go @@ -27,6 +27,6 @@ func (l *ConcentratedLiquidityListenerMock) AfterLastPoolPositionRemoved(ctx sdk l.AfterLastPoolPositionRemovedCallCount += 1 } -func (l *ConcentratedLiquidityListenerMock) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { +func (l *ConcentratedLiquidityListenerMock) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) { l.AfterConcentratedPoolSwapCallCount += 1 } diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 459d8f4f310..3d51abd5483 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -619,7 +619,7 @@ func (k Keeper) updatePoolForSwap( return err } - k.listeners.AfterConcentratedPoolSwap(ctx, sender, poolId) + k.listeners.AfterConcentratedPoolSwap(ctx, sender, poolId, sdk.Coins{tokenIn}, sdk.Coins{tokenOut}) // TODO: move this to poolmanager and remove from here. // Also, remove from gamm. diff --git a/x/concentrated-liquidity/types/listeners.go b/x/concentrated-liquidity/types/listeners.go index 48d38696912..2660a60ab5b 100644 --- a/x/concentrated-liquidity/types/listeners.go +++ b/x/concentrated-liquidity/types/listeners.go @@ -12,7 +12,7 @@ type ConcentratedLiquidityListener interface { // liquidity pool. AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) // AfterConcentratedPoolSwap is called after a swap in a concentrated liquidity pool. - AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) + AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) } type ConcentratedLiquidityListeners []ConcentratedLiquidityListener @@ -37,9 +37,9 @@ func (l ConcentratedLiquidityListeners) AfterLastPoolPositionRemoved(ctx sdk.Con } } -func (l ConcentratedLiquidityListeners) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { +func (l ConcentratedLiquidityListeners) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) { for i := range l { - l[i].AfterConcentratedPoolSwap(ctx, sender, poolId) + l[i].AfterConcentratedPoolSwap(ctx, sender, poolId, input, output) } } From 75ea9cc9baf2beea40ee6a99b56fd49cdf8a9595 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 18 Apr 2023 02:23:14 -0400 Subject: [PATCH 02/67] Update modules that use hook --- x/pool-incentives/keeper/hooks.go | 2 +- x/twap/listeners.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/pool-incentives/keeper/hooks.go b/x/pool-incentives/keeper/hooks.go index 7576eb724dc..ddef4e9892c 100644 --- a/x/pool-incentives/keeper/hooks.go +++ b/x/pool-incentives/keeper/hooks.go @@ -80,6 +80,6 @@ func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddre } // AfterConcentratedPoolSwap is a noop. -func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { +func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) { } diff --git a/x/twap/listeners.go b/x/twap/listeners.go index 1b462781cfa..e2a55f0ce5a 100644 --- a/x/twap/listeners.go +++ b/x/twap/listeners.go @@ -80,6 +80,6 @@ func (l *concentratedLiquidityListener) AfterLastPoolPositionRemoved(ctx sdk.Con l.k.trackChangedPool(ctx, poolId) } -func (l *concentratedLiquidityListener) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { +func (l *concentratedLiquidityListener) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) { l.k.trackChangedPool(ctx, poolId) } From 772af1b2ef556a4806b68eed07ad497ab556a51e Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 17 Apr 2023 16:36:14 -0400 Subject: [PATCH 03/67] 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 --- x/protorev/keeper/protorev.go | 83 +++++++++++++++++++++++++++++++++++ x/protorev/types/keys.go | 4 ++ 2 files changed, 87 insertions(+) diff --git a/x/protorev/keeper/protorev.go b/x/protorev/keeper/protorev.go index 443884d157a..f660d4c7728 100644 --- a/x/protorev/keeper/protorev.go +++ b/x/protorev/keeper/protorev.go @@ -146,6 +146,89 @@ func (k Keeper) DeleteAllPoolsForBaseDenom(ctx sdk.Context, baseDenom string) { k.DeleteAllEntriesForKeyPrefix(ctx, key) } +// SetSwapsToBackrun sets the swaps to backrun, updated via hooks +func (k Keeper) SetSwapsToBackrun(ctx sdk.Context, swapsToBackrun types.Route) error { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixSwapsToBackrun) + + bz, err := swapsToBackrun.Marshal() + if err != nil { + return err + } + + store.Set(types.KeyPrefixSwapsToBackrun, bz) + + return nil +} + +// GetSwapsToBackrun returns the swaps to backrun, updated via hooks +func (k Keeper) GetSwapsToBackrun(ctx sdk.Context) (types.Route, error) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixSwapsToBackrun) + bz := store.Get(types.KeyPrefixSwapsToBackrun) + + swapsToBackrun := types.Route{} + err := swapsToBackrun.Unmarshal(bz) + if err != nil { + return types.Route{}, err + } + + return swapsToBackrun, nil +} + +// InitSwapsToBackrun initializes the swaps to backrun store +func (k Keeper) InitSwapsToBackrun(ctx sdk.Context) error { + swapsToBackrun := types.Route{ + Trades: []types.Trade{}, + StepSize: sdk.NewInt(1), + } + + err := k.SetSwapsToBackrun(ctx, swapsToBackrun) + if err != nil { + return err + } + + return nil +} + +// DeleteSwapsToBackrun deletes the swaps to backrun +func (k Keeper) DeleteSwapsToBackrun(ctx sdk.Context) { + store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixSwapsToBackrun) + store.Delete(types.KeyPrefixSwapsToBackrun) +} + +// AddSwapToSwapsToBackrun appends a swap to the swaps to backrun +func (k Keeper) AddSwapsToSwapsToBackrun(ctx sdk.Context, swaps []types.Trade) error { + swapsToBackrun, err := k.GetSwapsToBackrun(ctx) + if err != nil { + return err + } + + swapsToBackrun.Trades = append(swapsToBackrun.Trades, swaps...) + + err = k.SetSwapsToBackrun(ctx, swapsToBackrun) + if err != nil { + return err + } + + return nil +} + +// ResetSwapsToBackrun clears out the swaps stored in SwapsToBackrun +func (k Keeper) ResetSwapsToBackrun(ctx sdk.Context) error { + swapsToBackrun, err := k.GetSwapsToBackrun(ctx) + if err != nil { + return err + } + + swapsToBackrun.Trades = []types.Trade{} + + err = k.SetSwapsToBackrun(ctx, swapsToBackrun) + if err != nil { + return err + } + + return nil +} + // DeleteAllEntriesForKeyPrefix deletes all the entries from the store for the given key prefix func (k Keeper) DeleteAllEntriesForKeyPrefix(ctx sdk.Context, keyPrefix []byte) { store := ctx.KVStore(k.storeKey) diff --git a/x/protorev/types/keys.go b/x/protorev/types/keys.go index cb4566ad141..86aeb24733a 100644 --- a/x/protorev/types/keys.go +++ b/x/protorev/types/keys.go @@ -35,6 +35,7 @@ const ( prefixPoolPointCountForBlock prefixLatestBlockHeight prefixPoolWeights + prefixSwapsToBackrun ) var ( @@ -85,6 +86,9 @@ var ( // KeyPrefixPoolWeights is the prefix for store that keeps track of the weights for different pool types KeyPrefixPoolWeights = []byte{prefixPoolWeights} + + // KeyPrefixSwapsToBackrun is the prefix for store that keeps track of the swaps that need to be backrun for a given tx + KeyPrefixSwapsToBackrun = []byte{prefixSwapsToBackrun} ) // Returns the key needed to fetch the pool id for a given denom From 089d0910646dcd41de177c31df3713402e677bd0 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 17 Apr 2023 16:37:13 -0400 Subject: [PATCH 04/67] Connect protorev to cfmm and cl hooks --- app/keepers/keepers.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/keepers/keepers.go b/app/keepers/keepers.go index e6217d998b4..9dd72ab9ae3 100644 --- a/app/keepers/keepers.go +++ b/app/keepers/keepers.go @@ -674,6 +674,7 @@ func (appKeepers *AppKeepers) SetupHooks() { // insert gamm hooks receivers here appKeepers.PoolIncentivesKeeper.Hooks(), appKeepers.TwapKeeper.GammHooks(), + appKeepers.ProtoRevKeeper.Hooks(), ), ) @@ -681,6 +682,7 @@ func (appKeepers *AppKeepers) SetupHooks() { concentratedliquiditytypes.NewConcentratedLiquidityListeners( appKeepers.TwapKeeper.ConcentratedLiquidityListener(), appKeepers.PoolIncentivesKeeper.Hooks(), + appKeepers.ProtoRevKeeper.Hooks(), ), ) From 1a6ed93195f3c93bd96571fef3b48b7a2dec81f1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 17 Apr 2023 16:37:22 -0400 Subject: [PATCH 05/67] Implement CFMMHook logic in protorev --- x/protorev/keeper/hooks.go | 174 +++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 x/protorev/keeper/hooks.go diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go new file mode 100644 index 00000000000..b4b4b22f338 --- /dev/null +++ b/x/protorev/keeper/hooks.go @@ -0,0 +1,174 @@ +package keeper + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + + gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" + "github.com/osmosis-labs/osmosis/v15/x/protorev/types" +) + +type Hooks struct { + k Keeper +} + +var ( + _ gammtypes.GammHooks = Hooks{} +) + +// Create new ProtoRev hooks. +func (k Keeper) Hooks() Hooks { return Hooks{k} } + +// AfterCFMMPoolCreated hook is a noop. +func (h Hooks) AfterCFMMPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + baseDenoms, err := h.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 := h.k.gammKeeper.GetPoolAndPoke(ctx, poolId) + if err != nil { + ctx.Logger().Error("Protorev error getting pool in AfterCFMMPoolCreated hook", err) + return + } + + coins := pool.GetTotalPoolLiquidity(ctx) + + // Pool must be active and the number of coins must be 2 + if pool.IsActive(ctx) && len(coins) == 2 { + tokenA := coins[0] + tokenB := coins[1] + + liquidity := tokenA.Amount.Mul(tokenB.Amount) + + if baseDenomMap[tokenA.Denom] { + h.k.compareAndStorePool(ctx, poolId, liquidity, tokenA.Denom, tokenB.Denom) + } + if baseDenomMap[tokenB.Denom] { + h.k.compareAndStorePool(ctx, poolId, liquidity, tokenB.Denom, tokenA.Denom) + } + } +} + +// AfterJoinPool hook is a noop. +func (h Hooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, enterCoins sdk.Coins, shareOutAmount sdk.Int) { + // TODO: Probably generalize to allow for more join denoms + // Right now, the main message only allows for a single join denom + // But I imagine we will want to allow for multiple join denoms in the future + // Since the function allows for multile join denoms, just not the main interface + joinDenom := enterCoins[0].Denom // As of v15, it is safe to assume only one input token + + h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, joinDenom, true) +} + +// AfterExitPool hook is a noop. +func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, shareInAmount sdk.Int, exitCoins sdk.Coins) { + fmt.Println("AfterExitPool hook is a noop in Protorev.") + + // TODO: Probably generalize to allow for more join denoms + // Right now, the main message only allows for a single exit denom + // But I imagine we will want to allow for multiple exit denoms in the future + // Since the function allows for multile exit denoms, just not the main interface + exitDenom := exitCoins[0].Denom // As of v15, it is safe to assume only one output token + + h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, exitDenom, false) +} + +// AfterCFMMSwap hook is a noop. +func (h Hooks) AfterCFMMSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) { + fmt.Println("AfterCFMMSwap hook is a noop in Protorev.") + + swapToBackrun := types.Trade{ + Pool: poolId, + TokenIn: input[0].Denom, // As of v15, it is safe to assume only one input token + TokenOut: output[0].Denom, // As of v15, it is safe to assume only one output token + } + + if err := h.k.AddSwapsToSwapsToBackrun(ctx, []types.Trade{swapToBackrun}); err != nil { + ctx.Logger().Error("Protorev error adding swap to backrun from AfterCFMMSwap hook", err) + } +} + +// AfterConcentratedPoolCreated creates a single gauge for the concentrated liquidity pool. +func (h Hooks) AfterConcentratedPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + fmt.Println("AfterConcentratedPoolCreated hook is a noop in Protorev.") +} + +// AfterInitialPoolPositionCreated is a noop. +func (h Hooks) AfterInitialPoolPositionCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + fmt.Println("AfterInitialPoolPositionCreated hook is a noop in Protorev.") +} + +// AfterLastPoolPositionRemoved is a noop. +func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + fmt.Println("AfterLastPoolPositionRemoved hook is a noop in Protorev.") +} + +// AfterConcentratedPoolSwap is a noop. +func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + fmt.Println("AfterConcentratedPoolSwap hook is a noop in Protorev.") +} + +func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, liquidity sdk.Int, 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 + } + + storedPool, err := k.gammKeeper.GetPoolAndPoke(ctx, storedPoolId) + if err != nil { + ctx.Logger().Error("Protorev error getting storedPool in AfterCFMMPoolCreated hook", err) + return + } + + storedPoolCoins := storedPool.GetTotalPoolLiquidity(ctx) + storedPoolLiquidity := storedPoolCoins[0].Amount.Mul(storedPoolCoins[1].Amount) + + // If the new pool has more liquidity, we set it + if liquidity.GT(storedPoolLiquidity) { + k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) + } +} + +func (k Keeper) storeJoinExitPoolMsgs(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 + swapsToBackrun := make([]types.Trade, 0) + for _, coin := range coins { + if coin.Denom == denom { + continue + } + // Join messages swap in the denom, exit messages swap out the denom + if isJoin { + swapsToBackrun = append(swapsToBackrun, types.Trade{ + Pool: poolId, + TokenIn: denom, + TokenOut: coin.Denom}) + } else { + swapsToBackrun = append(swapsToBackrun, types.Trade{ + Pool: poolId, + TokenIn: coin.Denom, + TokenOut: denom}) + } + } + + if err := k.AddSwapsToSwapsToBackrun(ctx, swapsToBackrun); err != nil { + ctx.Logger().Error("Protorev error adding swaps to backrun from AfterJoin/ExitPool hook", err) + } +} From dcc5549c0d0fba61c44a49806f2d0892deda7335 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 17 Apr 2023 16:43:59 -0400 Subject: [PATCH 06/67] Update comments, restrict join/exit pool logic to single denom in/out --- x/protorev/keeper/hooks.go | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index b4b4b22f338..c9736d07afd 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -20,7 +20,7 @@ var ( // Create new ProtoRev hooks. func (k Keeper) Hooks() Hooks { return Hooks{k} } -// AfterCFMMPoolCreated hook is a noop. +// 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) { baseDenoms, err := h.k.GetAllBaseDenoms(ctx) if err != nil { @@ -57,31 +57,33 @@ func (h Hooks) AfterCFMMPoolCreated(ctx sdk.Context, sender sdk.AccAddress, pool } } -// AfterJoinPool hook is a noop. +// 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) { // TODO: Probably generalize to allow for more join denoms // Right now, the main message only allows for a single join denom // But I imagine we will want to allow for multiple join denoms in the future // Since the function allows for multile join denoms, just not the main interface - joinDenom := enterCoins[0].Denom // As of v15, it is safe to assume only one input token + if len(enterCoins) != 1 { + return + } - h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, joinDenom, true) + h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, enterCoins[0].Denom, true) } -// AfterExitPool hook is a noop. +// 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) { - fmt.Println("AfterExitPool hook is a noop in Protorev.") - - // TODO: Probably generalize to allow for more join denoms + // TODO: Probably generalize to allow for more exit denoms // Right now, the main message only allows for a single exit denom // But I imagine we will want to allow for multiple exit denoms in the future // Since the function allows for multile exit denoms, just not the main interface - exitDenom := exitCoins[0].Denom // As of v15, it is safe to assume only one output token + if len(exitCoins) != 1 { + return + } - h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, exitDenom, false) + h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, exitCoins[0].Denom, false) } -// AfterCFMMSwap hook is a noop. +// 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) { fmt.Println("AfterCFMMSwap hook is a noop in Protorev.") @@ -116,6 +118,7 @@ func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, fmt.Println("AfterConcentratedPoolSwap hook is a noop in Protorev.") } +// 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, liquidity sdk.Int, baseDenom, otherDenom string) { storedPoolId, err := k.GetPoolForDenomPair(ctx, baseDenom, otherDenom) if err != nil { @@ -139,6 +142,7 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, liquidity sd } } +// storeJoinExitPoolMsgs stores the join/exit pool messages in the store, depending on if it is a join or exit. func (k Keeper) storeJoinExitPoolMsgs(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, denom string, isJoin bool) { pool, err := k.gammKeeper.GetPoolAndPoke(ctx, poolId) if err != nil { From 616f5dcf90f60656a0e63f256ed23118e02a56cf Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 17 Apr 2023 17:49:41 -0400 Subject: [PATCH 07/67] Remove old placeholder print --- x/protorev/keeper/hooks.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index c9736d07afd..48e6ebdd30f 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -85,8 +85,6 @@ func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint // 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) { - fmt.Println("AfterCFMMSwap hook is a noop in Protorev.") - swapToBackrun := types.Trade{ Pool: poolId, TokenIn: input[0].Denom, // As of v15, it is safe to assume only one input token From ef2d3175bff4386c219e94cde88011f4c481cb4d Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 17 Apr 2023 18:03:12 -0400 Subject: [PATCH 08/67] Change init function to reset --- x/protorev/keeper/protorev.go | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/x/protorev/keeper/protorev.go b/x/protorev/keeper/protorev.go index f660d4c7728..9174fdddb9a 100644 --- a/x/protorev/keeper/protorev.go +++ b/x/protorev/keeper/protorev.go @@ -175,7 +175,7 @@ func (k Keeper) GetSwapsToBackrun(ctx sdk.Context) (types.Route, error) { } // InitSwapsToBackrun initializes the swaps to backrun store -func (k Keeper) InitSwapsToBackrun(ctx sdk.Context) error { +func (k Keeper) ResetSwapsToBackrun(ctx sdk.Context) error { swapsToBackrun := types.Route{ Trades: []types.Trade{}, StepSize: sdk.NewInt(1), @@ -212,23 +212,6 @@ func (k Keeper) AddSwapsToSwapsToBackrun(ctx sdk.Context, swaps []types.Trade) e return nil } -// ResetSwapsToBackrun clears out the swaps stored in SwapsToBackrun -func (k Keeper) ResetSwapsToBackrun(ctx sdk.Context) error { - swapsToBackrun, err := k.GetSwapsToBackrun(ctx) - if err != nil { - return err - } - - swapsToBackrun.Trades = []types.Trade{} - - err = k.SetSwapsToBackrun(ctx, swapsToBackrun) - if err != nil { - return err - } - - return nil -} - // DeleteAllEntriesForKeyPrefix deletes all the entries from the store for the given key prefix func (k Keeper) DeleteAllEntriesForKeyPrefix(ctx sdk.Context, keyPrefix []byte) { store := ctx.KVStore(k.storeKey) From 9e71597304b0d30fc65452d26b91ef9df9cafaf1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Wed, 26 Apr 2023 20:17:58 -0400 Subject: [PATCH 09/67] add input and output coins in protorev cl hook --- x/protorev/keeper/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 48e6ebdd30f..c398d9f2706 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -112,7 +112,7 @@ func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddre } // AfterConcentratedPoolSwap is a noop. -func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { +func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) { fmt.Println("AfterConcentratedPoolSwap hook is a noop in Protorev.") } From 99d8e9b38d312a0b535810de8689fe878354070f Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 27 Apr 2023 18:59:47 -0400 Subject: [PATCH 10/67] add necessary poolmanager expected keepers --- x/protorev/types/expected_keepers.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/x/protorev/types/expected_keepers.go b/x/protorev/types/expected_keepers.go index 245eb13eba7..4d2fda0bca0 100644 --- a/x/protorev/types/expected_keepers.go +++ b/x/protorev/types/expected_keepers.go @@ -46,6 +46,16 @@ type PoolManagerKeeper interface { routes []poolmanagertypes.SwapAmountInRoute, tokenIn sdk.Coin, ) (tokenOutAmount sdk.Int, err error) + + AllPools( + ctx sdk.Context, + ) ([]poolmanagertypes.PoolI, error) + GetPool( + ctx sdk.Context, + poolId uint64, + ) (poolmanagertypes.PoolI, error) + GetPoolModule(ctx sdk.Context, poolId uint64) (poolmanagertypes.PoolModuleI, error) + GetTotalPoolLiquidity(ctx sdk.Context, poolId uint64) (sdk.Coins, error) } // EpochKeeper defines the Epoch contract that must be fulfilled when From 3212d18d48fcc52e68086194f3139c9e092ebb75 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 27 Apr 2023 19:00:09 -0400 Subject: [PATCH 11/67] add hook logic for cl pools and refactor --- x/protorev/keeper/hooks.go | 130 +++++++++++++++++++++---------------- 1 file changed, 73 insertions(+), 57 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index c398d9f2706..56a11ffc7d8 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -20,9 +20,67 @@ var ( // 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) { - baseDenoms, err := h.k.GetAllBaseDenoms(ctx) + h.k.afterPoolCreated(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) { + // TODO: Probably generalize to allow for more join denoms + h.k.storeJoinExitPoolMsgs(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) { + // TODO: Probably generalize to allow for more exit denoms + h.k.storeJoinExitPoolMsgs(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) { + // As of v15, it is safe to assume only one input token and one output token + // TODO: Probably generalize to allow for more swap denoms + h.k.storeSwap(ctx, poolId, input[0].Denom, output[0].Denom) +} + +// ---------------------------------------------------------------------------- +// CONCENTRATED LIQUIDITY HOOKS +// ---------------------------------------------------------------------------- + +// AfterConcentratedPoolCreated checks and potentially stores the pool via the highest liquidity method. +func (h Hooks) AfterConcentratedPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + h.k.afterPoolCreated(ctx, poolId) +} + +// AfterInitialPoolPositionCreated is a noop. +func (h Hooks) AfterInitialPoolPositionCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + fmt.Println("AfterInitialPoolPositionCreated hook is a noop in Protorev.") +} + +// AfterLastPoolPositionRemoved is a noop. +func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { + fmt.Println("AfterLastPoolPositionRemoved hook is a noop in Protorev.") +} + +// 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) { + // As of v15, it is safe to assume only one input token and one output token + // TODO: Probably generalize to allow for more swap denoms + h.k.storeSwap(ctx, poolId, input[0].Denom, output[0].Denom) +} + +// ---------------------------------------------------------------------------- +// HELPER METHODS +// ---------------------------------------------------------------------------- + +// afterPoolCreated checks if the new pool should be stored as the highest liquidity pool +func (k Keeper) afterPoolCreated(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 @@ -33,13 +91,17 @@ func (h Hooks) AfterCFMMPoolCreated(ctx sdk.Context, sender sdk.AccAddress, pool baseDenomMap[baseDenom.Denom] = true } - pool, err := h.k.gammKeeper.GetPoolAndPoke(ctx, poolId) + pool, err := k.poolmanagerKeeper.GetPool(ctx, poolId) if err != nil { ctx.Logger().Error("Protorev error getting pool in AfterCFMMPoolCreated hook", err) return } - coins := pool.GetTotalPoolLiquidity(ctx) + coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + if err != nil { + ctx.Logger().Error("Protorev error getting total pool liquidity in after swap hook", err) + return + } // Pool must be active and the number of coins must be 2 if pool.IsActive(ctx) && len(coins) == 2 { @@ -49,73 +111,27 @@ func (h Hooks) AfterCFMMPoolCreated(ctx sdk.Context, sender sdk.AccAddress, pool liquidity := tokenA.Amount.Mul(tokenB.Amount) if baseDenomMap[tokenA.Denom] { - h.k.compareAndStorePool(ctx, poolId, liquidity, tokenA.Denom, tokenB.Denom) + k.compareAndStorePool(ctx, poolId, liquidity, tokenA.Denom, tokenB.Denom) } if baseDenomMap[tokenB.Denom] { - h.k.compareAndStorePool(ctx, poolId, liquidity, tokenB.Denom, tokenA.Denom) + k.compareAndStorePool(ctx, poolId, liquidity, tokenB.Denom, tokenA.Denom) } } } -// 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) { - // TODO: Probably generalize to allow for more join denoms - // Right now, the main message only allows for a single join denom - // But I imagine we will want to allow for multiple join denoms in the future - // Since the function allows for multile join denoms, just not the main interface - if len(enterCoins) != 1 { - return - } - - h.k.storeJoinExitPoolMsgs(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) { - // TODO: Probably generalize to allow for more exit denoms - // Right now, the main message only allows for a single exit denom - // But I imagine we will want to allow for multiple exit denoms in the future - // Since the function allows for multile exit denoms, just not the main interface - if len(exitCoins) != 1 { - return - } - - h.k.storeJoinExitPoolMsgs(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) { +// 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: input[0].Denom, // As of v15, it is safe to assume only one input token - TokenOut: output[0].Denom, // As of v15, it is safe to assume only one output token + TokenIn: tokenIn, + TokenOut: tokenOut, } - if err := h.k.AddSwapsToSwapsToBackrun(ctx, []types.Trade{swapToBackrun}); err != nil { - ctx.Logger().Error("Protorev error adding swap to backrun from AfterCFMMSwap hook", err) + if err := k.AddSwapsToSwapsToBackrun(ctx, []types.Trade{swapToBackrun}); err != nil { + ctx.Logger().Error("Protorev error adding swap to backrun from storeSwap", err) } } -// AfterConcentratedPoolCreated creates a single gauge for the concentrated liquidity pool. -func (h Hooks) AfterConcentratedPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { - fmt.Println("AfterConcentratedPoolCreated hook is a noop in Protorev.") -} - -// AfterInitialPoolPositionCreated is a noop. -func (h Hooks) AfterInitialPoolPositionCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { - fmt.Println("AfterInitialPoolPositionCreated hook is a noop in Protorev.") -} - -// AfterLastPoolPositionRemoved is a noop. -func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { - fmt.Println("AfterLastPoolPositionRemoved hook is a noop in Protorev.") -} - -// AfterConcentratedPoolSwap is a noop. -func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, input sdk.Coins, output sdk.Coins) { - fmt.Println("AfterConcentratedPoolSwap hook is a noop in Protorev.") -} - // 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, liquidity sdk.Int, baseDenom, otherDenom string) { storedPoolId, err := k.GetPoolForDenomPair(ctx, baseDenom, otherDenom) From 9a73b348217caabc6107ed5db5983fe3a52dcc2f Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 27 Apr 2023 19:04:33 -0400 Subject: [PATCH 12/67] implement new hook-based logic in the posthandler --- x/protorev/keeper/posthandler.go | 35 ++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 295a36969cc..60f860a8df8 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -5,7 +5,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" - gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" ) @@ -50,7 +49,7 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu } // Extract all of the pools that were swapped in the tx - swappedPools := ExtractSwappedPools(tx) + swappedPools := protoRevDec.ProtoRevKeeper.ExtractSwappedPools(cacheCtx, tx) if len(swappedPools) == 0 { return next(ctx, tx, simulate) } @@ -63,6 +62,13 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu ctx.Logger().Error("ProtoRevTrade failed with error", err) } + // Reset swaps to backrun for next transaction + // TODO: Should this be placed before we attempt to execute trades? + if err := protoRevDec.ProtoRevKeeper.ResetSwapsToBackrun(ctx); err != nil { + ctx.Logger().Error("ResetSwapsToBackrun failed with error", err) + return next(ctx, tx, simulate) + } + return next(ctx, tx, simulate) } @@ -140,21 +146,20 @@ func (k Keeper) ProtoRevTrade(ctx sdk.Context, swappedPools []SwapToBackrun) (er // ExtractSwappedPools checks if there were any swaps made on pools and if so returns a list of all the pools that were // swapped on and metadata about the swap -func ExtractSwappedPools(tx sdk.Tx) []SwapToBackrun { +func (k Keeper) ExtractSwappedPools(ctx sdk.Context, tx sdk.Tx) []SwapToBackrun { swappedPools := make([]SwapToBackrun, 0) - // Extract only swaps types and the swapped pools from the tx - for _, msg := range tx.GetMsgs() { - switch msg := msg.(type) { - case *poolmanagertypes.MsgSwapExactAmountIn: - swappedPools = append(swappedPools, extractSwapInPools(msg.Routes, msg.TokenIn.Denom)...) - case *poolmanagertypes.MsgSwapExactAmountOut: - swappedPools = append(swappedPools, extractSwapOutPools(msg.Routes, msg.TokenOut.Denom)...) - case *gammtypes.MsgSwapExactAmountIn: - swappedPools = append(swappedPools, extractSwapInPools(msg.Routes, msg.TokenIn.Denom)...) - case *gammtypes.MsgSwapExactAmountOut: - swappedPools = append(swappedPools, extractSwapOutPools(msg.Routes, msg.TokenOut.Denom)...) - } + swapsToBackrun, err := k.GetSwapsToBackrun(ctx) + if err != nil { + return swappedPools + } + + for _, swap := range swapsToBackrun.Trades { + swappedPools = append(swappedPools, SwapToBackrun{ + PoolId: swap.Pool, + TokenInDenom: swap.TokenIn, + TokenOutDenom: swap.TokenOut, + }) } return swappedPools From 26c46a108389bf40f440d7b45d7af305f88fbeb1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 18 May 2023 15:19:05 -0400 Subject: [PATCH 13/67] no-op unused hooks --- x/protorev/keeper/hooks.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 56a11ffc7d8..c2e22206a3b 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -1,8 +1,6 @@ package keeper import ( - "fmt" - sdk "github.com/cosmos/cosmos-sdk/types" gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" @@ -59,12 +57,10 @@ func (h Hooks) AfterConcentratedPoolCreated(ctx sdk.Context, sender sdk.AccAddre // AfterInitialPoolPositionCreated is a noop. func (h Hooks) AfterInitialPoolPositionCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { - fmt.Println("AfterInitialPoolPositionCreated hook is a noop in Protorev.") } // AfterLastPoolPositionRemoved is a noop. func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { - fmt.Println("AfterLastPoolPositionRemoved hook is a noop in Protorev.") } // AfterConcentratedPoolSwap stores swaps to be checked by protorev given the coins swapped in the pool. @@ -79,6 +75,7 @@ func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, // ---------------------------------------------------------------------------- // afterPoolCreated 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) afterPoolCreated(ctx sdk.Context, poolId uint64) { baseDenoms, err := k.GetAllBaseDenoms(ctx) if err != nil { From 43b62ef65204b58623f16d902388af4684da0baa Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 18 May 2023 16:43:39 -0400 Subject: [PATCH 14/67] Remove ResetSwapToBackrun, only use DeleteSwapsToBackrun --- x/protorev/keeper/posthandler.go | 7 ++----- x/protorev/keeper/protorev.go | 15 --------------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index f73a6ea7f1e..640440435b0 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -62,12 +62,9 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu ctx.Logger().Error("ProtoRevTrade failed with error", err) } - // Reset swaps to backrun for next transaction + // Delete swaps to backrun for next transaction // TODO: Should this be placed before we attempt to execute trades? - if err := protoRevDec.ProtoRevKeeper.ResetSwapsToBackrun(ctx); err != nil { - ctx.Logger().Error("ResetSwapsToBackrun failed with error", err) - return next(ctx, tx, simulate) - } + protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx) return next(ctx, tx, simulate) } diff --git a/x/protorev/keeper/protorev.go b/x/protorev/keeper/protorev.go index 9174fdddb9a..c1b6d3998a1 100644 --- a/x/protorev/keeper/protorev.go +++ b/x/protorev/keeper/protorev.go @@ -174,21 +174,6 @@ func (k Keeper) GetSwapsToBackrun(ctx sdk.Context) (types.Route, error) { return swapsToBackrun, nil } -// InitSwapsToBackrun initializes the swaps to backrun store -func (k Keeper) ResetSwapsToBackrun(ctx sdk.Context) error { - swapsToBackrun := types.Route{ - Trades: []types.Trade{}, - StepSize: sdk.NewInt(1), - } - - err := k.SetSwapsToBackrun(ctx, swapsToBackrun) - if err != nil { - return err - } - - return nil -} - // DeleteSwapsToBackrun deletes the swaps to backrun func (k Keeper) DeleteSwapsToBackrun(ctx sdk.Context) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixSwapsToBackrun) From 6c8ab71b044b9fd2db5f419d918a0e824884f3a1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 18 May 2023 16:55:40 -0400 Subject: [PATCH 15/67] Add swap tests for hooks - Also creates a position in the cl pool defined in setupPools --- x/protorev/keeper/hooks_test.go | 136 +++++++++++++++++++++++++++++++ x/protorev/keeper/keeper_test.go | 3 +- 2 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 x/protorev/keeper/hooks_test.go diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go new file mode 100644 index 00000000000..1524508588b --- /dev/null +++ b/x/protorev/keeper/hooks_test.go @@ -0,0 +1,136 @@ +package keeper_test + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" + "github.com/osmosis-labs/osmosis/v15/x/protorev/types" +) + +func (s *KeeperTestSuite) TestSwaps() { + type param struct { + expectedTrades []types.Trade + executeSwap func() + } + + tests := []struct { + name string + param param + expectPass bool + }{ + { + name: "swap exact amount in", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executeSwap: func() { + _, err := s.App.PoolManagerKeeper.SwapExactAmountIn(s.Ctx, s.TestAccs[0], 1, sdk.NewCoin("akash", sdk.NewInt(100)), "Atom", sdk.NewInt(1)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "swap route exact amount in", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executeSwap: func() { + + route := []poolmanagertypes.SwapAmountInRoute{{PoolId: 1, TokenOutDenom: "Atom"}} + + _, err := s.App.PoolManagerKeeper.RouteExactAmountIn(s.Ctx, s.TestAccs[0], route, sdk.NewCoin("akash", sdk.NewInt(100)), sdk.NewInt(1)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "swap route exact amount out", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executeSwap: func() { + + route := []poolmanagertypes.SwapAmountOutRoute{{PoolId: 1, TokenInDenom: "akash"}} + + _, err := s.App.PoolManagerKeeper.RouteExactAmountOut(s.Ctx, s.TestAccs[0], route, sdk.NewInt(10000), sdk.NewCoin("Atom", sdk.NewInt(100))) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "swap route exact amount in - 2 routes", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + { + Pool: 1, + TokenIn: "Atom", + TokenOut: "akash", + }, + }, + executeSwap: func() { + + route := []poolmanagertypes.SwapAmountInRoute{{PoolId: 1, TokenOutDenom: "Atom"}, {PoolId: 1, TokenOutDenom: "akash"}} + + _, err := s.App.PoolManagerKeeper.RouteExactAmountIn(s.Ctx, s.TestAccs[0], route, sdk.NewCoin("akash", sdk.NewInt(100)), sdk.NewInt(1)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "swap route exact amount in - Concentrated Liquidity", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 49, + TokenIn: "uosmo", + TokenOut: "epochTwo", + }, + }, + executeSwap: func() { + + route := []poolmanagertypes.SwapAmountInRoute{{PoolId: 49, TokenOutDenom: "epochTwo"}} + + _, err := s.App.PoolManagerKeeper.RouteExactAmountIn(s.Ctx, s.TestAccs[0], route, sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewInt(1)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + tc.param.executeSwap() + + routes, err := s.App.ProtoRevKeeper.GetSwapsToBackrun(s.Ctx) + s.Require().NoError(err) + s.Require().Equal(tc.param.expectedTrades, routes.Trades) + + s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) + }) + } +} diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 6446af951ba..312817d1220 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -888,7 +888,8 @@ func (s *KeeperTestSuite) setUpPools() { } // Create a concentrated liquidity pool for epoch_hook testing - clPoolOne := s.PrepareConcentratedPoolWithCoins("epochTwo", "uosmo") + // Pool 49 + clPoolOne := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("epochTwo", "uosmo") // Provide liquidity to the concentrated liquidity pool clPoolOneLiquidity := sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(1000)), sdk.NewCoin("uosmo", sdk.NewInt(2000))) From fa03d8ea9237d548a218d969b8480b9634d25a51 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Fri, 19 May 2023 02:23:15 -0400 Subject: [PATCH 16/67] 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) --- x/protorev/keeper/hooks.go | 14 ++++- x/protorev/keeper/hooks_test.go | 107 +++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 3 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index c2e22206a3b..4b59f3b81e3 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -29,13 +29,23 @@ func (h Hooks) AfterCFMMPoolCreated(ctx sdk.Context, sender sdk.AccAddress, pool // 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) { - // TODO: Probably generalize to allow for more join denoms + if len(enterCoins) != 1 { + return + } + h.k.storeJoinExitPoolMsgs(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) { - // TODO: Probably generalize to allow for more exit denoms + // 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.storeJoinExitPoolMsgs(ctx, sender, poolId, exitCoins[0].Denom, false) } diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 1524508588b..3acda2ea192 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -7,7 +7,7 @@ import ( "github.com/osmosis-labs/osmosis/v15/x/protorev/types" ) -func (s *KeeperTestSuite) TestSwaps() { +func (s *KeeperTestSuite) TestSwapping() { type param struct { expectedTrades []types.Trade executeSwap func() @@ -134,3 +134,108 @@ func (s *KeeperTestSuite) TestSwaps() { }) } } + +func (s *KeeperTestSuite) TestLiquidityProviding() { + type param struct { + expectedTrades []types.Trade + executeLiquidityProviding func() + } + + tests := []struct { + name string + param param + expectPass bool + }{ + { + name: "GAMM - Join Swap Exact Amount In", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executeLiquidityProviding: func() { + _, err := s.App.GAMMKeeper.JoinSwapExactAmountIn(s.Ctx, s.TestAccs[0], 1, sdk.NewCoins(sdk.NewCoin("akash", sdk.NewInt(100))), sdk.NewInt(1)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "GAMM - Join Swap Share Amount Out", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executeLiquidityProviding: func() { + _, err := s.App.GAMMKeeper.JoinSwapShareAmountOut(s.Ctx, s.TestAccs[0], 1, "akash", sdk.NewInt(1000), sdk.NewInt(10000)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "GAMM - Exit Swap Exact Amount Out", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executeLiquidityProviding: func() { + _, err := s.App.GAMMKeeper.ExitSwapExactAmountOut(s.Ctx, s.TestAccs[0], 1, sdk.NewCoin("Atom", sdk.NewInt(1)), sdk.NewInt(1002141106353159235)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "GAMM - Exit Swap Share Amount In", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executeLiquidityProviding: func() { + _, err := s.App.GAMMKeeper.ExitSwapShareAmountIn(s.Ctx, s.TestAccs[0], 1, "Atom", sdk.NewInt(1000000000000000000), sdk.NewInt(1)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + { + name: "GAMM - Exit Swap Share Amount In - Low Shares", + param: param{ + expectedTrades: []types.Trade(nil), + executeLiquidityProviding: func() { + _, err := s.App.GAMMKeeper.ExitSwapShareAmountIn(s.Ctx, s.TestAccs[0], 1, "Atom", sdk.NewInt(1000), sdk.NewInt(0)) + s.Require().NoError(err) + }, + }, + expectPass: true, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + tc.param.executeLiquidityProviding() + + routes, err := s.App.ProtoRevKeeper.GetSwapsToBackrun(s.Ctx) + s.Require().NoError(err) + s.Require().Equal(tc.param.expectedTrades, routes.Trades) + + s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) + }) + } +} From cc5f2883984c658b1337b23251b52c748c591718 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Fri, 19 May 2023 02:52:41 -0400 Subject: [PATCH 17/67] Add pool creation hook test --- x/protorev/keeper/hooks_test.go | 55 +++++++++++++++++++++++++++++++- x/protorev/keeper/keeper_test.go | 1 + 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 3acda2ea192..111532dfa42 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -3,6 +3,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/osmosis-labs/osmosis/v15/x/gamm/pool-models/balancer" poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" "github.com/osmosis-labs/osmosis/v15/x/protorev/types" ) @@ -135,7 +136,7 @@ func (s *KeeperTestSuite) TestSwapping() { } } -func (s *KeeperTestSuite) TestLiquidityProviding() { +func (s *KeeperTestSuite) TestLiquidityChanging() { type param struct { expectedTrades []types.Trade executeLiquidityProviding func() @@ -239,3 +240,55 @@ func (s *KeeperTestSuite) TestLiquidityProviding() { }) } } + +func (s *KeeperTestSuite) TestPoolCreation() { + type param struct { + expectedTrades []types.Trade + executePoolCreation func() uint64 + } + + tests := []struct { + name string + param param + expectPass bool + }{ + { + name: "GAMM - Create Pool", + param: param{ + expectedTrades: []types.Trade{ + { + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + executePoolCreation: func() uint64 { + poolId := s.createGAMMPool([]balancer.PoolAsset{ + { + Token: sdk.NewCoin("hook", sdk.NewInt(1000000000)), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(1000000000)), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(2, 3), + sdk.NewDecWithPrec(0, 2)) + + return poolId + }, + }, + expectPass: true, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + poolId := tc.param.executePoolCreation() + setPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, types.OsmosisDenomination, "hook") + s.Require().NoError(err) + s.Require().Equal(poolId, setPoolId) + }) + } +} diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 312817d1220..8feeee8c58e 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -135,6 +135,7 @@ func (s *KeeperTestSuite) SetupTest() { sdk.NewCoin("usdy", sdk.NewInt(9000000000000000000)), sdk.NewCoin("epochOne", sdk.NewInt(9000000000000000000)), sdk.NewCoin("epochTwo", sdk.NewInt(9000000000000000000)), + sdk.NewCoin("hook", sdk.NewInt(9000000000000000000)), ) s.fundAllAccountsWith() s.Commit() From 8938ba5cb33b6b4dfe82f5ca8ae0f0838f67cc7c Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 12:32:26 -0400 Subject: [PATCH 18/67] Add pool creation tests, handle CL pools differently due to 0 liquidity creation --- x/protorev/keeper/hooks.go | 30 ++++++++++++++++++++++++++---- x/protorev/keeper/hooks_test.go | 25 +++++++++++++++---------- x/protorev/keeper/keeper_test.go | 3 ++- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 4b59f3b81e3..b20d3792816 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -3,7 +3,9 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" + cltypes "github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/types" gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" + poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" "github.com/osmosis-labs/osmosis/v15/x/protorev/types" ) @@ -104,10 +106,30 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { return } - coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) - if err != nil { - ctx.Logger().Error("Protorev error getting total pool liquidity in after swap hook", err) - return + // Handle concentrated pools differently since they can have 0 liquidity + coins := sdk.Coins{} + if pool.GetType() == poolmanagertypes.Concentrated { + clPool, ok := pool.(cltypes.ConcentratedPoolExtension) + if !ok { + ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") + return + } + + coins, err = k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + if err != nil { + ctx.Logger().Error("Protorev error getting total pool liquidity in after swap hook", err) + return + } + + if coins == nil { + coins = sdk.Coins{sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))} + } + } else { + coins, err = k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + if err != nil { + ctx.Logger().Error("Protorev error getting total pool liquidity in after pool created hook", err) + return + } } // Pool must be active and the number of coins must be 2 diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 111532dfa42..281d282fb37 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -243,7 +243,7 @@ func (s *KeeperTestSuite) TestLiquidityChanging() { func (s *KeeperTestSuite) TestPoolCreation() { type param struct { - expectedTrades []types.Trade + matchDenom string executePoolCreation func() uint64 } @@ -255,17 +255,11 @@ func (s *KeeperTestSuite) TestPoolCreation() { { name: "GAMM - Create Pool", param: param{ - expectedTrades: []types.Trade{ - { - Pool: 1, - TokenIn: "akash", - TokenOut: "Atom", - }, - }, + matchDenom: "hookGamm", executePoolCreation: func() uint64 { poolId := s.createGAMMPool([]balancer.PoolAsset{ { - Token: sdk.NewCoin("hook", sdk.NewInt(1000000000)), + Token: sdk.NewCoin("hookGamm", sdk.NewInt(1000000000)), Weight: sdk.NewInt(1), }, { @@ -281,12 +275,23 @@ func (s *KeeperTestSuite) TestPoolCreation() { }, expectPass: true, }, + { + name: "Concentrated Liquidity - Create Pool", + param: param{ + matchDenom: "hookCL", + executePoolCreation: func() uint64 { + clPool := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("hookCL", "uosmo") + return clPool.GetId() + }, + }, + expectPass: true, + }, } for _, tc := range tests { s.Run(tc.name, func() { poolId := tc.param.executePoolCreation() - setPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, types.OsmosisDenomination, "hook") + setPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, types.OsmosisDenomination, tc.param.matchDenom) s.Require().NoError(err) s.Require().Equal(poolId, setPoolId) }) diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 8feeee8c58e..3c261514a84 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -135,7 +135,8 @@ func (s *KeeperTestSuite) SetupTest() { sdk.NewCoin("usdy", sdk.NewInt(9000000000000000000)), sdk.NewCoin("epochOne", sdk.NewInt(9000000000000000000)), sdk.NewCoin("epochTwo", sdk.NewInt(9000000000000000000)), - sdk.NewCoin("hook", sdk.NewInt(9000000000000000000)), + sdk.NewCoin("hookGamm", sdk.NewInt(9000000000000000000)), + sdk.NewCoin("hookCL", sdk.NewInt(9000000000000000000)), ) s.fundAllAccountsWith() s.Commit() From 164615f46c8b656779deab83b002025bc04292c1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 13:02:06 -0400 Subject: [PATCH 19/67] Update ExtractSwappedPools to no longer need tx --- x/protorev/keeper/posthandler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 640440435b0..de2736528fe 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -49,7 +49,7 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu } // Extract all of the pools that were swapped in the tx - swappedPools := protoRevDec.ProtoRevKeeper.ExtractSwappedPools(cacheCtx, tx) + swappedPools := protoRevDec.ProtoRevKeeper.ExtractSwappedPools(cacheCtx) if len(swappedPools) == 0 { return next(ctx, tx, simulate) } @@ -143,7 +143,7 @@ func (k Keeper) ProtoRevTrade(ctx sdk.Context, swappedPools []SwapToBackrun) (er // ExtractSwappedPools checks if there were any swaps made on pools and if so returns a list of all the pools that were // swapped on and metadata about the swap -func (k Keeper) ExtractSwappedPools(ctx sdk.Context, tx sdk.Tx) []SwapToBackrun { +func (k Keeper) ExtractSwappedPools(ctx sdk.Context) []SwapToBackrun { swappedPools := make([]SwapToBackrun, 0) swapsToBackrun, err := k.GetSwapsToBackrun(ctx) From d291e692571b38d9d1baa667547069c121bedb26 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 13:02:34 -0400 Subject: [PATCH 20/67] Update TestExtractSwappedPools for new hook-based logic --- x/protorev/keeper/posthandler_test.go | 228 +------------------------- 1 file changed, 6 insertions(+), 222 deletions(-) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 56cff4f8876..402c8a80aba 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -12,7 +12,6 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" - gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" "github.com/osmosis-labs/osmosis/v15/x/protorev/keeper" "github.com/osmosis-labs/osmosis/v15/x/protorev/types" @@ -570,21 +569,10 @@ func (s *KeeperTestSuite) TestAnteHandle() { func (s *KeeperTestSuite) TestExtractSwappedPools() { type param struct { - msgs []sdk.Msg - txFee sdk.Coins - minGasPrices sdk.DecCoins - gasLimit uint64 - isCheckTx bool - baseDenomGas bool - expectedNumOfPools int expectedSwappedPools []keeper.SwapToBackrun + expectedNumOfPools int } - txBuilder := s.clientCtx.TxConfig.NewTxBuilder() - priv0, _, addr0 := testdata.KeyTestPubAddr() - acc1 := s.App.AccountKeeper.NewAccountWithAddress(s.Ctx, addr0) - s.App.AccountKeeper.SetAccount(s.Ctx, acc1) - tests := []struct { name string params param @@ -593,24 +581,6 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { { name: "Single Swap", params: param{ - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 28, - TokenOutDenom: "ibc/BE1BB42D4BE3C30D50B68D7C41DB4DFCE9678E8EF8C539F6E6A9345048894FCC", - }, - }, - TokenIn: sdk.NewCoin("ibc/D189335C6E4A68B513C10AB227BF1C1D38C746766278BA3EEB4FB14124F1D858", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(10000), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfPools: 1, expectedSwappedPools: []keeper.SwapToBackrun{ { @@ -625,35 +595,6 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { { name: "Two Swaps", params: param{ - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 28, - TokenOutDenom: "ibc/BE1BB42D4BE3C30D50B68D7C41DB4DFCE9678E8EF8C539F6E6A9345048894FCC", - }, - }, - TokenIn: sdk.NewCoin("ibc/D189335C6E4A68B513C10AB227BF1C1D38C746766278BA3EEB4FB14124F1D858", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(10000), - }, - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 22, - TokenOutDenom: "ibc/BE1BB42D4BE3C30D50B68D7C41DB4DFCE9678E8EF8C539F6E6A9345048894FCC", - }, - }, - TokenIn: sdk.NewCoin("uosmo", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(10000), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfPools: 2, expectedSwappedPools: []keeper.SwapToBackrun{ { @@ -673,24 +614,6 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { { name: "Single Swap Amount Out Test", params: param{ - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountOut{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountOutRoute{ - { - PoolId: 28, - TokenInDenom: "ibc/BE1BB42D4BE3C30D50B68D7C41DB4DFCE9678E8EF8C539F6E6A9345048894FCC", - }, - }, - TokenOut: sdk.NewCoin("ibc/D189335C6E4A68B513C10AB227BF1C1D38C746766278BA3EEB4FB14124F1D858", sdk.NewInt(10000)), - TokenInMaxAmount: sdk.NewInt(10000), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfPools: 1, expectedSwappedPools: []keeper.SwapToBackrun{ { @@ -705,32 +628,6 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { { name: "Single Swap with multiple hops (swapOut)", params: param{ - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountOut{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountOutRoute{ - { - PoolId: 28, - TokenInDenom: "atom", - }, - { - PoolId: 30, - TokenInDenom: "weth", - }, - { - PoolId: 35, - TokenInDenom: "bitcoin", - }, - }, - TokenOut: sdk.NewCoin("akash", sdk.NewInt(10000)), - TokenInMaxAmount: sdk.NewInt(10000), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfPools: 3, expectedSwappedPools: []keeper.SwapToBackrun{ { @@ -755,36 +652,6 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { { name: "Single Swap with multiple hops (swapIn)", params: param{ - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 28, - TokenOutDenom: "atom", - }, - { - PoolId: 30, - TokenOutDenom: "weth", - }, - { - PoolId: 35, - TokenOutDenom: "bitcoin", - }, - { - PoolId: 36, - TokenOutDenom: "juno", - }, - }, - TokenIn: sdk.NewCoin("akash", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(1), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfPools: 4, expectedSwappedPools: []keeper.SwapToBackrun{ { @@ -814,32 +681,6 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { { name: "Single Swap with multiple hops (gamm msg swapOut)", params: param{ - msgs: []sdk.Msg{ - &gammtypes.MsgSwapExactAmountOut{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountOutRoute{ - { - PoolId: 28, - TokenInDenom: "atom", - }, - { - PoolId: 30, - TokenInDenom: "weth", - }, - { - PoolId: 35, - TokenInDenom: "bitcoin", - }, - }, - TokenOut: sdk.NewCoin("akash", sdk.NewInt(10000)), - TokenInMaxAmount: sdk.NewInt(10000), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfPools: 3, expectedSwappedPools: []keeper.SwapToBackrun{ { @@ -864,36 +705,6 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { { name: "Single Swap with multiple hops (gamm swapIn)", params: param{ - msgs: []sdk.Msg{ - &gammtypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 28, - TokenOutDenom: "atom", - }, - { - PoolId: 30, - TokenOutDenom: "weth", - }, - { - PoolId: 35, - TokenOutDenom: "bitcoin", - }, - { - PoolId: 36, - TokenOutDenom: "juno", - }, - }, - TokenIn: sdk.NewCoin("akash", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(1), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfPools: 4, expectedSwappedPools: []keeper.SwapToBackrun{ { @@ -924,45 +735,18 @@ func (s *KeeperTestSuite) TestExtractSwappedPools() { for _, tc := range tests { s.Run(tc.name, func() { - s.Ctx = s.Ctx.WithIsCheckTx(tc.params.isCheckTx) - s.Ctx = s.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) - s.Ctx = s.Ctx.WithMinGasPrices(tc.params.minGasPrices) - msgs := tc.params.msgs - privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0} - signerData := authsigning.SignerData{ - ChainID: s.Ctx.ChainID(), - AccountNumber: accNums[0], - Sequence: accSeqs[0], + for _, swap := range tc.params.expectedSwappedPools { + s.App.ProtoRevKeeper.AddSwapsToSwapsToBackrun(s.Ctx, []types.Trade{{Pool: swap.PoolId, TokenIn: swap.TokenInDenom, TokenOut: swap.TokenOutDenom}}) } - gasLimit := tc.params.gasLimit - sigV2, _ := clienttx.SignWithPrivKey( - 1, - signerData, - txBuilder, - privs[0], - s.clientCtx.TxConfig, - accSeqs[0], - ) - err := simapp.FundAccount(s.App.BankKeeper, s.Ctx, addr0, tc.params.txFee) - s.Require().NoError(err) - // Can't use test suite BuildTx because it doesn't allow for multiple msgs - err = txBuilder.SetMsgs(msgs...) - s.Require().NoError(err) - err = txBuilder.SetSignatures(sigV2) - s.Require().NoError(err) - txBuilder.SetMemo("") - txBuilder.SetFeeAmount(tc.params.txFee) - txBuilder.SetGasLimit(gasLimit) - - tx := txBuilder.GetTx() - - swappedPools := keeper.ExtractSwappedPools(tx) + swappedPools := s.App.ProtoRevKeeper.ExtractSwappedPools(s.Ctx) if tc.expectPass { s.Require().Equal(tc.params.expectedNumOfPools, len(swappedPools)) s.Require().Equal(tc.params.expectedSwappedPools, swappedPools) } + + s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) }) } } From 087cc7cb51cd197908683121ca7007ae74a94576 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 13:47:48 -0400 Subject: [PATCH 21/67] Delete swaps without using user gas --- x/protorev/keeper/posthandler.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index de2736528fe..34ee798d001 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -41,7 +41,8 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu // // 50M is chosen as a large enough number to ensure that the posthandler will not run out of gas, // but will eventually terminate in event of an accidental infinite loop with some gas usage. - cacheCtx = cacheCtx.WithGasMeter(sdk.NewGasMeter(sdk.Gas(50_000_000))) + upperGasLimitMeter := sdk.NewGasMeter(sdk.Gas(50_000_000)) + cacheCtx = cacheCtx.WithGasMeter(upperGasLimitMeter) // Check if the protorev posthandler can be executed if err := protoRevDec.ProtoRevKeeper.AnteHandleCheck(cacheCtx); err != nil { @@ -62,9 +63,10 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu ctx.Logger().Error("ProtoRevTrade failed with error", err) } - // Delete swaps to backrun for next transaction - // TODO: Should this be placed before we attempt to execute trades? - protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx) + // Delete swaps to backrun for next transaction without consuming gas + originalGasMeter := ctx.GasMeter() + protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(upperGasLimitMeter)) + ctx = ctx.WithGasMeter(originalGasMeter) return next(ctx, tx, simulate) } From 3e259bf2d5e1737d82e31542899d7ff721cd8482 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 13:48:00 -0400 Subject: [PATCH 22/67] Update TestAnteHandle with new hook-based logic --- x/protorev/keeper/posthandler_test.go | 76 ++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 402c8a80aba..f79e2d37514 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -73,6 +73,7 @@ func BenchmarkFourHopHotRouteArb(b *testing.B) { func (s *KeeperTestSuite) TestAnteHandle() { type param struct { + trades []types.Trade msgs []sdk.Msg txFee sdk.Coins minGasPrices sdk.DecCoins @@ -100,6 +101,7 @@ func (s *KeeperTestSuite) TestAnteHandle() { { name: "Random Msg - Expect Nothing to Happen", params: param{ + trades: []types.Trade{}, msgs: []sdk.Msg{testdata.NewTestMsg(addr0)}, txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), minGasPrices: sdk.NewDecCoins(), @@ -115,6 +117,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { name: "No Arb", params: param{ + trades: []types.Trade{ + { + Pool: 12, + TokenOut: "akash", + TokenIn: "juno", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -142,6 +151,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { name: "Mainnet Arb (Block: 5905150) - Highest Liquidity Pool Build", params: param{ + trades: []types.Trade{ + { + Pool: 23, + TokenOut: "ibc/BE1BB42D4BE3C30D50B68D7C41DB4DFCE9678E8EF8C539F6E6A9345048894FCC", + TokenIn: "ibc/0EF15DF2F02480ADE0BB6E85D9EBB5DAEA2836D3860E9F97F9AADE4F57A31AA0", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -174,6 +190,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { name: "Mainnet Arb Route - Multi Asset, Same Weights (Block: 6906570) - Hot Route Build - Atom Arb", params: param{ + trades: []types.Trade{ + { + Pool: 33, + TokenOut: "ibc/A0CC0CF735BFB30E730C70019D4218A1244FF383503FF7579C9201AB93CA9293", + TokenIn: "Atom", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -210,6 +233,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { name: "Stableswap Test Arb Route - Hot Route Build", params: param{ + trades: []types.Trade{ + { + Pool: 29, + TokenOut: types.OsmosisDenomination, + TokenIn: "usdc", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -246,6 +276,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { name: "Four Pool Arb Route - Hot Route Build", params: param{ + trades: []types.Trade{ + { + Pool: 37, + TokenOut: "test/2", + TokenIn: "Atom", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -282,6 +319,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { name: "Two Pool Arb Route - Hot Route Build", params: param{ + trades: []types.Trade{ + { + Pool: 38, + TokenOut: "test/3", + TokenIn: types.OsmosisDenomination, + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -322,6 +366,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { // This test the tx pool points limit caps the number of iterations name: "Doomsday Test - Stableswap - Tx Pool Points Limit", params: param{ + trades: []types.Trade{ + { + Pool: 41, + TokenOut: "usdc", + TokenIn: "busd", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -362,6 +413,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { // This test the block pool points limit caps the number of iterations within a tx name: "Doomsday Test - Stableswap - Block Pool Points Limit - Within a tx", params: param{ + trades: []types.Trade{ + { + Pool: 41, + TokenOut: "usdc", + TokenIn: "busd", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -402,6 +460,13 @@ func (s *KeeperTestSuite) TestAnteHandle() { { // This test the block pool points limit caps the number of txs processed if already reached the limit name: "Doomsday Test - Stableswap - Block Pool Points Limit Already Reached - New tx", params: param{ + trades: []types.Trade{ + { + Pool: 41, + TokenOut: "usdc", + TokenIn: "busd", + }, + }, msgs: []sdk.Msg{ &poolmanagertypes.MsgSwapExactAmountIn{ Sender: addr0.String(), @@ -488,8 +553,11 @@ func (s *KeeperTestSuite) TestAnteHandle() { } if strings.Contains(tc.name, "Doomsday") { + singleMsg := tc.params.msgs[0] + singleTrade := tc.params.trades[0] for i := 0; i < 100; i++ { - msgs = append(msgs, tc.params.msgs...) + msgs = append(msgs, singleMsg) + tc.params.trades = append(tc.params.trades, singleTrade) } err := txBuilder.SetMsgs(msgs...) @@ -512,6 +580,10 @@ func (s *KeeperTestSuite) TestAnteHandle() { s.Ctx = s.Ctx.WithGasMeter(sdk.NewGasMeter(tc.params.gasLimit)) halfGas := tc.params.gasLimit / 2 s.Ctx.GasMeter().ConsumeGas(halfGas, "consume half gas") + + // Set pools to backrun + s.App.AppKeepers.ProtoRevKeeper.AddSwapsToSwapsToBackrun(s.Ctx, tc.params.trades) + gasBefore := s.Ctx.GasMeter().GasConsumed() gasLimitBefore := s.Ctx.GasMeter().Limit() @@ -555,6 +627,8 @@ func (s *KeeperTestSuite) TestAnteHandle() { s.Require().Error(err) } + s.App.AppKeepers.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) + // Reset the max points per tx and block if strings.Contains(tc.name, "Tx Pool Points Limit") { err = s.App.ProtoRevKeeper.SetMaxPointsPerTx(s.Ctx, 18) From 9d28bff97431c8aca3e7c38c5a3148c3aa8705e1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 14:38:04 -0400 Subject: [PATCH 23/67] 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 --- x/protorev/keeper/epoch_hook_test.go | 2 +- x/protorev/keeper/keeper_test.go | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/x/protorev/keeper/epoch_hook_test.go b/x/protorev/keeper/epoch_hook_test.go index 1c4bc71307c..99d42480e2e 100644 --- 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/keeper_test.go b/x/protorev/keeper/keeper_test.go index 3c261514a84..c2703c48d8f 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -891,15 +891,10 @@ func (s *KeeperTestSuite) setUpPools() { // Create a concentrated liquidity pool for epoch_hook testing // Pool 49 - clPoolOne := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("epochTwo", "uosmo") - - // Provide liquidity to the concentrated liquidity pool - clPoolOneLiquidity := sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(1000)), sdk.NewCoin("uosmo", sdk.NewInt(2000))) - err := s.App.BankKeeper.SendCoins(s.Ctx, s.TestAccs[0], clPoolOne.GetAddress(), clPoolOneLiquidity) - s.Require().NoError(err) + s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("epochTwo", "uosmo") // Set all of the pool info into the stores - err = s.App.ProtoRevKeeper.UpdatePools(s.Ctx) + err := s.App.ProtoRevKeeper.UpdatePools(s.Ctx) s.Require().NoError(err) } From 840416eaabd83fcf3a0d6f860e7a28466041ab47 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 14:42:12 -0400 Subject: [PATCH 24/67] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e605d96a7e..7bccb8cc791 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,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 ### Misc Improvements From 6c1944fa0a2418033be75f048293e571443baa68 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 14:59:18 -0400 Subject: [PATCH 25/67] Remove unused messages in TestAnteHandle Since we now have hook-based logic, can remove msgs from inputs into TestAnteHandle() --- x/protorev/keeper/posthandler_test.go | 199 ++------------------------ 1 file changed, 13 insertions(+), 186 deletions(-) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index f79e2d37514..cb50db63662 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -74,12 +74,6 @@ func BenchmarkFourHopHotRouteArb(b *testing.B) { func (s *KeeperTestSuite) TestAnteHandle() { type param struct { trades []types.Trade - msgs []sdk.Msg - txFee sdk.Coins - minGasPrices sdk.DecCoins - gasLimit uint64 - isCheckTx bool - baseDenomGas bool expectedNumOfTrades sdk.Int expectedProfits []sdk.Coin expectedPoolPoints uint64 @@ -102,12 +96,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { name: "Random Msg - Expect Nothing to Happen", params: param{ trades: []types.Trade{}, - msgs: []sdk.Msg{testdata.NewTestMsg(addr0)}, - txFee: sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.ZeroInt(), expectedProfits: []sdk.Coin{}, expectedPoolPoints: 0, @@ -124,24 +112,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "juno", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 12, - TokenOutDenom: "akash", - }, - }, - TokenIn: sdk.NewCoin("juno", sdk.NewInt(10)), - TokenOutMinAmount: sdk.NewInt(1), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.ZeroInt(), expectedProfits: []sdk.Coin{}, expectedPoolPoints: 0, @@ -158,24 +128,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "ibc/0EF15DF2F02480ADE0BB6E85D9EBB5DAEA2836D3860E9F97F9AADE4F57A31AA0", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 23, - TokenOutDenom: "ibc/BE1BB42D4BE3C30D50B68D7C41DB4DFCE9678E8EF8C539F6E6A9345048894FCC", - }, - }, - TokenIn: sdk.NewCoin("ibc/0EF15DF2F02480ADE0BB6E85D9EBB5DAEA2836D3860E9F97F9AADE4F57A31AA0", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(10000), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.OneInt(), expectedProfits: []sdk.Coin{ { @@ -197,24 +149,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "Atom", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 33, - TokenOutDenom: "ibc/A0CC0CF735BFB30E730C70019D4218A1244FF383503FF7579C9201AB93CA9293", - }, - }, - TokenIn: sdk.NewCoin("Atom", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(10000), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.NewInt(2), expectedProfits: []sdk.Coin{ { @@ -240,24 +174,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "usdc", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 29, - TokenOutDenom: types.OsmosisDenomination, - }, - }, - TokenIn: sdk.NewCoin("usdc", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(100), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.NewInt(3), expectedProfits: []sdk.Coin{ { @@ -283,24 +199,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "Atom", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 37, - TokenOutDenom: "test/2", - }, - }, - TokenIn: sdk.NewCoin("Atom", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(100), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.NewInt(4), expectedProfits: []sdk.Coin{ { @@ -326,24 +224,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: types.OsmosisDenomination, }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 38, - TokenOutDenom: "test/3", - }, - }, - TokenIn: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(100), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.NewInt(5), expectedProfits: []sdk.Coin{ { @@ -373,24 +253,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "busd", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 41, - TokenOutDenom: "usdc", - }, - }, - TokenIn: sdk.NewCoin("busd", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(100), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.NewInt(5), expectedProfits: []sdk.Coin{ { @@ -420,24 +282,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "busd", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 41, - TokenOutDenom: "usdc", - }, - }, - TokenIn: sdk.NewCoin("busd", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(100), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.NewInt(5), expectedProfits: []sdk.Coin{ { @@ -467,24 +311,6 @@ func (s *KeeperTestSuite) TestAnteHandle() { TokenIn: "busd", }, }, - msgs: []sdk.Msg{ - &poolmanagertypes.MsgSwapExactAmountIn{ - Sender: addr0.String(), - Routes: []poolmanagertypes.SwapAmountInRoute{ - { - PoolId: 41, - TokenOutDenom: "usdc", - }, - }, - TokenIn: sdk.NewCoin("busd", sdk.NewInt(10000)), - TokenOutMinAmount: sdk.NewInt(100), - }, - }, - txFee: sdk.NewCoins(sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000))), - minGasPrices: sdk.NewDecCoins(), - gasLimit: 500000, - isCheckTx: false, - baseDenomGas: true, expectedNumOfTrades: sdk.NewInt(5), expectedProfits: []sdk.Coin{ { @@ -515,9 +341,12 @@ func (s *KeeperTestSuite) TestAnteHandle() { for _, tc := range tests { s.Run(tc.name, func() { - s.Ctx = s.Ctx.WithIsCheckTx(tc.params.isCheckTx) + s.Ctx = s.Ctx.WithIsCheckTx(false) s.Ctx = s.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) - s.Ctx = s.Ctx.WithMinGasPrices(tc.params.minGasPrices) + s.Ctx = s.Ctx.WithMinGasPrices(sdk.NewDecCoins()) + + gasLimit := uint64(500000) + txFee := sdk.NewCoins(sdk.NewCoin("uosmo", sdk.NewInt(10000))) privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0}, []uint64{0}, []uint64{0} signerData := authsigning.SignerData{ @@ -525,7 +354,7 @@ func (s *KeeperTestSuite) TestAnteHandle() { AccountNumber: accNums[0], Sequence: accSeqs[0], } - gasLimit := tc.params.gasLimit + sigV2, _ := clienttx.SignWithPrivKey( 1, signerData, @@ -534,11 +363,12 @@ func (s *KeeperTestSuite) TestAnteHandle() { s.clientCtx.TxConfig, accSeqs[0], ) - err := simapp.FundAccount(s.App.BankKeeper, s.Ctx, addr0, tc.params.txFee) + + err := simapp.FundAccount(s.App.BankKeeper, s.Ctx, addr0, txFee) s.Require().NoError(err) var tx authsigning.Tx - var msgs []sdk.Msg + msgs := []sdk.Msg{testdata.NewTestMsg(addr0)} // Lower the max points per tx and block if the test cases are doomsday testing if strings.Contains(tc.name, "Tx Pool Points Limit") { @@ -553,10 +383,8 @@ func (s *KeeperTestSuite) TestAnteHandle() { } if strings.Contains(tc.name, "Doomsday") { - singleMsg := tc.params.msgs[0] singleTrade := tc.params.trades[0] for i := 0; i < 100; i++ { - msgs = append(msgs, singleMsg) tc.params.trades = append(tc.params.trades, singleTrade) } @@ -565,20 +393,19 @@ func (s *KeeperTestSuite) TestAnteHandle() { err = txBuilder.SetSignatures(sigV2) s.Require().NoError(err) txBuilder.SetMemo("") - txBuilder.SetFeeAmount(tc.params.txFee) + txBuilder.SetFeeAmount(txFee) txBuilder.SetGasLimit(gasLimit) tx = txBuilder.GetTx() } else { - msgs = tc.params.msgs - tx = s.BuildTx(txBuilder, msgs, sigV2, "", tc.params.txFee, gasLimit) + tx = s.BuildTx(txBuilder, msgs, sigV2, "", txFee, gasLimit) } protoRevDecorator := keeper.NewProtoRevDecorator(*s.App.ProtoRevKeeper) posthandlerProtoRev := sdk.ChainAnteDecorators(protoRevDecorator) // Added so we can check the gas consumed during the posthandler - s.Ctx = s.Ctx.WithGasMeter(sdk.NewGasMeter(tc.params.gasLimit)) - halfGas := tc.params.gasLimit / 2 + s.Ctx = s.Ctx.WithGasMeter(sdk.NewGasMeter(gasLimit)) + halfGas := gasLimit / 2 s.Ctx.GasMeter().ConsumeGas(halfGas, "consume half gas") // Set pools to backrun From 225f1b9b92b8c325afb9509c1be40ccecf79d64d Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 15:06:17 -0400 Subject: [PATCH 26/67] Remove no longer used functions --- x/protorev/keeper/posthandler.go | 39 -------------------------------- 1 file changed, 39 deletions(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index 34ee798d001..a4b294825f8 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -4,8 +4,6 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" - - poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" ) type SwapToBackrun struct { @@ -163,40 +161,3 @@ func (k Keeper) ExtractSwappedPools(ctx sdk.Context) []SwapToBackrun { return swappedPools } - -// extractSwapInPools extracts the pools that were swapped on for a MsgSwapExactAmountIn -func extractSwapInPools(routes []poolmanagertypes.SwapAmountInRoute, tokenInDenom string) []SwapToBackrun { - swappedPools := make([]SwapToBackrun, 0) - - prevTokenIn := tokenInDenom - for _, route := range routes { - swappedPools = append(swappedPools, SwapToBackrun{ - PoolId: route.PoolId, - TokenOutDenom: route.TokenOutDenom, - TokenInDenom: prevTokenIn, - }) - - prevTokenIn = route.TokenOutDenom - } - - return swappedPools -} - -// extractSwapOutPools extracts the pools that were swapped on for a MsgSwapExactAmountOut -func extractSwapOutPools(routes []poolmanagertypes.SwapAmountOutRoute, tokenOutDenom string) []SwapToBackrun { - swappedPools := make([]SwapToBackrun, 0) - - prevTokenOut := tokenOutDenom - for i := len(routes) - 1; i >= 0; i-- { - route := routes[i] - swappedPools = append(swappedPools, SwapToBackrun{ - PoolId: route.PoolId, - TokenOutDenom: prevTokenOut, - TokenInDenom: route.TokenInDenom, - }) - - prevTokenOut = route.TokenInDenom - } - - return swappedPools -} From cb4c85be9719ef4ef69862aa6c800ab151f867e2 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 15:13:25 -0400 Subject: [PATCH 27/67] Check len on all hooks to avoid unintended behavior --- x/protorev/keeper/hooks.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index b20d3792816..cd31199a11b 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -31,6 +31,7 @@ func (h Hooks) AfterCFMMPoolCreated(ctx sdk.Context, sender sdk.AccAddress, pool // 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 } @@ -53,8 +54,11 @@ func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint // 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) { - // As of v15, it is safe to assume only one input token and one output token - // TODO: Probably generalize to allow for more swap denoms + // 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) } @@ -77,8 +81,11 @@ func (h Hooks) AfterLastPoolPositionRemoved(ctx sdk.Context, sender sdk.AccAddre // 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) { - // As of v15, it is safe to assume only one input token and one output token - // TODO: Probably generalize to allow for more swap denoms + // 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) } From 12b5c7dbecad92c7e02b9a185a36424bf3b4a41d Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 22 May 2023 15:16:38 -0400 Subject: [PATCH 28/67] lint --- x/protorev/keeper/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index cd31199a11b..d2ffc3ff38d 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -114,7 +114,7 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { } // Handle concentrated pools differently since they can have 0 liquidity - coins := sdk.Coins{} + var coins sdk.Coins if pool.GetType() == poolmanagertypes.Concentrated { clPool, ok := pool.(cltypes.ConcentratedPoolExtension) if !ok { From 65356d91cef42c39bf169e51363d1eceef196623 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 23 May 2023 10:27:58 -0400 Subject: [PATCH 29/67] Move all mul logic into same fn in hook logic afterpoolCreated, add panic catching to function --- x/protorev/keeper/hooks.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index d2ffc3ff38d..01a2feeada6 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -144,13 +144,11 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { tokenA := coins[0] tokenB := coins[1] - liquidity := tokenA.Amount.Mul(tokenB.Amount) - if baseDenomMap[tokenA.Denom] { - k.compareAndStorePool(ctx, poolId, liquidity, tokenA.Denom, tokenB.Denom) + k.compareAndStorePool(ctx, poolId, tokenA, tokenB) } if baseDenomMap[tokenB.Denom] { - k.compareAndStorePool(ctx, poolId, liquidity, tokenB.Denom, tokenA.Denom) + k.compareAndStorePool(ctx, poolId, tokenB, tokenA) } } } @@ -169,14 +167,22 @@ func (k Keeper) storeSwap(ctx sdk.Context, poolId uint64, tokenIn, tokenOut stri } // 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, liquidity sdk.Int, baseDenom, otherDenom string) { - storedPoolId, err := k.GetPoolForDenomPair(ctx, baseDenom, otherDenom) +func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseToken, otherToken sdk.Coin) { + storedPoolId, err := k.GetPoolForDenomPair(ctx, baseToken.Denom, otherToken.Denom) if err != nil { // Error means no pool exists for this pair, so we set it - k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) + k.SetPoolForDenomPair(ctx, baseToken.Denom, otherToken.Denom, poolId) return } + defer func() { + if r := recover(); r != nil { + ctx.Logger().Error("Protorev error recovering from panic in AfterCFMMPoolCreated hook, likely an overflow error", r) + } + }() + + liquidity := baseToken.Amount.Mul(otherToken.Amount) + storedPool, err := k.gammKeeper.GetPoolAndPoke(ctx, storedPoolId) if err != nil { ctx.Logger().Error("Protorev error getting storedPool in AfterCFMMPoolCreated hook", err) @@ -188,7 +194,7 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, liquidity sd // If the new pool has more liquidity, we set it if liquidity.GT(storedPoolLiquidity) { - k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) + k.SetPoolForDenomPair(ctx, baseToken.Denom, otherToken.Denom, poolId) } } From 523c711b8db978e6081d6013e3cd698b001d29ca Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 23 May 2023 10:35:24 -0400 Subject: [PATCH 30/67] Separate getCoins logic into separate function --- x/protorev/keeper/hooks.go | 42 ++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 01a2feeada6..eaa055ea863 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -113,44 +113,50 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { return } - // Handle concentrated pools differently since they can have 0 liquidity + coins := k.getCoinsFromPool(ctx, pool, poolId) + + // Pool must be active and the number of coins must be 2 + if pool.IsActive(ctx) && len(coins) == 2 { + tokenA := coins[0] + tokenB := coins[1] + + if baseDenomMap[tokenA.Denom] { + k.compareAndStorePool(ctx, poolId, tokenA, tokenB) + } + if baseDenomMap[tokenB.Denom] { + k.compareAndStorePool(ctx, poolId, tokenB, tokenA) + } + } +} + +// getCoinsFromPool gets the coins from the pool, handling concentrated pools differently since they can have 0 liquidity. +func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, poolId uint64) sdk.Coins { var coins sdk.Coins if pool.GetType() == poolmanagertypes.Concentrated { clPool, ok := pool.(cltypes.ConcentratedPoolExtension) if !ok { ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") - return + return coins } - coins, err = k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) if err != nil { ctx.Logger().Error("Protorev error getting total pool liquidity in after swap hook", err) - return + return coins } if coins == nil { coins = sdk.Coins{sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))} } } else { - coins, err = k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) if err != nil { ctx.Logger().Error("Protorev error getting total pool liquidity in after pool created hook", err) - return + return coins } } - // Pool must be active and the number of coins must be 2 - if pool.IsActive(ctx) && len(coins) == 2 { - tokenA := coins[0] - tokenB := coins[1] - - if baseDenomMap[tokenA.Denom] { - k.compareAndStorePool(ctx, poolId, tokenA, tokenB) - } - if baseDenomMap[tokenB.Denom] { - k.compareAndStorePool(ctx, poolId, tokenB, tokenA) - } - } + return coins } // storeSwap stores a swap to be checked by protorev when attempting backruns. From 031a52bef54d87240d55b2536d0d2ad8c75fadf1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 23 May 2023 10:52:26 -0400 Subject: [PATCH 31/67] fix lint --- x/protorev/keeper/hooks.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index eaa055ea863..04d9a0bdf3f 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -131,29 +131,21 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { // getCoinsFromPool gets the coins from the pool, handling concentrated pools differently since they can have 0 liquidity. func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, poolId uint64) sdk.Coins { - var coins sdk.Coins - if pool.GetType() == poolmanagertypes.Concentrated { - clPool, ok := pool.(cltypes.ConcentratedPoolExtension) - if !ok { - ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") - return coins - } - - coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) - if err != nil { - ctx.Logger().Error("Protorev error getting total pool liquidity in after swap hook", err) - return coins - } + coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + if err != nil { + ctx.Logger().Error("Protorev error getting pool liquidity in AfterCFMMPoolCreated hook", err) + return nil + } - if coins == nil { + if coins == nil { + if pool.GetType() == poolmanagertypes.Concentrated { + clPool, ok := pool.(cltypes.ConcentratedPoolExtension) + if !ok { + ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") + return nil + } coins = sdk.Coins{sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))} } - } else { - coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) - if err != nil { - ctx.Logger().Error("Protorev error getting total pool liquidity in after pool created hook", err) - return coins - } } return coins From e2729f46d4e88d3c11cf75d735b84dab6f976155 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 23 May 2023 10:54:44 -0400 Subject: [PATCH 32/67] return sdk.Coins{} instead of nil --- x/protorev/keeper/hooks.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 04d9a0bdf3f..756a91a0102 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -134,7 +134,7 @@ func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, p coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) if err != nil { ctx.Logger().Error("Protorev error getting pool liquidity in AfterCFMMPoolCreated hook", err) - return nil + return sdk.Coins{} } if coins == nil { @@ -142,9 +142,12 @@ func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, p clPool, ok := pool.(cltypes.ConcentratedPoolExtension) if !ok { ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") - return nil + return sdk.Coins{} } coins = sdk.Coins{sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))} + } else { + ctx.Logger().Error("Protorev error getting pool liquidity in AfterCFMMPoolCreated hook") + return sdk.Coins{} } } From 92933f1e9ef0cf5dbfec3b73c20f8a0a44d8c506 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Fri, 26 May 2023 02:14:29 -0400 Subject: [PATCH 33/67] remove unused import --- x/protorev/keeper/posthandler_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 90226d9b704..84531d320b7 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -13,7 +13,6 @@ import ( authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" "github.com/osmosis-labs/osmosis/v15/app/apptesting" - gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" "github.com/osmosis-labs/osmosis/v15/x/protorev/keeper" "github.com/osmosis-labs/osmosis/v15/x/protorev/types" From f88995ca587187fc948387c5c2b7b8840f511019 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Fri, 26 May 2023 10:38:34 -0400 Subject: [PATCH 34/67] Remove unnecessary code --- x/protorev/keeper/posthandler.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index a4b294825f8..e81f0c9e322 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -62,9 +62,7 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu } // Delete swaps to backrun for next transaction without consuming gas - originalGasMeter := ctx.GasMeter() protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(upperGasLimitMeter)) - ctx = ctx.WithGasMeter(originalGasMeter) return next(ctx, tx, simulate) } From 4c977125894d28f60c8dfc13f9539ad1a95dfdf5 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 30 May 2023 09:50:53 -0400 Subject: [PATCH 35/67] clarify comment Co-authored-by: Sishir Giri --- x/protorev/keeper/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 756a91a0102..21140f26757 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -129,7 +129,7 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { } } -// getCoinsFromPool gets the coins from the pool, handling concentrated pools differently since they can have 0 liquidity. +// getCoinsFromPool gets the coins from the pool, handling Concentrated Liquidity pools differently since they can have 0 liquidity. func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, poolId uint64) sdk.Coins { coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) if err != nil { From 6c744ac96782041fdcc08f4993c53494d8a5fa38 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 30 May 2023 09:56:03 -0400 Subject: [PATCH 36/67] use NewCoins instead of Coins Co-authored-by: Sishir Giri --- x/protorev/keeper/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 21140f26757..da16ec7225b 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -144,7 +144,7 @@ func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, p ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") return sdk.Coins{} } - coins = sdk.Coins{sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))} + coins = sdk.NewCoins(sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))) } else { ctx.Logger().Error("Protorev error getting pool liquidity in AfterCFMMPoolCreated hook") return sdk.Coins{} From 6faeab31fe71d17a5963497542c0532f174db2fb Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 30 May 2023 12:11:25 -0400 Subject: [PATCH 37/67] Revert "use NewCoins instead of Coins" This reverts commit 6c744ac96782041fdcc08f4993c53494d8a5fa38. --- x/protorev/keeper/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index da16ec7225b..21140f26757 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -144,7 +144,7 @@ func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, p ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") return sdk.Coins{} } - coins = sdk.NewCoins(sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))) + coins = sdk.Coins{sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))} } else { ctx.Logger().Error("Protorev error getting pool liquidity in AfterCFMMPoolCreated hook") return sdk.Coins{} From 6d7d411780b88f2da10382376061f67618ee8f1b Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 30 May 2023 17:00:08 -0400 Subject: [PATCH 38/67] add inline comment for why no return in err --- x/protorev/keeper/hooks.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 21140f26757..aad97d404c1 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -163,7 +163,7 @@ func (k Keeper) storeSwap(ctx sdk.Context, poolId uint64, tokenIn, tokenOut stri } if err := k.AddSwapsToSwapsToBackrun(ctx, []types.Trade{swapToBackrun}); err != nil { - ctx.Logger().Error("Protorev error adding swap to backrun from storeSwap", err) + ctx.Logger().Error("Protorev error adding swap to backrun from storeSwap", err) // Does not return since logging is last thing in the function } } From 66c1e8cce646be8440da2e82dd51697f5dfef392 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 1 Jun 2023 16:05:38 -0400 Subject: [PATCH 39/67] use SetupTest instead of custom logic --- x/protorev/keeper/hooks_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 281d282fb37..4c60e316fda 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -125,13 +125,12 @@ func (s *KeeperTestSuite) TestSwapping() { for _, tc := range tests { s.Run(tc.name, func() { + s.SetupTest() tc.param.executeSwap() routes, err := s.App.ProtoRevKeeper.GetSwapsToBackrun(s.Ctx) s.Require().NoError(err) s.Require().Equal(tc.param.expectedTrades, routes.Trades) - - s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) }) } } From 53633195ea027b9b83a3a5344d298b857bfb208b Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 1 Jun 2023 17:31:15 -0400 Subject: [PATCH 40/67] 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 --- x/protorev/keeper/hooks.go | 45 ++++++++------------------------- x/protorev/keeper/hooks_test.go | 23 ++++++++++++++--- 2 files changed, 31 insertions(+), 37 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index aad97d404c1..70e016307e6 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -3,9 +3,7 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" - cltypes "github.com/osmosis-labs/osmosis/v15/x/concentrated-liquidity/types" gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" - poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" "github.com/osmosis-labs/osmosis/v15/x/protorev/types" ) @@ -26,7 +24,7 @@ func (k Keeper) Hooks() Hooks { return Hooks{k} } // 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.afterPoolCreated(ctx, poolId) + h.k.afterPoolCreatedWithCoins(ctx, poolId) } // AfterJoinPool stores swaps to be checked by protorev given the coins entered into the pool. @@ -66,13 +64,13 @@ func (h Hooks) AfterCFMMSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint // CONCENTRATED LIQUIDITY HOOKS // ---------------------------------------------------------------------------- -// AfterConcentratedPoolCreated checks and potentially stores the pool via the highest liquidity method. +// AfterConcentratedPoolCreated is a noop. func (h Hooks) AfterConcentratedPoolCreated(ctx sdk.Context, sender sdk.AccAddress, poolId uint64) { - h.k.afterPoolCreated(ctx, poolId) } -// AfterInitialPoolPositionCreated is a noop. +// 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. @@ -93,9 +91,9 @@ func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, // HELPER METHODS // ---------------------------------------------------------------------------- -// afterPoolCreated checks if the new pool should be stored as the highest liquidity pool +// 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) afterPoolCreated(ctx sdk.Context, poolId uint64) { +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) @@ -113,7 +111,11 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { return } - coins := k.getCoinsFromPool(ctx, pool, poolId) + coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(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 coins must be 2 if pool.IsActive(ctx) && len(coins) == 2 { @@ -129,31 +131,6 @@ func (k Keeper) afterPoolCreated(ctx sdk.Context, poolId uint64) { } } -// getCoinsFromPool gets the coins from the pool, handling Concentrated Liquidity pools differently since they can have 0 liquidity. -func (k Keeper) getCoinsFromPool(ctx sdk.Context, pool poolmanagertypes.PoolI, poolId uint64) sdk.Coins { - coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) - if err != nil { - ctx.Logger().Error("Protorev error getting pool liquidity in AfterCFMMPoolCreated hook", err) - return sdk.Coins{} - } - - if coins == nil { - if pool.GetType() == poolmanagertypes.Concentrated { - clPool, ok := pool.(cltypes.ConcentratedPoolExtension) - if !ok { - ctx.Logger().Error("Protorev error casting pool to concentrated pool in AfterCFMMPoolCreated hook") - return sdk.Coins{} - } - coins = sdk.Coins{sdk.NewCoin(clPool.GetToken0(), sdk.NewInt(0)), sdk.NewCoin(clPool.GetToken1(), sdk.NewInt(0))} - } else { - ctx.Logger().Error("Protorev error getting pool liquidity in AfterCFMMPoolCreated hook") - return sdk.Coins{} - } - } - - return coins -} - // 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{ diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 4c60e316fda..1a683dba60a 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -275,7 +275,18 @@ func (s *KeeperTestSuite) TestPoolCreation() { expectPass: true, }, { - name: "Concentrated Liquidity - Create Pool", + name: "Concentrated Liquidity - Create Pool w/ No Liqudity", + param: param{ + matchDenom: "hookCL", + executePoolCreation: func() uint64 { + clPool := s.PrepareConcentratedPool() + return clPool.GetId() + }, + }, + expectPass: false, + }, + { + name: "Concentrated Liquidity - Create Pool w/ Liqudity", param: param{ matchDenom: "hookCL", executePoolCreation: func() uint64 { @@ -291,8 +302,14 @@ func (s *KeeperTestSuite) TestPoolCreation() { s.Run(tc.name, func() { poolId := tc.param.executePoolCreation() setPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, types.OsmosisDenomination, tc.param.matchDenom) - s.Require().NoError(err) - s.Require().Equal(poolId, setPoolId) + + if tc.expectPass { + s.Require().NoError(err) + s.Require().Equal(poolId, setPoolId) + } else { + s.Require().Error(err) + s.Require().NotEqual(poolId, setPoolId) + } }) } } From e3a9998609d74d77e51696f5a6f37edeae131eb0 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 1 Jun 2023 17:53:40 -0400 Subject: [PATCH 41/67] Use denoms first, then coins in pool updating - Does all logic it can just using denoms first, and only gets coins object if necessary --- x/protorev/keeper/hooks.go | 38 +++++++++++++++------------- x/protorev/types/expected_keepers.go | 1 + 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 70e016307e6..e772fcf9ea0 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -111,22 +111,20 @@ func (k Keeper) afterPoolCreatedWithCoins(ctx sdk.Context, poolId uint64) { return } - coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + //coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + 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 coins must be 2 - if pool.IsActive(ctx) && len(coins) == 2 { - tokenA := coins[0] - tokenB := coins[1] - - if baseDenomMap[tokenA.Denom] { - k.compareAndStorePool(ctx, poolId, tokenA, tokenB) + // Pool must be active and the number of denoms must be 2 + if pool.IsActive(ctx) && len(denoms) == 2 { + if baseDenomMap[denoms[0]] { + k.compareAndStorePool(ctx, poolId, denoms[0], denoms[1]) } - if baseDenomMap[tokenB.Denom] { - k.compareAndStorePool(ctx, poolId, tokenB, tokenA) + if baseDenomMap[denoms[1]] { + k.compareAndStorePool(ctx, poolId, denoms[1], denoms[0]) } } } @@ -145,11 +143,11 @@ func (k Keeper) storeSwap(ctx sdk.Context, poolId uint64, tokenIn, tokenOut stri } // 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, baseToken, otherToken sdk.Coin) { - storedPoolId, err := k.GetPoolForDenomPair(ctx, baseToken.Denom, otherToken.Denom) +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, baseToken.Denom, otherToken.Denom, poolId) + k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) return } @@ -159,20 +157,26 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseToken, o } }() - liquidity := baseToken.Amount.Mul(otherToken.Amount) + // Get coins and calculate liquidity for the new pool + newPoolCoins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + if err != nil { + ctx.Logger().Error("Protorev error getting newPoolCoins in compareAndStorePool", err) + return + } + newPoolliquidity := newPoolCoins[0].Amount.Mul(newPoolCoins[1].Amount) + // Get coins and calculate liquidity for the stored pool storedPool, err := k.gammKeeper.GetPoolAndPoke(ctx, storedPoolId) if err != nil { ctx.Logger().Error("Protorev error getting storedPool in AfterCFMMPoolCreated hook", err) return } - storedPoolCoins := storedPool.GetTotalPoolLiquidity(ctx) storedPoolLiquidity := storedPoolCoins[0].Amount.Mul(storedPoolCoins[1].Amount) // If the new pool has more liquidity, we set it - if liquidity.GT(storedPoolLiquidity) { - k.SetPoolForDenomPair(ctx, baseToken.Denom, otherToken.Denom, poolId) + if newPoolliquidity.GT(storedPoolLiquidity) { + k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) } } diff --git a/x/protorev/types/expected_keepers.go b/x/protorev/types/expected_keepers.go index f1cd028f66a..5c82dd29cae 100644 --- a/x/protorev/types/expected_keepers.go +++ b/x/protorev/types/expected_keepers.go @@ -53,6 +53,7 @@ type PoolManagerKeeper interface { ) (poolmanagertypes.PoolI, error) GetPoolModule(ctx sdk.Context, poolId uint64) (poolmanagertypes.PoolModuleI, error) GetTotalPoolLiquidity(ctx sdk.Context, poolId uint64) (sdk.Coins, error) + RouteGetPoolDenoms(ctx sdk.Context, poolId uint64) ([]string, error) } // EpochKeeper defines the Epoch contract that must be fulfilled when From 319f0fcdc827aa85bb4643101267c5e54505a4a1 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Thu, 1 Jun 2023 17:56:03 -0400 Subject: [PATCH 42/67] remove comment --- x/protorev/keeper/hooks.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index e772fcf9ea0..d8097d3abfe 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -111,7 +111,6 @@ func (k Keeper) afterPoolCreatedWithCoins(ctx sdk.Context, poolId uint64) { return } - //coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) denoms, err := k.poolmanagerKeeper.RouteGetPoolDenoms(ctx, poolId) if err != nil { ctx.Logger().Error("Protorev error getting pool liquidity in afterPoolCreated", err) From 91f16f0e0bc60638d002c21a7a2224e8fdd5bf72 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Fri, 2 Jun 2023 18:47:24 -0400 Subject: [PATCH 43/67] 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 --- x/protorev/keeper/hooks.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index d8097d3abfe..5f88136dfee 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -156,22 +156,19 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o } }() - // Get coins and calculate liquidity for the new pool - newPoolCoins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + // Get comparable liquidity for the new pool + newPoolliquidity, err := k.getComparablePoolLiquidity(ctx, poolId) if err != nil { - ctx.Logger().Error("Protorev error getting newPoolCoins in compareAndStorePool", err) + ctx.Logger().Error("Protorev error getting newPoolLiquidity in compareAndStorePool", err) return } - newPoolliquidity := newPoolCoins[0].Amount.Mul(newPoolCoins[1].Amount) - // Get coins and calculate liquidity for the stored pool - storedPool, err := k.gammKeeper.GetPoolAndPoke(ctx, storedPoolId) + // Get comparable liquidity for the stored pool + storedPoolLiquidity, err := k.getComparablePoolLiquidity(ctx, storedPoolId) if err != nil { - ctx.Logger().Error("Protorev error getting storedPool in AfterCFMMPoolCreated hook", err) + ctx.Logger().Error("Protorev error getting storedPoolLiquidity in compareAndStorePool", err) return } - storedPoolCoins := storedPool.GetTotalPoolLiquidity(ctx) - storedPoolLiquidity := storedPoolCoins[0].Amount.Mul(storedPoolCoins[1].Amount) // If the new pool has more liquidity, we set it if newPoolliquidity.GT(storedPoolLiquidity) { @@ -179,6 +176,16 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o } } +// 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) (sdk.Int, error) { + coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + if err != nil { + return sdk.Int{}, err + } + + return coins[0].Amount.Mul(coins[1].Amount), nil +} + // storeJoinExitPoolMsgs stores the join/exit pool messages in the store, depending on if it is a join or exit. func (k Keeper) storeJoinExitPoolMsgs(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, denom string, isJoin bool) { pool, err := k.gammKeeper.GetPoolAndPoke(ctx, poolId) From c24c3b98f9c987f36a539713bc196d09cc829b52 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 10:02:53 -0400 Subject: [PATCH 44/67] Add CL first, then balancer (and vice-versa) pool creation tests --- x/protorev/keeper/hooks_test.go | 75 ++++++++++++++++++++++++++++++++ x/protorev/keeper/keeper_test.go | 1 + 2 files changed, 76 insertions(+) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 1a683dba60a..4c1d0129d56 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -8,6 +8,7 @@ import ( "github.com/osmosis-labs/osmosis/v15/x/protorev/types" ) +// Tests the hook implementation that is called after swapping func (s *KeeperTestSuite) TestSwapping() { type param struct { expectedTrades []types.Trade @@ -135,6 +136,7 @@ func (s *KeeperTestSuite) TestSwapping() { } } +// Tests the hook implementation that is called after liquidity providing func (s *KeeperTestSuite) TestLiquidityChanging() { type param struct { expectedTrades []types.Trade @@ -240,6 +242,7 @@ func (s *KeeperTestSuite) TestLiquidityChanging() { } } +// Tests the hook implementation that is called after pool creation with coins func (s *KeeperTestSuite) TestPoolCreation() { type param struct { matchDenom string @@ -296,10 +299,82 @@ func (s *KeeperTestSuite) TestPoolCreation() { }, expectPass: true, }, + { + name: "Create Balancer Pool First, Then Concentrated Liquidity w/ Liquidity - CL with more liquidity so should be stored", + param: param{ + matchDenom: "hook", + executePoolCreation: func() uint64 { + // Create balancer pool first with a new denom pair + balancerPoolId := s.createGAMMPool([]balancer.PoolAsset{ + { + Token: sdk.NewCoin("hook", sdk.NewInt(1)), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin("uosmo", sdk.NewInt(1)), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(1, 1), + sdk.NewDecWithPrec(0, 2), + ) + + // Ensure that the balancer pool is stored since no other pool exists for the denom pair + setPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "hook") + s.Require().NoError(err) + s.Require().Equal(balancerPoolId, setPoolId) + + // Create Concentrated Liquidity pool with the same denom pair and more liquidity + // The returned pool id should be what is finally stored in the protorev keeper + clPool := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("hook", "uosmo") + return clPool.GetId() + }, + }, + expectPass: true, + }, + { + name: "Create Concentrated Liquidity Pool w/ Liquidity First, Then Balancer Pool - Balancer with more liquidity so should be stored", + param: param{ + matchDenom: "hook", + executePoolCreation: func() uint64 { + // Create Concentrated Liquidity pool with a denom pair not already stored + clPool := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("hook", "uosmo") + + // Ensure that the concentrated pool is stored since no other pool exists for the denom pair + setPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "hook") + s.Require().NoError(err) + s.Require().Equal(clPool.GetId(), setPoolId) + + // Get clPool liquidity + clPoolLiquidity, err := s.App.PoolManagerKeeper.GetTotalPoolLiquidity(s.Ctx, clPool.GetId()) + s.Require().NoError(err) + + // Create balancer pool with the same denom pair and more liquidity + balancerPoolId := s.createGAMMPool([]balancer.PoolAsset{ + { + Token: sdk.NewCoin(clPoolLiquidity[0].Denom, clPoolLiquidity[0].Amount.Add(sdk.NewInt(1))), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin(clPoolLiquidity[1].Denom, clPoolLiquidity[1].Amount.Add(sdk.NewInt(1))), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(1, 1), + sdk.NewDecWithPrec(0, 2), + ) + + // The returned pool id should be what is finally stored in the protorev keeper since it has more liquidity + return balancerPoolId + }, + }, + expectPass: true, + }, } for _, tc := range tests { s.Run(tc.name, func() { + s.SetupTest() poolId := tc.param.executePoolCreation() setPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, types.OsmosisDenomination, tc.param.matchDenom) diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 408776d397a..de265e278bb 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -137,6 +137,7 @@ func (s *KeeperTestSuite) SetupTest() { sdk.NewCoin("epochTwo", sdk.NewInt(9000000000000000000)), sdk.NewCoin("hookGamm", sdk.NewInt(9000000000000000000)), sdk.NewCoin("hookCL", sdk.NewInt(9000000000000000000)), + sdk.NewCoin("hook", sdk.NewInt(9000000000000000000)), ) s.fundAllAccountsWith() s.Commit() From 93e1edde3a410e2954d69e8db388bf89e0b3b146 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 10:10:15 -0400 Subject: [PATCH 45/67] v16 version bump --- x/protorev/keeper/hooks.go | 4 ++-- x/protorev/keeper/hooks_test.go | 6 +++--- x/protorev/keeper/posthandler_test.go | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 5f88136dfee..cb0bba25cd6 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -3,8 +3,8 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" - gammtypes "github.com/osmosis-labs/osmosis/v15/x/gamm/types" - "github.com/osmosis-labs/osmosis/v15/x/protorev/types" + gammtypes "github.com/osmosis-labs/osmosis/v16/x/gamm/types" + "github.com/osmosis-labs/osmosis/v16/x/protorev/types" ) type Hooks struct { diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 4c1d0129d56..b3329b137c7 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -3,9 +3,9 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/osmosis-labs/osmosis/v15/x/gamm/pool-models/balancer" - poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" - "github.com/osmosis-labs/osmosis/v15/x/protorev/types" + "github.com/osmosis-labs/osmosis/v16/x/gamm/pool-models/balancer" + poolmanagertypes "github.com/osmosis-labs/osmosis/v16/x/poolmanager/types" + "github.com/osmosis-labs/osmosis/v16/x/protorev/types" ) // Tests the hook implementation that is called after swapping diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 84531d320b7..83b217fdc2c 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -12,10 +12,10 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing" - "github.com/osmosis-labs/osmosis/v15/app/apptesting" - poolmanagertypes "github.com/osmosis-labs/osmosis/v15/x/poolmanager/types" - "github.com/osmosis-labs/osmosis/v15/x/protorev/keeper" - "github.com/osmosis-labs/osmosis/v15/x/protorev/types" + "github.com/osmosis-labs/osmosis/v16/app/apptesting" + poolmanagertypes "github.com/osmosis-labs/osmosis/v16/x/poolmanager/types" + "github.com/osmosis-labs/osmosis/v16/x/protorev/keeper" + "github.com/osmosis-labs/osmosis/v16/x/protorev/types" ) // BenchmarkBalancerSwapHighestLiquidityArb benchmarks a balancer swap that creates a single three hop arbitrage From 817fb285d8509cc1126af7a73635bd264b6ea69b Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 10:28:00 -0400 Subject: [PATCH 46/67] implement ok checking pattern for map check --- x/protorev/keeper/hooks.go | 4 ++-- x/protorev/keeper/hooks_test.go | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index cb0bba25cd6..54c73066231 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -119,10 +119,10 @@ func (k Keeper) afterPoolCreatedWithCoins(ctx sdk.Context, poolId uint64) { // Pool must be active and the number of denoms must be 2 if pool.IsActive(ctx) && len(denoms) == 2 { - if baseDenomMap[denoms[0]] { + if _, ok := baseDenomMap[denoms[0]]; ok { k.compareAndStorePool(ctx, poolId, denoms[0], denoms[1]) } - if baseDenomMap[denoms[1]] { + if _, ok := baseDenomMap[denoms[1]]; ok { k.compareAndStorePool(ctx, poolId, denoms[1], denoms[0]) } } diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index b3329b137c7..fb25865aa19 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -388,3 +388,5 @@ func (s *KeeperTestSuite) TestPoolCreation() { }) } } + +// Helper function tests From 5bda10f7a99b9a6219b2dcc8287d50717f088792 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 10:30:03 -0400 Subject: [PATCH 47/67] fix typo --- x/protorev/keeper/hooks.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 54c73066231..32592f54d84 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -157,7 +157,7 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o }() // Get comparable liquidity for the new pool - newPoolliquidity, err := k.getComparablePoolLiquidity(ctx, poolId) + newPoolLiquidity, err := k.getComparablePoolLiquidity(ctx, poolId) if err != nil { ctx.Logger().Error("Protorev error getting newPoolLiquidity in compareAndStorePool", err) return @@ -171,7 +171,7 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o } // If the new pool has more liquidity, we set it - if newPoolliquidity.GT(storedPoolLiquidity) { + if newPoolLiquidity.GT(storedPoolLiquidity) { k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) } } From 7a69efa58a70e15b29e5d4fecbbbcfc34ed4eda5 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 10:35:23 -0400 Subject: [PATCH 48/67] Use expectPass bool in test --- x/protorev/keeper/hooks_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index fb25865aa19..acfd90f3d9a 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -234,8 +234,10 @@ func (s *KeeperTestSuite) TestLiquidityChanging() { tc.param.executeLiquidityProviding() routes, err := s.App.ProtoRevKeeper.GetSwapsToBackrun(s.Ctx) - s.Require().NoError(err) - s.Require().Equal(tc.param.expectedTrades, routes.Trades) + if tc.expectPass { + s.Require().NoError(err) + s.Require().Equal(tc.param.expectedTrades, routes.Trades) + } s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) }) From 28732381b4e87cc00fb944806bfd54829b045938 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 11:13:21 -0400 Subject: [PATCH 49/67] reuse k.storeSwap in join/exit logic --- x/protorev/keeper/hooks.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 32592f54d84..7269460f811 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -197,26 +197,15 @@ func (k Keeper) storeJoinExitPoolMsgs(ctx sdk.Context, sender sdk.AccAddress, po coins := pool.GetTotalPoolLiquidity(ctx) // Create swaps to backrun being the join coin swapped against all other pool coins - swapsToBackrun := make([]types.Trade, 0) for _, coin := range coins { if coin.Denom == denom { continue } // Join messages swap in the denom, exit messages swap out the denom if isJoin { - swapsToBackrun = append(swapsToBackrun, types.Trade{ - Pool: poolId, - TokenIn: denom, - TokenOut: coin.Denom}) + k.storeSwap(ctx, poolId, denom, coin.Denom) } else { - swapsToBackrun = append(swapsToBackrun, types.Trade{ - Pool: poolId, - TokenIn: coin.Denom, - TokenOut: denom}) + k.storeSwap(ctx, poolId, coin.Denom, denom) } } - - if err := k.AddSwapsToSwapsToBackrun(ctx, swapsToBackrun); err != nil { - ctx.Logger().Error("Protorev error adding swaps to backrun from AfterJoin/ExitPool hook", err) - } } From 5393e78f910bbff28af04bef6c60b928facbee7d Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 12:44:16 -0400 Subject: [PATCH 50/67] Add direct unit test for StoreSwap helper method --- x/protorev/keeper/hooks.go | 34 +++++++-------- x/protorev/keeper/hooks_test.go | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 17 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 7269460f811..9b3d70d3d11 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -57,7 +57,7 @@ func (h Hooks) AfterCFMMSwap(ctx sdk.Context, sender sdk.AccAddress, poolId uint return } - h.k.storeSwap(ctx, poolId, input[0].Denom, output[0].Denom) + h.k.StoreSwap(ctx, poolId, input[0].Denom, output[0].Denom) } // ---------------------------------------------------------------------------- @@ -84,13 +84,26 @@ func (h Hooks) AfterConcentratedPoolSwap(ctx sdk.Context, sender sdk.AccAddress, return } - h.k.storeSwap(ctx, poolId, input[0].Denom, output[0].Denom) + 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 + } +} + // 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) { @@ -128,19 +141,6 @@ func (k Keeper) afterPoolCreatedWithCoins(ctx sdk.Context, poolId uint64) { } } -// 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 - } -} - // 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) @@ -203,9 +203,9 @@ func (k Keeper) storeJoinExitPoolMsgs(ctx sdk.Context, sender sdk.AccAddress, po } // Join messages swap in the denom, exit messages swap out the denom if isJoin { - k.storeSwap(ctx, poolId, denom, coin.Denom) + k.StoreSwap(ctx, poolId, denom, coin.Denom) } else { - k.storeSwap(ctx, poolId, coin.Denom, denom) + k.StoreSwap(ctx, poolId, coin.Denom, denom) } } } diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index acfd90f3d9a..26c1f2adb57 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -392,3 +392,77 @@ func (s *KeeperTestSuite) TestPoolCreation() { } // Helper function tests + +// Tests StoreSwap stores a swap properly +func (s *KeeperTestSuite) TestStoreSwap() { + type param struct { + expectedSwap types.Trade + prepateState func() + expectedSwapsStoredLen int + } + + tests := []struct { + name string + param param + expectPass bool + }{ + { + name: "Store Swap Without An Existing Swap Stored", + param: param{ + expectedSwap: types.Trade{ + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + prepateState: func() { + s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) + }, + expectedSwapsStoredLen: 1, + }, + expectPass: true, + }, + { + name: "Store Swap With With An Existing Swap Stored", + param: param{ + expectedSwap: types.Trade{ + Pool: 2, + TokenIn: "uosmo", + TokenOut: "test", + }, + prepateState: func() { + s.App.ProtoRevKeeper.SetSwapsToBackrun(s.Ctx, types.Route{ + Trades: []types.Trade{ + { + Pool: 1, + TokenIn: "Atom", + TokenOut: "akash", + }, + }, + StepSize: sdk.NewInt(1), + }) + }, + expectedSwapsStoredLen: 2, + }, + expectPass: true, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.SetupTest() + + // Run any state preparation + tc.param.prepateState() + + // Store the swap + s.App.ProtoRevKeeper.StoreSwap(s.Ctx, tc.param.expectedSwap.Pool, tc.param.expectedSwap.TokenIn, tc.param.expectedSwap.TokenOut) + + routes, err := s.App.ProtoRevKeeper.GetSwapsToBackrun(s.Ctx) + if tc.expectPass { + s.Require().NoError(err) + s.Require().Equal(tc.param.expectedSwapsStoredLen, len(routes.Trades)) + s.Require().Equal(tc.param.expectedSwap, routes.Trades[len(routes.Trades)-1]) + } + }) + } +} From 3aa26ea0946a8192dd4454f82f40cb87c0fbe8e8 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 13:37:16 -0400 Subject: [PATCH 51/67] Add direct unit test for GetComparablePoolLiquidity helper method --- x/protorev/keeper/hooks.go | 32 ++++++------- x/protorev/keeper/hooks_test.go | 80 ++++++++++++++++++++++++++++++++ x/protorev/keeper/keeper_test.go | 6 ++- 3 files changed, 100 insertions(+), 18 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 9b3d70d3d11..e34eb4c2f93 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -34,7 +34,7 @@ func (h Hooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint return } - h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, enterCoins[0].Denom, true) + 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. @@ -47,7 +47,7 @@ func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint return } - h.k.storeJoinExitPoolMsgs(ctx, sender, poolId, exitCoins[0].Denom, false) + 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. @@ -104,6 +104,16 @@ func (k Keeper) StoreSwap(ctx sdk.Context, poolId uint64, tokenIn, tokenOut stri } } +// 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) (sdk.Int, error) { + coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) + if err != nil { + return sdk.Int{}, err + } + + return coins[0].Amount.Mul(coins[1].Amount), nil +} + // 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) { @@ -157,14 +167,14 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o }() // Get comparable liquidity for the new pool - newPoolLiquidity, err := k.getComparablePoolLiquidity(ctx, poolId) + 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) + storedPoolLiquidity, err := k.GetComparablePoolLiquidity(ctx, storedPoolId) if err != nil { ctx.Logger().Error("Protorev error getting storedPoolLiquidity in compareAndStorePool", err) return @@ -176,18 +186,8 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o } } -// 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) (sdk.Int, error) { - coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) - if err != nil { - return sdk.Int{}, err - } - - return coins[0].Amount.Mul(coins[1].Amount), nil -} - -// storeJoinExitPoolMsgs stores the join/exit pool messages in the store, depending on if it is a join or exit. -func (k Keeper) storeJoinExitPoolMsgs(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, denom string, isJoin bool) { +// storeJoinExitPoolSwaps stores the swaps associated with 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 diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 26c1f2adb57..3f74b967617 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -4,6 +4,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/osmosis-labs/osmosis/v16/x/gamm/pool-models/balancer" + "github.com/osmosis-labs/osmosis/v16/x/gamm/pool-models/stableswap" poolmanagertypes "github.com/osmosis-labs/osmosis/v16/x/poolmanager/types" "github.com/osmosis-labs/osmosis/v16/x/protorev/types" ) @@ -466,3 +467,82 @@ func (s *KeeperTestSuite) TestStoreSwap() { }) } } + +// Tests GetComparablePoolLiquidity gets the comparable liquidity of a pool by multiplying the amounts of the pool coins. +func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { + type param struct { + executePoolCreation func() uint64 + expectedComparableLiquidity sdk.Int + } + + tests := []struct { + name string + param param + expectPass bool + }{ + { + name: "Get Balancer Pool Comparable Liquidity", + param: param{ + executePoolCreation: func() uint64 { + return s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("juno", sdk.NewInt(10))) + }, + expectedComparableLiquidity: sdk.NewInt(100), + }, + expectPass: true, + }, + { + name: "Get Stable Swap Pool Comparable Liquidity", + param: param{ + executePoolCreation: func() uint64 { + return s.createStableswapPool( + sdk.NewCoins( + sdk.NewCoin("uosmo", sdk.NewInt(10)), + sdk.NewCoin("juno", sdk.NewInt(10)), + ), + stableswap.PoolParams{ + SwapFee: sdk.NewDecWithPrec(1, 4), + ExitFee: sdk.NewDecWithPrec(0, 2), + }, + []uint64{1, 1}, + ) + }, + expectedComparableLiquidity: sdk.NewInt(100), + }, + expectPass: true, + }, + { + name: "Get Concentrated Liquidity Pool Comparable Liquidity", + param: param{ + executePoolCreation: func() uint64 { + pool := s.PrepareConcentratedPool() + err := s.App.BankKeeper.SendCoins( + s.Ctx, + s.TestAccs[0], + pool.GetAddress(), + sdk.NewCoins(sdk.NewCoin("eth", sdk.NewInt(10)), sdk.NewCoin("usdc", sdk.NewInt(10)))) + s.Require().NoError(err) + return pool.GetId() + }, + expectedComparableLiquidity: sdk.NewInt(100), + }, + expectPass: true, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.SetupTest() + + // Create the pool + poolId := tc.param.executePoolCreation() + + // Get the comparable liquidity + comparableLiquidity, err := s.App.ProtoRevKeeper.GetComparablePoolLiquidity(s.Ctx, poolId) + + if tc.expectPass { + s.Require().NoError(err) + s.Require().Equal(tc.param.expectedComparableLiquidity, comparableLiquidity) + } + }) + } +} diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 44032e01c57..35f41c2a9a3 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -138,6 +138,7 @@ func (s *KeeperTestSuite) SetupTest() { sdk.NewCoin("hookGamm", sdk.NewInt(9000000000000000000)), sdk.NewCoin("hookCL", sdk.NewInt(9000000000000000000)), sdk.NewCoin("hook", sdk.NewInt(9000000000000000000)), + sdk.NewCoin("eth", sdk.NewInt(9000000000000000000)), ) s.fundAllAccountsWith() s.Commit() @@ -900,11 +901,12 @@ func (s *KeeperTestSuite) setUpPools() { } // createStableswapPool creates a stableswap pool with the given pool assets and params -func (s *KeeperTestSuite) createStableswapPool(initialLiquidity sdk.Coins, poolParams stableswap.PoolParams, scalingFactors []uint64) { - _, err := s.App.PoolManagerKeeper.CreatePool( +func (s *KeeperTestSuite) createStableswapPool(initialLiquidity sdk.Coins, poolParams stableswap.PoolParams, scalingFactors []uint64) uint64 { + poolId, err := s.App.PoolManagerKeeper.CreatePool( s.Ctx, stableswap.NewMsgCreateStableswapPool(s.TestAccs[1], poolParams, initialLiquidity, scalingFactors, "")) s.Require().NoError(err) + return poolId } // createGAMMPool creates a balancer pool with the given pool assets and params From 7fc81f8776f6e53dd369dea916916bbc15373357 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:33:19 -0400 Subject: [PATCH 52/67] Add direct unit test for StoreJoinExitPoolSwaps helper method --- x/protorev/keeper/hooks.go | 30 ++++++++++++++-- x/protorev/keeper/hooks_test.go | 62 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index e34eb4c2f93..919b75faa8c 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -34,7 +34,7 @@ func (h Hooks) AfterJoinPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint return } - h.k.storeJoinExitPoolSwaps(ctx, sender, poolId, enterCoins[0].Denom, true) + 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. @@ -47,7 +47,7 @@ func (h Hooks) AfterExitPool(ctx sdk.Context, sender sdk.AccAddress, poolId uint return } - h.k.storeJoinExitPoolSwaps(ctx, sender, poolId, exitCoins[0].Denom, false) + 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. @@ -104,7 +104,7 @@ func (k Keeper) StoreSwap(ctx sdk.Context, poolId uint64, tokenIn, tokenOut stri } } -// getComparablePoolLiquidity gets the comparable liquidity of a pool by multiplying the amounts of the pool coins. +// 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) (sdk.Int, error) { coins, err := k.poolmanagerKeeper.GetTotalPoolLiquidity(ctx, poolId) if err != nil { @@ -114,6 +114,30 @@ func (k Keeper) GetComparablePoolLiquidity(ctx sdk.Context, poolId uint64) (sdk. return coins[0].Amount.Mul(coins[1].Amount), 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) { diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 3f74b967617..07e3a9d20b4 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -546,3 +546,65 @@ func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { }) } } + +// Tests StoreJoinExitPoolSwaps stores the swaps associated with GAMM join/exit pool messages in the store, depending on if it is a join or exit. +func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() { + type param struct { + poolId uint64 + denom string + isJoin bool + expectedSwap types.Trade + } + + tests := []struct { + name string + param param + expectPass bool + }{ + { + name: "Store Join Pool Swap", + param: param{ + poolId: 1, + denom: "akash", + isJoin: true, + expectedSwap: types.Trade{ + Pool: 1, + TokenIn: "akash", + TokenOut: "Atom", + }, + }, + expectPass: true, + }, + { + name: "Store Exit Pool Swap", + param: param{ + poolId: 1, + denom: "akash", + isJoin: false, + expectedSwap: types.Trade{ + Pool: 1, + TokenIn: "Atom", + TokenOut: "akash", + }, + }, + expectPass: true, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.SetupTest() + + // All pools are already created in the setup + s.App.ProtoRevKeeper.StoreJoinExitPoolSwaps(s.Ctx, s.TestAccs[0], tc.param.poolId, tc.param.denom, tc.param.isJoin) + + // Get the swaps to backrun after storing the swap via StoreJoinExitPoolSwaps + swapsToBackrun, err := s.App.ProtoRevKeeper.GetSwapsToBackrun(s.Ctx) + + if tc.expectPass { + s.Require().NoError(err) + s.Require().Equal(tc.param.expectedSwap, swapsToBackrun.Trades[len(swapsToBackrun.Trades)-1]) + } + }) + } +} From da7c839388549846e35c1e8936758441c3684e4d Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 14:34:38 -0400 Subject: [PATCH 53/67] Remove old function --- x/protorev/keeper/hooks.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 919b75faa8c..15fa103e5d0 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -209,27 +209,3 @@ func (k Keeper) compareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o k.SetPoolForDenomPair(ctx, baseDenom, otherDenom, poolId) } } - -// storeJoinExitPoolSwaps stores the swaps associated with 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) - } - } -} From 924a7495adff8adb3c976c450b8ac6ae5ee1b28f Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 15:04:42 -0400 Subject: [PATCH 54/67] Add direct unit test for CompareAndStorePool helper method --- x/protorev/keeper/hooks.go | 8 +- x/protorev/keeper/hooks_test.go | 142 +++++++++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 8 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 15fa103e5d0..535123e7441 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -167,16 +167,16 @@ func (k Keeper) afterPoolCreatedWithCoins(ctx sdk.Context, poolId uint64) { // Pool must be active and the number of denoms must be 2 if pool.IsActive(ctx) && len(denoms) == 2 { if _, ok := baseDenomMap[denoms[0]]; ok { - k.compareAndStorePool(ctx, poolId, denoms[0], denoms[1]) + k.CompareAndStorePool(ctx, poolId, denoms[0], denoms[1]) } if _, ok := baseDenomMap[denoms[1]]; ok { - k.compareAndStorePool(ctx, poolId, denoms[1], denoms[0]) + 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) { +// 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 diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 07e3a9d20b4..5bc5b614dc2 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -398,7 +398,7 @@ func (s *KeeperTestSuite) TestPoolCreation() { func (s *KeeperTestSuite) TestStoreSwap() { type param struct { expectedSwap types.Trade - prepateState func() + prepareState func() expectedSwapsStoredLen int } @@ -415,7 +415,7 @@ func (s *KeeperTestSuite) TestStoreSwap() { TokenIn: "akash", TokenOut: "Atom", }, - prepateState: func() { + prepareState: func() { s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) }, expectedSwapsStoredLen: 1, @@ -430,7 +430,7 @@ func (s *KeeperTestSuite) TestStoreSwap() { TokenIn: "uosmo", TokenOut: "test", }, - prepateState: func() { + prepareState: func() { s.App.ProtoRevKeeper.SetSwapsToBackrun(s.Ctx, types.Route{ Trades: []types.Trade{ { @@ -453,7 +453,7 @@ func (s *KeeperTestSuite) TestStoreSwap() { s.SetupTest() // Run any state preparation - tc.param.prepateState() + tc.param.prepareState() // Store the swap s.App.ProtoRevKeeper.StoreSwap(s.Ctx, tc.param.expectedSwap.Pool, tc.param.expectedSwap.TokenIn, tc.param.expectedSwap.TokenOut) @@ -608,3 +608,137 @@ func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() { }) } } + +// Tests CompareAndStorePool compares the comparable liquidity of a pool with the stored pool, storing the new pool if it has higher comparable liquidity. +func (s *KeeperTestSuite) TestCompareAndStorePool() { + type param struct { + baseDenom string + matchDenom string + prepareStateAndGetExpectedPoolId func() uint64 + } + + tests := []struct { + name string + param param + expectPass bool + }{ + { + name: "Nothing Stored, Store Balancer", + param: param{ + baseDenom: "uosmo", + matchDenom: "juno", + prepareStateAndGetExpectedPoolId: func() uint64 { + return s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("juno", sdk.NewInt(10))) + }, + }, + expectPass: true, + }, + { + name: "Nothing Stored, Store Concentrated Liquidity Pool w/ Coins", + param: param{ + baseDenom: "uosmo", + matchDenom: "stake", + prepareStateAndGetExpectedPoolId: func() uint64 { + return s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + }, + }, + expectPass: true, + }, + { + name: "Balancer Previously Stored w/ Less liquidity, Compare Concentrated Liquidity Pool w/ More liqudidity, Ensure CL Gets Stored", + param: param{ + baseDenom: "uosmo", + matchDenom: "stake", + prepareStateAndGetExpectedPoolId: func() uint64 { + preparedPoolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("stake", sdk.NewInt(10))) + storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") + s.Require().NoError(err) + s.Require().Equal(preparedPoolId, storedPoolId) + + return s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + }, + }, + expectPass: true, + }, + { + name: "Balancer Previously Stored w/ More liquidity, Compare Concentrated Liquidity Pool w/ Less liqudidity, Ensure Balancer Stays Stored", + param: param{ + baseDenom: "uosmo", + matchDenom: "stake", + prepareStateAndGetExpectedPoolId: func() uint64 { + // Prepare a balancer pool with more liquidity + balancerPoolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(2000000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) + storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") + s.Require().NoError(err) + s.Require().Equal(balancerPoolId, storedPoolId) + + // Prepare a concentrated liquidity pool with less liquidity + s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake") + + // Return the balancer pool id since it should be stored + return balancerPoolId + }, + }, + expectPass: true, + }, + { + name: "Concentrated Liquidity Previously Stored w/ Less liquidity, Compare Balancer Pool w/ More liqudidity, Ensure Balancer Gets Stored", + param: param{ + baseDenom: "uosmo", + matchDenom: "stake", + prepareStateAndGetExpectedPoolId: func() uint64 { + // Prepare a concentrated liquidity pool with less liquidity, should be stored since nothing is stored + clPoolId := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") + s.Require().NoError(err) + s.Require().Equal(clPoolId, storedPoolId) + + // Prepare a balancer pool with more liquidity and return the pool id since it should be stored + return s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(2000000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) + }, + }, + expectPass: true, + }, + { + name: "Concentrated Liquidity Previously Stored w/ More liquidity, Compare Balancer Pool w/ Less liqudidity, Ensure CL Stays Stored", + param: param{ + baseDenom: "uosmo", + matchDenom: "stake", + prepareStateAndGetExpectedPoolId: func() uint64 { + // Prepare a concentrated liquidity pool with less liquidity, should be stored since nothing is stored + clPoolId := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") + s.Require().NoError(err) + s.Require().Equal(clPoolId, storedPoolId) + + // Prepare a balancer pool with less liquidity + s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(500000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) + + // Return the cl pool id since it should be stored + return clPoolId + }, + }, + expectPass: true, + }, + } + + for _, tc := range tests { + s.Run(tc.name, func() { + s.SetupTest() + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + + // Run any state preparation and get the expected pool id + poolId := tc.param.prepareStateAndGetExpectedPoolId() + + // Get the stored pool id for the highest liquidity pool in protorev + storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, tc.param.baseDenom, tc.param.matchDenom) + + if tc.expectPass { + s.Require().NoError(err) + s.Require().Equal(poolId, storedPoolId) + } + }) + } +} From 7fcdb558c05ef562dbd128f024818b032da30499 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 15:27:17 -0400 Subject: [PATCH 55/67] Actually test CompareAndStorePool --- x/protorev/keeper/hooks_test.go | 89 ++++++++++++++++++++++----------- 1 file changed, 60 insertions(+), 29 deletions(-) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 5bc5b614dc2..dffcb61de42 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -612,9 +612,9 @@ func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() { // Tests CompareAndStorePool compares the comparable liquidity of a pool with the stored pool, storing the new pool if it has higher comparable liquidity. func (s *KeeperTestSuite) TestCompareAndStorePool() { type param struct { - baseDenom string - matchDenom string - prepareStateAndGetExpectedPoolId func() uint64 + baseDenom string + matchDenom string + prepareStateAndGetPoolIdToCompare func() (uint64, uint64) } tests := []struct { @@ -627,8 +627,14 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { param: param{ baseDenom: "uosmo", matchDenom: "juno", - prepareStateAndGetExpectedPoolId: func() uint64 { - return s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("juno", sdk.NewInt(10))) + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Prepare a balancer pool with coins + poolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("juno", sdk.NewInt(10))) + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + + return poolId, poolId }, }, expectPass: true, @@ -638,8 +644,14 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { param: param{ baseDenom: "uosmo", matchDenom: "stake", - prepareStateAndGetExpectedPoolId: func() uint64 { - return s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Prepare a concentrated liquidity pool with coins + poolId := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + + return poolId, poolId }, }, expectPass: true, @@ -649,13 +661,20 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { param: param{ baseDenom: "uosmo", matchDenom: "stake", - prepareStateAndGetExpectedPoolId: func() uint64 { + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Create a concentrated liquidity pool with more liquidity + clPoolId := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + preparedPoolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("stake", sdk.NewInt(10))) storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") s.Require().NoError(err) s.Require().Equal(preparedPoolId, storedPoolId) - return s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + // Return the cl pool id as expected since it has higher liquidity, compare the cl pool id + return clPoolId, clPoolId }, }, expectPass: true, @@ -665,18 +684,21 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { param: param{ baseDenom: "uosmo", matchDenom: "stake", - prepareStateAndGetExpectedPoolId: func() uint64 { + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Create a concentrated liquidity pool with more liquidity + clPoolId := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + // Prepare a balancer pool with more liquidity balancerPoolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(2000000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") s.Require().NoError(err) s.Require().Equal(balancerPoolId, storedPoolId) - // Prepare a concentrated liquidity pool with less liquidity - s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake") - - // Return the balancer pool id since it should be stored - return balancerPoolId + // Return the balancer pool id as expected since it has higher liquidity, compare the cl pool id + return balancerPoolId, clPoolId }, }, expectPass: true, @@ -686,15 +708,21 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { param: param{ baseDenom: "uosmo", matchDenom: "stake", - prepareStateAndGetExpectedPoolId: func() uint64 { + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Prepare a balancer pool with more liquidity + balancerPoolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(2000000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + // Prepare a concentrated liquidity pool with less liquidity, should be stored since nothing is stored clPoolId := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") s.Require().NoError(err) s.Require().Equal(clPoolId, storedPoolId) - // Prepare a balancer pool with more liquidity and return the pool id since it should be stored - return s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(2000000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) + // Return the balancer pool id as expected since it has higher liquidity, compare the balancer pool id + return balancerPoolId, balancerPoolId }, }, expectPass: true, @@ -704,18 +732,21 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { param: param{ baseDenom: "uosmo", matchDenom: "stake", - prepareStateAndGetExpectedPoolId: func() uint64 { + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Prepare a balancer pool with less liquidity + balancerPoolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(500000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + // Prepare a concentrated liquidity pool with less liquidity, should be stored since nothing is stored clPoolId := s.PrepareConcentratedPoolWithCoinsAndFullRangePosition("uosmo", "stake").GetId() storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, "uosmo", "stake") s.Require().NoError(err) s.Require().Equal(clPoolId, storedPoolId) - // Prepare a balancer pool with less liquidity - s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(500000000000000000)), sdk.NewCoin("stake", sdk.NewInt(1000000000000000000))) - - // Return the cl pool id since it should be stored - return clPoolId + // Return the cl pool id as expected since it has higher liquidity, compare the balancer pool id + return clPoolId, balancerPoolId }, }, expectPass: true, @@ -726,18 +757,18 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { s.Run(tc.name, func() { s.SetupTest() - // Delete all pools for the base denom uosmo so that all tests start with a clean slate - s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + // Run any state preparation and get the pool id to compare to the stored pool + expectedStoredPoolId, comparePoolId := tc.param.prepareStateAndGetPoolIdToCompare() - // Run any state preparation and get the expected pool id - poolId := tc.param.prepareStateAndGetExpectedPoolId() + // Compare and store the pool + s.App.ProtoRevKeeper.CompareAndStorePool(s.Ctx, comparePoolId, tc.param.baseDenom, tc.param.matchDenom) // Get the stored pool id for the highest liquidity pool in protorev storedPoolId, err := s.App.ProtoRevKeeper.GetPoolForDenomPair(s.Ctx, tc.param.baseDenom, tc.param.matchDenom) if tc.expectPass { s.Require().NoError(err) - s.Require().Equal(poolId, storedPoolId) + s.Require().Equal(expectedStoredPoolId, storedPoolId) } }) } From a0d2a3b7110ab108cdbd5d9b563b4e0babfac8c5 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 15:34:38 -0400 Subject: [PATCH 56/67] export AfterPoolCreatedWithCoins --- x/protorev/keeper/hooks.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 535123e7441..b5892634e32 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -24,7 +24,7 @@ func (k Keeper) Hooks() Hooks { return Hooks{k} } // 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) + h.k.AfterPoolCreatedWithCoins(ctx, poolId) } // AfterJoinPool stores swaps to be checked by protorev given the coins entered into the pool. @@ -70,7 +70,7 @@ func (h Hooks) AfterConcentratedPoolCreated(ctx sdk.Context, sender sdk.AccAddre // 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) + h.k.AfterPoolCreatedWithCoins(ctx, poolId) } // AfterLastPoolPositionRemoved is a noop. @@ -138,9 +138,9 @@ func (k Keeper) StoreJoinExitPoolSwaps(ctx sdk.Context, sender sdk.AccAddress, p } } -// afterPoolCreatedWithCoins checks if the new pool should be stored as the highest liquidity pool +// 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) { +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) From e5d17c636c2f8a3442cff3fc18ebe56ad0415180 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 19:30:00 -0400 Subject: [PATCH 57/67] Clarify why the test calls DeleteAllPoolsForBaseDenom --- x/protorev/keeper/hooks_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index dffcb61de42..ad0dbc51135 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -610,6 +610,8 @@ func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() { } // Tests CompareAndStorePool compares the comparable liquidity of a pool with the stored pool, storing the new pool if it has higher comparable liquidity. +// Note: This test calls DeleteAllPoolsForBaseDenom in the prepareStateAndGetPoolIdToCompare function because the +// hooks are triggered by default and we want to test the CompareAndStorePool on the state before the hooks are triggered. func (s *KeeperTestSuite) TestCompareAndStorePool() { type param struct { baseDenom string From 22d528e23f5ff3733c0d7b4c83f03e6914ecf5c4 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 21:43:03 -0400 Subject: [PATCH 58/67] 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 --- x/protorev/keeper/hooks.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index b5892634e32..a2ffc480826 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -105,13 +105,23 @@ func (k Keeper) StoreSwap(ctx sdk.Context, poolId uint64, tokenIn, tokenOut stri } // 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) (sdk.Int, error) { +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 } - return coins[0].Amount.Mul(coins[1].Amount), nil + // Recover from overflow panic + defer func() { + if r := recover(); r != nil { + comparableLiquidity = sdk.Int{} + err = sdk.ErrIntOverflowAbci + } + }() + + 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. @@ -184,12 +194,6 @@ func (k Keeper) CompareAndStorePool(ctx sdk.Context, poolId uint64, baseDenom, o return } - defer func() { - if r := recover(); r != nil { - ctx.Logger().Error("Protorev error recovering from panic in AfterCFMMPoolCreated hook, likely an overflow error", r) - } - }() - // Get comparable liquidity for the new pool newPoolLiquidity, err := k.GetComparablePoolLiquidity(ctx, poolId) if err != nil { From ed66bce110964f30b32d9d2a2c88922c522d5556 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 22:06:55 -0400 Subject: [PATCH 59/67] Add int overflow panic catching test --- x/protorev/keeper/hooks.go | 4 +++- x/protorev/keeper/hooks_test.go | 15 +++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index a2ffc480826..612010aca44 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -1,6 +1,8 @@ package keeper import ( + "errors" + sdk "github.com/cosmos/cosmos-sdk/types" gammtypes "github.com/osmosis-labs/osmosis/v16/x/gamm/types" @@ -115,7 +117,7 @@ func (k Keeper) GetComparablePoolLiquidity(ctx sdk.Context, poolId uint64) (comp defer func() { if r := recover(); r != nil { comparableLiquidity = sdk.Int{} - err = sdk.ErrIntOverflowAbci + err = errors.New("Int overflow in GetComparablePoolLiquidity") } }() diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index ad0dbc51135..bd2984f4861 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -527,6 +527,18 @@ func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { }, expectPass: true, }, + { + name: "Catch overflow error on liquidity multiplication", + param: param{ + executePoolCreation: func() uint64 { + return s.PrepareBalancerPoolWithCoins( + sdk.NewCoin("uosmo", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999"))), + sdk.NewCoin("juno", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999")))) + }, + expectedComparableLiquidity: sdk.Int{}, + }, + expectPass: false, + }, } for _, tc := range tests { @@ -542,6 +554,9 @@ func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { if tc.expectPass { s.Require().NoError(err) s.Require().Equal(tc.param.expectedComparableLiquidity, comparableLiquidity) + } else { + s.Require().Error(err) + s.Require().Equal(tc.param.expectedComparableLiquidity, comparableLiquidity) } }) } From 61e7076eb385c16dcc65c786a8279d40a27cbca0 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 22:17:51 -0400 Subject: [PATCH 60/67] Revert "Add int overflow panic catching test" This reverts commit ed66bce110964f30b32d9d2a2c88922c522d5556. --- x/protorev/keeper/hooks.go | 4 +--- x/protorev/keeper/hooks_test.go | 15 --------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 612010aca44..a2ffc480826 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -1,8 +1,6 @@ package keeper import ( - "errors" - sdk "github.com/cosmos/cosmos-sdk/types" gammtypes "github.com/osmosis-labs/osmosis/v16/x/gamm/types" @@ -117,7 +115,7 @@ func (k Keeper) GetComparablePoolLiquidity(ctx sdk.Context, poolId uint64) (comp defer func() { if r := recover(); r != nil { comparableLiquidity = sdk.Int{} - err = errors.New("Int overflow in GetComparablePoolLiquidity") + err = sdk.ErrIntOverflowAbci } }() diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index bd2984f4861..ad0dbc51135 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -527,18 +527,6 @@ func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { }, expectPass: true, }, - { - name: "Catch overflow error on liquidity multiplication", - param: param{ - executePoolCreation: func() uint64 { - return s.PrepareBalancerPoolWithCoins( - sdk.NewCoin("uosmo", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999"))), - sdk.NewCoin("juno", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999")))) - }, - expectedComparableLiquidity: sdk.Int{}, - }, - expectPass: false, - }, } for _, tc := range tests { @@ -554,9 +542,6 @@ func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { if tc.expectPass { s.Require().NoError(err) s.Require().Equal(tc.param.expectedComparableLiquidity, comparableLiquidity) - } else { - s.Require().Error(err) - s.Require().Equal(tc.param.expectedComparableLiquidity, comparableLiquidity) } }) } From 995b1151d26702f1e5eaceee5983b5829b799f5d Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 22:32:50 -0400 Subject: [PATCH 61/67] Add int overflow panic catch test - try 2 --- x/protorev/keeper/hooks_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index ad0dbc51135..bd2984f4861 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -527,6 +527,18 @@ func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { }, expectPass: true, }, + { + name: "Catch overflow error on liquidity multiplication", + param: param{ + executePoolCreation: func() uint64 { + return s.PrepareBalancerPoolWithCoins( + sdk.NewCoin("uosmo", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999"))), + sdk.NewCoin("juno", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999")))) + }, + expectedComparableLiquidity: sdk.Int{}, + }, + expectPass: false, + }, } for _, tc := range tests { @@ -542,6 +554,9 @@ func (s *KeeperTestSuite) TestGetComparablePoolLiquidity() { if tc.expectPass { s.Require().NoError(err) s.Require().Equal(tc.param.expectedComparableLiquidity, comparableLiquidity) + } else { + s.Require().Error(err) + s.Require().Equal(tc.param.expectedComparableLiquidity, comparableLiquidity) } }) } From 026fe772d0b96319259d525db1fd203eaa01d221 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 23:30:18 -0400 Subject: [PATCH 62/67] Add custom error message --- x/protorev/keeper/hooks.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index a2ffc480826..612010aca44 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -1,6 +1,8 @@ package keeper import ( + "errors" + sdk "github.com/cosmos/cosmos-sdk/types" gammtypes "github.com/osmosis-labs/osmosis/v16/x/gamm/types" @@ -115,7 +117,7 @@ func (k Keeper) GetComparablePoolLiquidity(ctx sdk.Context, poolId uint64) (comp defer func() { if r := recover(); r != nil { comparableLiquidity = sdk.Int{} - err = sdk.ErrIntOverflowAbci + err = errors.New("Int overflow in GetComparablePoolLiquidity") } }() From c18ec190e1d60747e6b89a0578a111471cf6e601 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Mon, 5 Jun 2023 23:54:16 -0400 Subject: [PATCH 63/67] Add overflow catching logical branch tests for CompareAndStorePool --- x/protorev/keeper/hooks_test.go | 46 +++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index bd2984f4861..1447b18860f 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -768,6 +768,52 @@ func (s *KeeperTestSuite) TestCompareAndStorePool() { }, expectPass: true, }, + { + name: "Catch overflow error when getting newPoolLiquidity - Ensure test doesn't panic", + param: param{ + baseDenom: "uosmo", + matchDenom: "stake", + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Prepare a balancer pool with liquidity levels that will overflow when multiplied + overflowPoolId := s.PrepareBalancerPoolWithCoins( + sdk.NewCoin("uosmo", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999"))), + sdk.NewCoin("stake", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999")))) + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + + // Prepare a balancer pool with normal liquidity + poolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("stake", sdk.NewInt(10))) + + // The normal liquidity pool should be stored since the function will return early when catching the overflow error + return poolId, overflowPoolId + }, + }, + expectPass: true, + }, + { + name: "Catch overflow error when getting storedPoolLiquidity - Ensure test doesn't panic", + param: param{ + baseDenom: "uosmo", + matchDenom: "stake", + prepareStateAndGetPoolIdToCompare: func() (uint64, uint64) { + // Prepare a balancer pool with normal liquidity + poolId := s.PrepareBalancerPoolWithCoins(sdk.NewCoin("uosmo", sdk.NewInt(10)), sdk.NewCoin("stake", sdk.NewInt(10))) + + // Delete all pools for the base denom uosmo so that all tests start with a clean slate + s.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(s.Ctx, "uosmo") + + // Prepare a balancer pool with liquidity levels that will overflow when multiplied + overflowPoolId := s.PrepareBalancerPoolWithCoins( + sdk.NewCoin("uosmo", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999"))), + sdk.NewCoin("stake", sdk.Int(sdk.NewUintFromString("999999999999999999999999999999999999999")))) + + // The overflow pool should be stored since the function will return early when catching the overflow error + return overflowPoolId, poolId + }, + }, + expectPass: true, + }, } for _, tc := range tests { From c2db6d772fad7e4fe77770e757e5561d05ac9c75 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 6 Jun 2023 00:07:14 -0400 Subject: [PATCH 64/67] Add non-gamm pool error test for StoreJoinExitPoolSwaps() --- x/protorev/keeper/hooks_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index 1447b18860f..cd1dfeaab43 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -604,6 +604,16 @@ func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() { }, expectPass: true, }, + { + name: "Non-Gamm Pool, Return Early Do Not Store Any Swaps", + param: param{ + poolId: 49, + denom: "uosmo", + isJoin: true, + expectedSwap: types.Trade{}, + }, + expectPass: false, + }, } for _, tc := range tests { @@ -619,6 +629,8 @@ func (s *KeeperTestSuite) TestStoreJoinExitPoolSwaps() { if tc.expectPass { s.Require().NoError(err) s.Require().Equal(tc.param.expectedSwap, swapsToBackrun.Trades[len(swapsToBackrun.Trades)-1]) + } else { + s.Require().Equal(swapsToBackrun, types.Route{}) } }) } From 460da4a42d4c3cf341890d0537982419dd6aee51 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 6 Jun 2023 00:15:26 -0400 Subject: [PATCH 65/67] Use s.SetupTest() and give testAcc pool shares --- x/protorev/keeper/hooks_test.go | 3 +-- x/protorev/keeper/keeper_test.go | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x/protorev/keeper/hooks_test.go b/x/protorev/keeper/hooks_test.go index cd1dfeaab43..6166fa22529 100644 --- a/x/protorev/keeper/hooks_test.go +++ b/x/protorev/keeper/hooks_test.go @@ -232,6 +232,7 @@ func (s *KeeperTestSuite) TestLiquidityChanging() { for _, tc := range tests { s.Run(tc.name, func() { + s.SetupTest() tc.param.executeLiquidityProviding() routes, err := s.App.ProtoRevKeeper.GetSwapsToBackrun(s.Ctx) @@ -239,8 +240,6 @@ func (s *KeeperTestSuite) TestLiquidityChanging() { s.Require().NoError(err) s.Require().Equal(tc.param.expectedTrades, routes.Trades) } - - s.App.ProtoRevKeeper.DeleteSwapsToBackrun(s.Ctx) }) } } diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index 35f41c2a9a3..e568bc87ae1 100644 --- a/x/protorev/keeper/keeper_test.go +++ b/x/protorev/keeper/keeper_test.go @@ -139,6 +139,7 @@ func (s *KeeperTestSuite) SetupTest() { sdk.NewCoin("hookCL", sdk.NewInt(9000000000000000000)), sdk.NewCoin("hook", sdk.NewInt(9000000000000000000)), sdk.NewCoin("eth", sdk.NewInt(9000000000000000000)), + sdk.NewCoin("gamm/pool/1", sdk.NewInt(9000000000000000000)), ) s.fundAllAccountsWith() s.Commit() From 61d3474a6a489d1247c556950e28aaa46209ebb0 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 6 Jun 2023 00:19:09 -0400 Subject: [PATCH 66/67] Add clarifying comment about gas meter --- x/protorev/keeper/posthandler.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/protorev/keeper/posthandler.go b/x/protorev/keeper/posthandler.go index e81f0c9e322..ec60ff4621f 100644 --- a/x/protorev/keeper/posthandler.go +++ b/x/protorev/keeper/posthandler.go @@ -62,6 +62,7 @@ func (protoRevDec ProtoRevDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simu } // Delete swaps to backrun for next transaction without consuming gas + // from the current transaction's gas meter, but instead from a new gas meter protoRevDec.ProtoRevKeeper.DeleteSwapsToBackrun(ctx.WithGasMeter(upperGasLimitMeter)) return next(ctx, tx, simulate) From 8966b6a0e3bf2cc11ba5c13f91f8429a7e014625 Mon Sep 17 00:00:00 2001 From: Jeremy Liu <31809888+NotJeremyLiu@users.noreply.github.com> Date: Tue, 6 Jun 2023 00:40:08 -0400 Subject: [PATCH 67/67] Add comment explaining the base denoms check -> CompareAndStorePool logic --- x/protorev/keeper/hooks.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x/protorev/keeper/hooks.go b/x/protorev/keeper/hooks.go index 612010aca44..2904b5c3165 100644 --- a/x/protorev/keeper/hooks.go +++ b/x/protorev/keeper/hooks.go @@ -178,6 +178,11 @@ func (k Keeper) AfterPoolCreatedWithCoins(ctx sdk.Context, poolId uint64) { // 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]) }