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

Cleanup/Optimize gas for swaps #5850

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
279 changes: 6 additions & 273 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,281 +2,14 @@
"version": "0.2.0",
"configurations": [
{
"name": "E2E: (make test-e2e-short)",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/tests/e2e",
"args": [
"-test.timeout",
"30m",
"-test.run",
"IntegrationTestSuite",
"-test.v"
],
"buildFlags": "-tags e2e",
"env": {
"OSMOSIS_E2E": "True",
"OSMOSIS_E2E_SKIP_IBC": "true",
"OSMOSIS_E2E_SKIP_UPGRADE": "true",
"OSMOSIS_E2E_SKIP_CLEANUP": "true",
"OSMOSIS_E2E_SKIP_STATE_SYNC": "true",
"OSMOSIS_E2E_UPGRADE_VERSION": "v17",
"OSMOSIS_E2E_DEBUG_LOG": "false",
},
"preLaunchTask": "e2e-setup"
},
{
"name": "x/concentrated-liquidity",
"name": "Debug Unit Test",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/concentrated-liquidity",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/cosmwasmpool",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/cosmwasmpool",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/downtime-detector",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/downtime-detector",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/epochs",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/epochs",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/gamm",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/gamm",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/ibc-hooks",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/ibc-hooks",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/ibc-rate-limit",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/ibc-rate-limit",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/incentives",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/incentives",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/lockup",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/lockup",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/mint",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/mint",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/pool-incentives",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/pool-incentives",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/poolmanager",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/poolmanager",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/protorev",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/protorev",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/superfluid",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/superfluid",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/tokenfactory",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/tokenfactory",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/twap",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/twap",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/txfees",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/txfees",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
{
"name": "x/valset-pref",
"type": "go",
"request": "launch",
"mode": "test",
"program": "${workspaceFolder}/x/valset-pref",
"args": [
"-test.timeout",
"30m",
"-test.run",
"TestKeeperTestSuite/TestYourName",
"-test.v"
],
},
"args": ["-test.run", "TestKeeperTestSuite/TestComputeOutAmtGivenIn"],
"env": {},
"showLog": true
}
]
}
}
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

- [#5850] (https://github.com/osmosis-labs/osmosis/pull/5850) Remove GetPool from computeSwap and updateSwap to optimize gas
stackman27 marked this conversation as resolved.
Show resolved Hide resolved

### State Breaking

* [#5532](https://github.com/osmosis-labs/osmosis/pull/5532) fix: Fix x/tokenfactory genesis import denoms reset x/bank existing denom metadata
Expand Down
8 changes: 4 additions & 4 deletions x/concentrated-liquidity/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ func (k Keeper) SwapOutAmtGivenIn(

func (k Keeper) ComputeOutAmtGivenIn(
ctx sdk.Context,
poolId uint64,
pool types.ConcentratedPoolExtension,
tokenInMin sdk.Coin,
tokenOutDenom string,
spreadFactor sdk.Dec,
priceLimit sdk.Dec,

) (calcTokenIn, calcTokenOut sdk.Coin, poolUpdates PoolUpdates, totalSpreadRewards sdk.Dec, err error) {
return k.computeOutAmtGivenIn(ctx, poolId, tokenInMin, tokenOutDenom, spreadFactor, priceLimit)
return k.computeOutAmtGivenIn(ctx, pool, tokenInMin, tokenOutDenom, spreadFactor, priceLimit)
}

func (k Keeper) SwapInAmtGivenOut(
Expand All @@ -88,14 +88,14 @@ func (k Keeper) SwapInAmtGivenOut(

func (k Keeper) ComputeInAmtGivenOut(
ctx sdk.Context,
pool types.ConcentratedPoolExtension,
desiredTokenOut sdk.Coin,
tokenInDenom string,
spreadFactor sdk.Dec,
priceLimit sdk.Dec,
poolId uint64,

) (calcTokenIn, calcTokenOut sdk.Coin, poolUpdates PoolUpdates, totalSpreadRewards sdk.Dec, err error) {
return k.computeInAmtGivenOut(ctx, desiredTokenOut, tokenInDenom, spreadFactor, priceLimit, poolId)
return k.computeInAmtGivenOut(ctx, pool, desiredTokenOut, tokenInDenom, spreadFactor, priceLimit)
}

func (k Keeper) InitOrUpdateTick(ctx sdk.Context, poolId uint64, currentTick int64, tickIndex int64, liquidityIn sdk.Dec, upper bool) (err error) {
Expand Down
6 changes: 6 additions & 0 deletions x/concentrated-liquidity/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ func (s *KeeperTestSuite) swap(pool types.ConcentratedPoolExtension, swapInFunde
// // Execute swap
fmt.Printf("swap in: %s\n", swapInFunded)
cacheCtx, writeOutGivenIn := s.Ctx.CacheContext()
pool, err := s.clk.GetPoolById(s.Ctx, pool.GetId())
s.Require().NoError(err)
Comment on lines +281 to +282
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed in place of the pool object that is provided in the args? No state change occurs prior to this call on the pool obj as far as I see.

Copy link
Contributor Author

@stackman27 stackman27 Jul 25, 2023

Choose a reason for hiding this comment

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

So pool obj change when we create first position. It updates the current Sqr Price https://github.com/osmosis-labs/osmosis/blob/main/x/concentrated-liquidity/lp.go#L476


_, tokenOut, _, err := s.clk.SwapOutAmtGivenIn(cacheCtx, s.TestAccs[0], pool, swapInFunded, swapOutDenom, pool.GetSpreadFactor(s.Ctx), sdk.ZeroDec())
if errors.As(err, &types.InvalidAmountCalculatedError{}) {
// If the swap we're about to execute will not generate enough output, we skip the swap.
Expand All @@ -297,6 +300,9 @@ func (s *KeeperTestSuite) swap(pool types.ConcentratedPoolExtension, swapInFunde
// We expect the returned amountIn to be roughly equal to the original swapInFunded.
cacheCtx, _ = s.Ctx.CacheContext()
fmt.Printf("swap out: %s\n", tokenOut)
pool, err = s.clk.GetPoolById(s.Ctx, pool.GetId())
s.Require().NoError(err)
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved

amountInSwapResult, _, _, err := s.clk.SwapInAmtGivenOut(cacheCtx, s.TestAccs[0], pool, tokenOut, swapInFunded.Denom, pool.GetSpreadFactor(s.Ctx), sdk.ZeroDec())
if errors.As(err, &types.InvalidAmountCalculatedError{}) {
// If the swap we're about to execute will not generate enough output, we skip the swap.
Expand Down
4 changes: 3 additions & 1 deletion x/concentrated-liquidity/position_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2428,7 +2428,9 @@ func (s *KeeperTestSuite) TestTickRoundingEdgeCase() {
swapAddr := testAccs[2]
desiredTokenOut := sdk.NewCoin(USDC, sdk.NewInt(10000))
s.FundAcc(swapAddr, sdk.NewCoins(sdk.NewCoin(ETH, sdk.NewInt(1000000000000000000))))
_, _, _, err := s.clk.SwapInAmtGivenOut(s.Ctx, swapAddr, pool, desiredTokenOut, ETH, sdk.ZeroDec(), sdk.ZeroDec())
pool, err := s.clk.GetPoolById(s.Ctx, pool.GetId())
s.Require().NoError(err)
czarcas7ic marked this conversation as resolved.
Show resolved Hide resolved
_, _, _, err = s.clk.SwapInAmtGivenOut(s.Ctx, swapAddr, pool, desiredTokenOut, ETH, sdk.ZeroDec(), sdk.ZeroDec())
s.Require().NoError(err)

// Both positions should be able to withdraw successfully
Expand Down
3 changes: 3 additions & 0 deletions x/concentrated-liquidity/range_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,9 @@ func (s *KeeperTestSuite) executeRandomizedSwap(pool types.ConcentratedPoolExten
}
}

pool, err := s.clk.GetPoolById(s.Ctx, pool.GetId())
s.Require().NoError(err)

// Note that we set the price limit to zero to ensure that the swap can execute in either direction (gets automatically set to correct limit)
swappedIn, swappedOut, _, err := s.clk.SwapInAmtGivenOut(s.Ctx, swapAddress, pool, swapOutCoin, swapInDenom, pool.GetSpreadFactor(s.Ctx), sdk.ZeroDec())
s.Require().NoError(err)
Expand Down
Loading