Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ProtoRev): Supporting Two Pool Routes #5953

Merged
merged 14 commits into from
Aug 16, 2023
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ 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
* [#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
* [#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

### Minor improvements & Bug Fixes

Expand Down
8 changes: 8 additions & 0 deletions tests/e2e/configurer/chain/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down Expand Up @@ -770,6 +777,7 @@ func (n *NodeConfig) ParamChangeProposal(subspace, key string, value []byte, cha
Deposit: "625000000uosmo",
}
proposalJson, err := json.Marshal(proposal)
fmt.Println("marshalling output", proposalJson, err)
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}
Expand Down
13 changes: 13 additions & 0 deletions tests/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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") {
Expand Down Expand Up @@ -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() {
Expand Down
90 changes: 88 additions & 2 deletions x/protorev/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -916,16 +917,101 @@ 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add in the comment what test these pools are for

// 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)))
s.FundAcc(s.TestAccs[0], fundCoins)
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)))
Expand Down
34 changes: 9 additions & 25 deletions x/protorev/keeper/posthandler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand All @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion x/protorev/keeper/protorev.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions x/protorev/keeper/rebalance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
}
Expand All @@ -198,7 +198,7 @@ var clPoolRoute = poolmanagertypes.SwapAmountInRoutes{
TokenOutDenom: "uosmo",
},
poolmanagertypes.SwapAmountInRoute{
PoolId: 52,
PoolId: 57,
TokenOutDenom: "epochTwo",
},
}
Expand Down
68 changes: 68 additions & 0 deletions x/protorev/keeper/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
Loading