diff --git a/CHANGELOG.md b/CHANGELOG.md index d82a2341046..fdeeacb2d27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,8 +88,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#5890](https://github.com/osmosis-labs/osmosis/pull/5890) feat: CreateCLPool & LinkCFMMtoCL pool into one gov-prop * [#5959](https://github.com/osmosis-labs/osmosis/pull/5959) allow testing with different chain-id's in E2E testing * [#5964](https://github.com/osmosis-labs/osmosis/pull/5964) fix e2e test concurrency bugs -* [#5948](https://github.com/osmosis-labs/osmosis/pull/5948) Parameterizing Pool Type Information in Protorev -* [#6001](https://github.com/osmosis-labs/osmosis/pull/6001) feat: improve set-env CLI cmd +* [#5948] (https://github.com/osmosis-labs/osmosis/pull/5948) Parameterizing Pool Type Information in Protorev +* [#6001](https://github.com/osmosis-labs/osmosis/pull/6001) feat: improve set-env CLI cmd\ +* [#5953] (https://github.com/osmosis-labs/osmosis/pull/5953) Supporting two pool routes in ProtoRev * [#6012](https://github.com/osmosis-labs/osmosis/pull/6012) chore: add autocomplete to makefile ### Minor improvements & Bug Fixes diff --git a/tests/e2e/configurer/chain/commands.go b/tests/e2e/configurer/chain/commands.go index f419dc6259e..a9c3c3e9358 100644 --- a/tests/e2e/configurer/chain/commands.go +++ b/tests/e2e/configurer/chain/commands.go @@ -42,6 +42,13 @@ type params struct { Value string `json:"value"` } +func (n *NodeConfig) SetMaxPoolPointsPerTx(maxPoolPoints int, from string) { + n.LogActionF("setting max pool points per tx for protorev at %d points", maxPoolPoints) + cmd := []string{"osmosisd", "tx", "protorev", "set-max-pool-points-per-tx", fmt.Sprintf("%d", maxPoolPoints), fmt.Sprintf("--from=%s", from), "--gas=700000", "--fees=5000uosmo"} + _, _, err := n.containerManager.ExecTxCmd(n.t, n.chainId, n.Name, cmd) + require.NoError(n.t, err) +} + func (n *NodeConfig) CreateBalancerPool(poolFile, from string) uint64 { n.LogActionF("creating balancer pool from file %s", poolFile) cmd := []string{"osmosisd", "tx", "gamm", "create-pool", fmt.Sprintf("--pool-file=/osmosis/%s", poolFile), fmt.Sprintf("--from=%s", from), "--gas=700000", "--fees=5000uosmo"} diff --git a/tests/e2e/e2e_test.go b/tests/e2e/e2e_test.go index 13237521061..b0c1fb919a5 100644 --- a/tests/e2e/e2e_test.go +++ b/tests/e2e/e2e_test.go @@ -35,6 +35,7 @@ import ( "github.com/osmosis-labs/osmosis/v17/tests/e2e/initialization" clmath "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/math" cltypes "github.com/osmosis-labs/osmosis/v17/x/concentrated-liquidity/types" + protorevtypes "github.com/osmosis-labs/osmosis/v17/x/protorev/types" ) var ( @@ -326,6 +327,15 @@ func (s *IntegrationTestSuite) ConcentratedLiquidity() { err = chainABNode.ParamChangeProposal("concentratedliquidity", string(cltypes.KeyIsPermisionlessPoolCreationEnabled), []byte("true"), chainAB) s.Require().NoError(err) + // Update the protorev admin address to a known wallet we can control + + adminWalletAddr := chainABNode.CreateWalletAndFund("admin", []string{"4000000uosmo"}, chainAB) + err = chainABNode.ParamChangeProposal("protorev", string(protorevtypes.ParamStoreKeyAdminAccount), []byte(fmt.Sprintf(`"%s"`, adminWalletAddr)), chainAB) + s.Require().NoError(err) + + // Update the weight of CL pools so that this test case is not back run by protorev. + chainABNode.SetMaxPoolPointsPerTx(7, adminWalletAddr) + // Confirm that the parameter has been changed. isPermisionlessCreationEnabledStr = chainABNode.QueryParams(cltypes.ModuleName, string(cltypes.KeyIsPermisionlessPoolCreationEnabled)) if !strings.EqualFold(isPermisionlessCreationEnabledStr, "true") { @@ -869,6 +879,9 @@ func (s *IntegrationTestSuite) ConcentratedLiquidity() { // Check that the tick spacing was reduced to the expected new tick spacing concentratedPool = s.updatedConcentratedPool(chainABNode, poolID) s.Require().Equal(newTickSpacing, concentratedPool.GetTickSpacing()) + + // Reset the maximum number of pool points + chainABNode.SetMaxPoolPointsPerTx(int(protorevtypes.DefaultMaxPoolPointsPerTx), adminWalletAddr) } func (s *IntegrationTestSuite) StableSwapPostUpgrade() { diff --git a/x/protorev/keeper/keeper_test.go b/x/protorev/keeper/keeper_test.go index b0e73167292..2d21b6a2451 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("gamm/pool/1", sdk.NewInt(9000000000000000000)), sdk.NewCoin(apptesting.DefaultTransmuterDenomA, sdk.NewInt(9000000000000000000)), sdk.NewCoin(apptesting.DefaultTransmuterDenomB, sdk.NewInt(9000000000000000000)), + sdk.NewCoin("stake", sdk.NewInt(9000000000000000000)), ) s.fundAllAccountsWith() s.Commit() @@ -916,8 +917,93 @@ func (s *KeeperTestSuite) setUpPools() { } s.App.ProtoRevKeeper.SetInfoByPoolType(s.Ctx, poolInfo) - // Create a concentrated liquidity pool for range testing + // Create a duplicate pool for testing // Pool 52 + s.createGAMMPool( + []balancer.PoolAsset{ + { + Token: sdk.NewCoin("Atom", sdk.NewInt(10000)), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000)), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(2, 3), + sdk.NewDecWithPrec(0, 2), + ) + + // Create a duplicate pool for testing + // Pool 53 + s.createGAMMPool( + []balancer.PoolAsset{ + { + Token: sdk.NewCoin("usdc", sdk.NewInt(10000)), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000)), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(2, 3), + sdk.NewDecWithPrec(0, 2), + ) + + // Create a duplicate pool for testing + // Pool 54 + s.createGAMMPool( + []balancer.PoolAsset{ + { + Token: sdk.NewCoin("stake", sdk.NewInt(10000)), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(10000)), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(2, 3), + sdk.NewDecWithPrec(0, 2), + ) + + // Create a duplicate pool for testing + // Pool 55 + s.createGAMMPool( + []balancer.PoolAsset{ + { + Token: sdk.NewCoin("stake", sdk.NewInt(100000000)), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin(types.OsmosisDenomination, sdk.NewInt(1000000000)), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(2, 3), + sdk.NewDecWithPrec(0, 2), + ) + + // Create a duplicate pool for testing + // Pool 56 + s.createGAMMPool( + []balancer.PoolAsset{ + { + Token: sdk.NewCoin("bitcoin", sdk.NewInt(100)), + Weight: sdk.NewInt(1), + }, + { + Token: sdk.NewCoin("Atom", sdk.NewInt(100)), + Weight: sdk.NewInt(1), + }, + }, + sdk.NewDecWithPrec(2, 3), + sdk.NewDecWithPrec(0, 2), + ) + + // Create a concentrated liquidity pool for range testing + // Pool 58 // Create the CL pool clPool := s.PrepareCustomConcentratedPool(s.TestAccs[0], "epochTwo", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) fundCoins := sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(10_000_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(10_000_000_000_000))) @@ -925,7 +1011,7 @@ func (s *KeeperTestSuite) setUpPools() { s.CreateFullRangePosition(clPool, fundCoins) // Create a concentrated liquidity pool for range testing - // Pool 53 + // Pool 59 // Create the CL pool clPool = s.PrepareCustomConcentratedPool(s.TestAccs[0], "epochTwo", "uosmo", apptesting.DefaultTickSpacing, sdk.ZeroDec()) fundCoins = sdk.NewCoins(sdk.NewCoin("epochTwo", sdk.NewInt(2_000_000_000)), sdk.NewCoin("uosmo", sdk.NewInt(1_000_000_000))) diff --git a/x/protorev/keeper/posthandler_test.go b/x/protorev/keeper/posthandler_test.go index 784d7493a01..d552053f4d6 100644 --- a/x/protorev/keeper/posthandler_test.go +++ b/x/protorev/keeper/posthandler_test.go @@ -220,7 +220,7 @@ func (s *KeeperTestSuite) TestAnteHandle() { expectPass: true, }, { - name: "Two Pool Arb Route - Hot Route Build", + name: "Two Pool Arb Route", params: param{ trades: []types.Trade{ { @@ -235,16 +235,12 @@ func (s *KeeperTestSuite) TestAnteHandle() { Denom: "Atom", Amount: sdk.NewInt(15_767_231), }, - { - Denom: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", - Amount: sdk.NewInt(218_149_058), - }, { Denom: types.OsmosisDenomination, - Amount: sdk.NewInt(56_609_900), + Amount: sdk.NewInt(256_086_256), }, }, - expectedPoolPoints: 33, + expectedPoolPoints: 41, }, expectPass: true, }, @@ -264,16 +260,12 @@ func (s *KeeperTestSuite) TestAnteHandle() { Denom: "Atom", Amount: sdk.NewInt(15_767_231), }, - { - Denom: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", - Amount: sdk.NewInt(218_149_058), - }, { Denom: types.OsmosisDenomination, - Amount: sdk.NewInt(56_609_900), + Amount: sdk.NewInt(256_086_256), }, }, - expectedPoolPoints: 33, + expectedPoolPoints: 41, }, expectPass: true, }, @@ -293,16 +285,12 @@ func (s *KeeperTestSuite) TestAnteHandle() { Denom: "Atom", Amount: sdk.NewInt(15_767_231), }, - { - Denom: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", - Amount: sdk.NewInt(218_149_058), - }, { Denom: types.OsmosisDenomination, - Amount: sdk.NewInt(56_609_900), + Amount: sdk.NewInt(256_086_256), }, }, - expectedPoolPoints: 33, + expectedPoolPoints: 41, }, expectPass: true, }, @@ -322,16 +310,12 @@ func (s *KeeperTestSuite) TestAnteHandle() { Denom: "Atom", Amount: sdk.NewInt(15_767_231), }, - { - Denom: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7", - Amount: sdk.NewInt(218_149_058), - }, { Denom: types.OsmosisDenomination, - Amount: sdk.NewInt(56_609_900), + Amount: sdk.NewInt(256_086_256), }, }, - expectedPoolPoints: 33, + expectedPoolPoints: 41, }, expectPass: true, }, diff --git a/x/protorev/keeper/protorev.go b/x/protorev/keeper/protorev.go index 0a97320eeff..531dec98313 100644 --- a/x/protorev/keeper/protorev.go +++ b/x/protorev/keeper/protorev.go @@ -118,7 +118,7 @@ func (k Keeper) DeleteBaseDenoms(ctx sdk.Context) { k.DeleteAllEntriesForKeyPrefix(ctx, types.KeyPrefixBaseDenoms) } -// GetPoolForDenomPair returns the id of the highest liquidty pool between the base denom and the denom to match +// GetPoolForDenomPair returns the id of the highest liquidity pool between the base denom and the denom to match func (k Keeper) GetPoolForDenomPair(ctx sdk.Context, baseDenom, denomToMatch string) (uint64, error) { store := prefix.NewStore(ctx.KVStore(k.storeKey), types.KeyPrefixDenomPairToPool) key := types.GetKeyPrefixDenomPairToPool(baseDenom, denomToMatch) diff --git a/x/protorev/keeper/rebalance_test.go b/x/protorev/keeper/rebalance_test.go index c6753381326..6a26714ea74 100644 --- a/x/protorev/keeper/rebalance_test.go +++ b/x/protorev/keeper/rebalance_test.go @@ -182,11 +182,11 @@ var clPoolRouteExtended = poolmanagertypes.SwapAmountInRoutes{ // Tests multiple CL pools in the same route var clPoolRouteMulti = poolmanagertypes.SwapAmountInRoutes{ poolmanagertypes.SwapAmountInRoute{ - PoolId: 52, + PoolId: 57, TokenOutDenom: "uosmo", }, poolmanagertypes.SwapAmountInRoute{ - PoolId: 53, + PoolId: 58, TokenOutDenom: "epochTwo", }, } @@ -198,7 +198,7 @@ var clPoolRoute = poolmanagertypes.SwapAmountInRoutes{ TokenOutDenom: "uosmo", }, poolmanagertypes.SwapAmountInRoute{ - PoolId: 52, + PoolId: 57, TokenOutDenom: "epochTwo", }, } diff --git a/x/protorev/keeper/routes.go b/x/protorev/keeper/routes.go index d33ee42c900..c0f8c3c9fdc 100644 --- a/x/protorev/keeper/routes.go +++ b/x/protorev/keeper/routes.go @@ -102,6 +102,10 @@ func (k Keeper) BuildHighestLiquidityRoutes(ctx sdk.Context, tokenIn, tokenOut s if newRoute, err := k.BuildHighestLiquidityRoute(ctx, baseDenom, tokenIn, tokenOut, poolId); err == nil { routes = append(routes, newRoute) } + + if newRoute, err := k.BuildTwoPoolRoute(ctx, baseDenom, tokenIn, tokenOut, poolId); err == nil { + routes = append(routes, newRoute) + } } return routes, nil @@ -149,6 +153,70 @@ func (k Keeper) BuildHighestLiquidityRoute(ctx sdk.Context, swapDenom types.Base }, nil } +// BuildTwoPoolRoute will attempt to create a two pool route that will rebalance pools that are paired +// with the base denom. This is useful for pools that contain the same assets but are imbalanced. +func (k Keeper) BuildTwoPoolRoute( + ctx sdk.Context, + baseDenom types.BaseDenom, + tokenInDenom, tokenOutDenom string, + poolId uint64, +) (RouteMetaData, error) { + if baseDenom.Denom != tokenInDenom && baseDenom.Denom != tokenOutDenom { + return RouteMetaData{}, fmt.Errorf("base denom (%s) must be either tokenIn (%s) or tokenOut (%s)", baseDenom.Denom, tokenInDenom, tokenOutDenom) + } + + var ( + pool1, pool2 uint64 + ) + + // In the case where the base denom is the swap out, the base denom becomes more ~expensive~ on the current pool id + // and potentially cheaper on the highest liquidity pool. So we swap first on the current pool id and then on the + // highest liquidity pool. + if tokenOutDenom == baseDenom.Denom { + highestLiquidityPool, err := k.GetPoolForDenomPair(ctx, baseDenom.Denom, tokenInDenom) + if err != nil { + return RouteMetaData{}, err + } + + pool1, pool2 = poolId, highestLiquidityPool + tokenOutDenom = tokenInDenom + } else { + highestLiquidityPool, err := k.GetPoolForDenomPair(ctx, baseDenom.Denom, tokenOutDenom) + if err != nil { + return RouteMetaData{}, err + } + + pool1, pool2 = highestLiquidityPool, poolId + } + + if pool1 == pool2 { + return RouteMetaData{}, fmt.Errorf("cannot be trading on the same pool twice") + } + + newRoute := poolmanagertypes.SwapAmountInRoutes{ + poolmanagertypes.SwapAmountInRoute{ + TokenOutDenom: tokenOutDenom, + PoolId: pool1, + }, + poolmanagertypes.SwapAmountInRoute{ + TokenOutDenom: baseDenom.Denom, + PoolId: pool2, + }, + } + + // Check that the route is valid and update the number of pool points that this route will consume when simulating and executing trades + routePoolPoints, err := k.CalculateRoutePoolPoints(ctx, newRoute) + if err != nil { + return RouteMetaData{}, err + } + + return RouteMetaData{ + Route: newRoute, + PoolPoints: routePoolPoints, + StepSize: baseDenom.StepSize, + }, nil +} + // CalculateRoutePoolPoints calculates the number of pool points that will be consumed by a route when simulating and executing trades. This // is only added to the global pool point counter if the route simulated is minimally profitable i.e. it will make a profit. func (k Keeper) CalculateRoutePoolPoints(ctx sdk.Context, route poolmanagertypes.SwapAmountInRoutes) (uint64, error) { diff --git a/x/protorev/keeper/routes_test.go b/x/protorev/keeper/routes_test.go index 86c8e3a2833..6e7b9676720 100644 --- a/x/protorev/keeper/routes_test.go +++ b/x/protorev/keeper/routes_test.go @@ -44,13 +44,17 @@ func (s *KeeperTestSuite) TestBuildRoutes() { description: "Route exists for swap in Bitcoin and swap out Atom", inputDenom: "bitcoin", outputDenom: "Atom", - poolID: 4, + poolID: 55, expectedRoutes: [][]TestRoute{ { {PoolId: 25, InputDenom: types.OsmosisDenomination, OutputDenom: "Atom"}, - {PoolId: 4, InputDenom: "Atom", OutputDenom: "bitcoin"}, + {PoolId: 55, InputDenom: "Atom", OutputDenom: "bitcoin"}, {PoolId: 10, InputDenom: "bitcoin", OutputDenom: types.OsmosisDenomination}, }, + { + {PoolId: 55, InputDenom: "Atom", OutputDenom: "bitcoin"}, + {PoolId: 4, InputDenom: "bitcoin", OutputDenom: "Atom"}, + }, }, }, { @@ -91,6 +95,22 @@ func (s *KeeperTestSuite) TestBuildRoutes() { }, }, }, + { + description: "Two Pool Route exists for (osmo, atom)", + inputDenom: "Atom", + outputDenom: types.OsmosisDenomination, + poolID: 51, + expectedRoutes: [][]TestRoute{ + { + {PoolId: 51, InputDenom: types.OsmosisDenomination, OutputDenom: "Atom"}, + {PoolId: 25, InputDenom: "Atom", OutputDenom: types.OsmosisDenomination}, + }, + { + {PoolId: 25, InputDenom: "Atom", OutputDenom: types.OsmosisDenomination}, + {PoolId: 51, InputDenom: types.OsmosisDenomination, OutputDenom: "Atom"}, + }, + }, + }, } for _, tc := range cases { @@ -101,6 +121,7 @@ func (s *KeeperTestSuite) TestBuildRoutes() { for routeIndex, route := range routes { for tradeIndex, poolID := range route.Route.PoolIds() { s.Require().Equal(tc.expectedRoutes[routeIndex][tradeIndex].PoolId, poolID) + s.Require().Equal(tc.expectedRoutes[routeIndex][tradeIndex].OutputDenom, route.Route[tradeIndex].TokenOutDenom) } } }) @@ -221,6 +242,99 @@ func (s *KeeperTestSuite) TestBuildHighestLiquidityRoute() { } } +// TestBuildTwoPoolRoute tests the BuildTwoPoolRoute function +func (s *KeeperTestSuite) TestBuildTwoPoolRoute() { + cases := []struct { + description string + swapDenom types.BaseDenom + tokenIn string + tokenOut string + poolId uint64 + expectedRoute []TestRoute + hasRoute bool + }{ + { + description: "two pool route can be created with base as token out", + swapDenom: types.BaseDenom{ + Denom: types.OsmosisDenomination, + StepSize: sdk.NewInt(1_000_000), + }, + tokenIn: "stake", + tokenOut: types.OsmosisDenomination, + poolId: 54, + expectedRoute: []TestRoute{ + {PoolId: 54, InputDenom: types.OsmosisDenomination, OutputDenom: "stake"}, + {PoolId: 55, InputDenom: "stake", OutputDenom: types.OsmosisDenomination}, + }, + hasRoute: true, + }, + { + description: "two pool route can be created with base as token in", + swapDenom: types.BaseDenom{ + Denom: types.OsmosisDenomination, + StepSize: sdk.NewInt(1_000_000), + }, + tokenIn: types.OsmosisDenomination, + tokenOut: "stake", + poolId: 54, + expectedRoute: []TestRoute{ + {PoolId: 55, InputDenom: types.OsmosisDenomination, OutputDenom: "stake"}, + {PoolId: 54, InputDenom: "stake", OutputDenom: types.OsmosisDenomination}, + }, + hasRoute: true, + }, + { + description: "two pool route where swap is on the highest liquidity pool", + swapDenom: types.BaseDenom{ + Denom: types.OsmosisDenomination, + StepSize: sdk.NewInt(1_000_000), + }, + tokenIn: "stake", + tokenOut: types.OsmosisDenomination, + poolId: 55, + expectedRoute: []TestRoute{}, + hasRoute: false, + }, + { + description: "trade executes on pool not tracked by the module", + swapDenom: types.BaseDenom{ + Denom: types.OsmosisDenomination, + StepSize: sdk.NewInt(1_000_000), + }, + tokenIn: "stake", + tokenOut: types.OsmosisDenomination, + poolId: 1000000, + expectedRoute: []TestRoute{}, + hasRoute: false, + }, + } + + for _, tc := range cases { + s.Run(tc.description, func() { + routeMetaData, err := s.App.ProtoRevKeeper.BuildTwoPoolRoute( + s.Ctx, + tc.swapDenom, + tc.tokenIn, + tc.tokenOut, + tc.poolId, + ) + + if tc.hasRoute { + s.Require().NoError(err) + s.Require().Equal(len(tc.expectedRoute), len(routeMetaData.Route.PoolIds())) + + for index, trade := range tc.expectedRoute { + s.Require().Equal(trade.PoolId, routeMetaData.Route[index].PoolId) + s.Require().Equal(trade.OutputDenom, routeMetaData.Route[index].TokenOutDenom) + } + } else { + s.Require().Equal(len(tc.expectedRoute), len(routeMetaData.Route.PoolIds())) + s.Require().Error(err) + } + }) + } +} + // TestBuildHotRoutes tests the BuildHotRoutes function func (s *KeeperTestSuite) TestBuildHotRoutes() { cases := []struct {