Skip to content

Commit

Permalink
feat(ProtoRev): Supporting Two Pool Routes (#5953)
Browse files Browse the repository at this point in the history
* testing

* changelog

* trading both ways

* updating test

* nit

* test fix

* protorev test update

* Update tests/e2e/configurer/chain/commands.go

---------

Co-authored-by: devbot-wizard <[email protected]>
Co-authored-by: Adam Tucker <[email protected]>
  • Loading branch information
3 people authored Aug 16, 2023
1 parent 32b7a72 commit 12fca5b
Show file tree
Hide file tree
Showing 9 changed files with 308 additions and 35 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,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
Expand Down
7 changes: 7 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
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
// 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

0 comments on commit 12fca5b

Please sign in to comment.