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.

Sign up for GitHub

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
Merged

feat(ProtoRev): Supporting Two Pool Routes #5953

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
16ef254
testing
davidterpay Aug 3, 2023
a65020a
changelog
davidterpay Aug 3, 2023
c788e3d
Merge branch 'main' into terpay/two-pool-routes
davidterpay Aug 3, 2023
0d8b036
trading both ways
davidterpay Aug 3, 2023
c8a9dc3
Merge branch 'main' into terpay/two-pool-routes
davidterpay Aug 7, 2023
5213b40
updating test
davidterpay Aug 7, 2023
5e12273
nit
davidterpay Aug 7, 2023
3508497
Merge branch 'main' into terpay/two-pool-routes
davidterpay Aug 7, 2023
fd0424f
Merge branch 'main' into terpay/two-pool-routes
devbot-wizard Aug 8, 2023
92e2979
test fix
davidterpay Aug 8, 2023
f8b018d
protorev test update
davidterpay Aug 9, 2023
8f3113b
Merge branch 'main' into terpay/two-pool-routes
davidterpay Aug 9, 2023
b2c792e
Update tests/e2e/configurer/chain/commands.go
czarcas7ic Aug 10, 2023
476d82d
Merge branch 'main' into terpay/two-pool-routes
davidterpay Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* [#5923] (https://github.com/osmosis-labs/osmosis/pull/5923) CL: Lower gas for initializing ticks
* [#5927] (https://github.com/osmosis-labs/osmosis/pull/5927) Add gas metering to x/tokenfactory trackBeforeSend hook
* [#5890](https://github.com/osmosis-labs/osmosis/pull/5890) feat: CreateCLPool & LinkCFMMtoCL pool into one gov-prop
* [#5953] (https://github.com/osmosis-labs/osmosis/pull/5953) Supporting two pool routes in ProtoRev

### Minor improvements & Bug Fixes

Expand Down
86 changes: 86 additions & 0 deletions x/protorev/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,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 @@ -902,6 +903,91 @@ func (s *KeeperTestSuite) setUpPools() {
// Pool 50
s.PrepareCosmWasmPool()

// 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 51
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 52
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 53
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 54
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 55
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),
)

// Set all of the pool info into the stores
err := s.App.ProtoRevKeeper.UpdatePools(s.Ctx)
s.Require().NoError(err)
Expand Down
44 changes: 12 additions & 32 deletions x/protorev/keeper/posthandler_test.go
View file
Open in desktop
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 All @@ -340,7 +324,7 @@ func (s *KeeperTestSuite) TestAnteHandle() {
params: param{
trades: []types.Trade{
{
Pool: 51,
Pool: 56,
TokenOut: "ibc/0CD3A0285E1341859B5E86B6AB7682F023D03E97607CCC1DC95706411D866DF7",
TokenIn: "uosmo",
},
Expand All @@ -351,16 +335,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: 37,
expectedPoolPoints: 45,
},
expectPass: true,
},
Expand Down
2 changes: 1 addition & 1 deletion x/protorev/keeper/protorev.go
View file
Open in desktop
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
70 changes: 70 additions & 0 deletions x/protorev/keeper/routes.go
View file
Open in desktop
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,72 @@ 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,
swapIn, swapOut string,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: or SwapInDenom, SwapOutDenom

Suggested change
swapIn, swapOut string,
tokenInDenom, tokenOutDenom string,

poolId uint64,
) (RouteMetaData, error) {
if baseDenom.Denom != swapIn && baseDenom.Denom != swapOut {
return RouteMetaData{}, fmt.Errorf("base denom (%s) must be either tokenIn (%s) or tokenOut (%s)", baseDenom.Denom, swapIn, swapOut)
}

var (
tokenOutDenom string
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 swapOut == baseDenom.Denom {
highestLiquidityPool, err := k.GetPoolForDenomPair(ctx, baseDenom.Denom, swapIn)
if err != nil {
return RouteMetaData{}, err
}

pool1, pool2 = poolId, highestLiquidityPool
tokenOutDenom = swapIn
} else {
highestLiquidityPool, err := k.GetPoolForDenomPair(ctx, baseDenom.Denom, swapOut)
if err != nil {
return RouteMetaData{}, err
}

pool1, pool2 = highestLiquidityPool, poolId
tokenOutDenom = swapOut
}

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