From 557fac4db3fd81459c02c9993e70585cb4403edf Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Mon, 11 Jul 2022 00:27:56 -0700 Subject: [PATCH 1/9] add gas for swap + test --- x/gamm/keeper/swap.go | 2 ++ x/gamm/keeper/swap_test.go | 8 ++++++++ x/gamm/types/constants.go | 2 ++ 3 files changed, 12 insertions(+) diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go index ec3f6f71acb..2f72b6ab8f4 100644 --- a/x/gamm/keeper/swap.go +++ b/x/gamm/keeper/swap.go @@ -49,6 +49,7 @@ func (k Keeper) swapExactAmountIn( } tokensIn := sdk.Coins{tokenIn} + ctx.GasMeter().ConsumeGas(types.GasFeeForSwap, "swap computation") // Executes the swap in the pool and stores the output. Updates pool assets but // does not actually transfer any tokens to or from the pool. tokenOutCoin, err := pool.SwapOutAmtGivenIn(ctx, tokensIn, tokenOutDenom, swapFee) @@ -113,6 +114,7 @@ func (k Keeper) swapExactAmountOut( return sdk.Int{}, sdkerrors.Wrapf(types.ErrTooManyTokensOut, "can't get more tokens out than there are tokens in the pool") } + ctx.GasMeter().ConsumeGas(types.GasFeeForSwap, "swap computation") tokenIn, err := pool.SwapInAmtGivenOut(ctx, sdk.Coins{tokenOut}, tokenInDenom, swapFee) if err != nil { return sdk.Int{}, err diff --git a/x/gamm/keeper/swap_test.go b/x/gamm/keeper/swap_test.go index 4b4f9beb1f3..2b3096ef602 100644 --- a/x/gamm/keeper/swap_test.go +++ b/x/gamm/keeper/swap_test.go @@ -89,9 +89,13 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountIn() { spotPriceBefore, err := keeper.CalculateSpotPrice(suite.Ctx, poolId, test.param.tokenIn.Denom, test.param.tokenOutDenom) suite.NoError(err, "test: %v", test.name) + prevGasConsumed := suite.Ctx.GasMeter().GasConsumed() tokenOutAmount, err := keeper.SwapExactAmountIn(suite.Ctx, suite.TestAccs[0], poolId, test.param.tokenIn, test.param.tokenOutDenom, test.param.tokenOutMinAmount) suite.NoError(err, "test: %v", test.name) suite.True(tokenOutAmount.Equal(test.param.expectedTokenOut), "test: %v", test.name) + gasConsumedForSwap := suite.Ctx.GasMeter().GasConsumed() - prevGasConsumed + // We consume 10,000 gas directly, so the extra I/O operation mean we end up consuming more. + suite.Assert().Greater(gasConsumedForSwap, uint64(10000)) spotPriceAfter, err := keeper.CalculateSpotPrice(suite.Ctx, poolId, test.param.tokenIn.Denom, test.param.tokenOutDenom) suite.NoError(err, "test: %v", test.name) @@ -188,11 +192,15 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountOut() { spotPriceBefore, err := keeper.CalculateSpotPrice(suite.Ctx, poolId, test.param.tokenInDenom, test.param.tokenOut.Denom) suite.NoError(err, "test: %v", test.name) + prevGasConsumed := suite.Ctx.GasMeter().GasConsumed() tokenInAmount, err := keeper.SwapExactAmountOut(suite.Ctx, suite.TestAccs[0], poolId, test.param.tokenInDenom, test.param.tokenInMaxAmount, test.param.tokenOut) suite.NoError(err, "test: %v", test.name) suite.True(tokenInAmount.Equal(test.param.expectedTokenInAmount), "test: %v\n expect_eq actual: %s, expected: %s", test.name, tokenInAmount, test.param.expectedTokenInAmount) + gasConsumedForSwap := suite.Ctx.GasMeter().GasConsumed() - prevGasConsumed + // We consume 10,000 gas directly, so the extra I/O operation mean we end up consuming more. + suite.Assert().Greater(gasConsumedForSwap, uint64(10000)) spotPriceAfter, err := keeper.CalculateSpotPrice(suite.Ctx, poolId, test.param.tokenInDenom, test.param.tokenOut.Denom) suite.NoError(err, "test: %v", test.name) diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go index acdde3c66e2..1b6482b1c60 100644 --- a/x/gamm/types/constants.go +++ b/x/gamm/types/constants.go @@ -12,6 +12,8 @@ const ( // Raise 10 to the power of SigFigsExponent to determine number of significant figures. // i.e. SigFigExponent = 8 is 10^8 which is 100000000. This gives 8 significant figures. SigFigsExponent = 8 + // TODO: Current fixed cost gas fee per swap -- turn this into a param in the future. + GasFeeForSwap = 10000 ) var ( From 88e61129d6fba5b6e92b2c233aedfc0b0983c288 Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Mon, 11 Jul 2022 23:45:59 -0400 Subject: [PATCH 2/9] test file comments --- x/gamm/keeper/swap_test.go | 5 +++-- x/gamm/types/constants.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/x/gamm/keeper/swap_test.go b/x/gamm/keeper/swap_test.go index 2b3096ef602..f2752f8f956 100644 --- a/x/gamm/keeper/swap_test.go +++ b/x/gamm/keeper/swap_test.go @@ -6,6 +6,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/osmosis-labs/osmosis/v7/x/gamm/pool-models/balancer" + "github.com/osmosis-labs/osmosis/v7/x/gamm/types" ) func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountIn() { @@ -94,8 +95,8 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountIn() { suite.NoError(err, "test: %v", test.name) suite.True(tokenOutAmount.Equal(test.param.expectedTokenOut), "test: %v", test.name) gasConsumedForSwap := suite.Ctx.GasMeter().GasConsumed() - prevGasConsumed - // We consume 10,000 gas directly, so the extra I/O operation mean we end up consuming more. - suite.Assert().Greater(gasConsumedForSwap, uint64(10000)) + // We consume `types.GasFeeForSwap` directly, so the extra I/O operation mean we end up consuming more. + suite.Assert().Greater(gasConsumedForSwap, uint64(types.GasFeeForSwap)) spotPriceAfter, err := keeper.CalculateSpotPrice(suite.Ctx, poolId, test.param.tokenIn.Denom, test.param.tokenOutDenom) suite.NoError(err, "test: %v", test.name) diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go index 1b6482b1c60..a794b0442f7 100644 --- a/x/gamm/types/constants.go +++ b/x/gamm/types/constants.go @@ -12,7 +12,7 @@ const ( // Raise 10 to the power of SigFigsExponent to determine number of significant figures. // i.e. SigFigExponent = 8 is 10^8 which is 100000000. This gives 8 significant figures. SigFigsExponent = 8 - // TODO: Current fixed cost gas fee per swap -- turn this into a param in the future. + // TODO: Turn this into a param in the future. GasFeeForSwap = 10000 ) From 4bdfc68dfbac1254090a81a72f301a34243240f8 Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Mon, 11 Jul 2022 23:53:59 -0400 Subject: [PATCH 3/9] move swap consumption to balancer only --- x/gamm/keeper/swap.go | 3 +-- x/gamm/keeper/swap_test.go | 2 +- x/gamm/pool-models/balancer/pool.go | 2 ++ x/gamm/types/constants.go | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/x/gamm/keeper/swap.go b/x/gamm/keeper/swap.go index 2f72b6ab8f4..3d69ac7b577 100644 --- a/x/gamm/keeper/swap.go +++ b/x/gamm/keeper/swap.go @@ -49,7 +49,6 @@ func (k Keeper) swapExactAmountIn( } tokensIn := sdk.Coins{tokenIn} - ctx.GasMeter().ConsumeGas(types.GasFeeForSwap, "swap computation") // Executes the swap in the pool and stores the output. Updates pool assets but // does not actually transfer any tokens to or from the pool. tokenOutCoin, err := pool.SwapOutAmtGivenIn(ctx, tokensIn, tokenOutDenom, swapFee) @@ -114,7 +113,7 @@ func (k Keeper) swapExactAmountOut( return sdk.Int{}, sdkerrors.Wrapf(types.ErrTooManyTokensOut, "can't get more tokens out than there are tokens in the pool") } - ctx.GasMeter().ConsumeGas(types.GasFeeForSwap, "swap computation") + tokenIn, err := pool.SwapInAmtGivenOut(ctx, sdk.Coins{tokenOut}, tokenInDenom, swapFee) if err != nil { return sdk.Int{}, err diff --git a/x/gamm/keeper/swap_test.go b/x/gamm/keeper/swap_test.go index f2752f8f956..d03a706cca5 100644 --- a/x/gamm/keeper/swap_test.go +++ b/x/gamm/keeper/swap_test.go @@ -96,7 +96,7 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountIn() { suite.True(tokenOutAmount.Equal(test.param.expectedTokenOut), "test: %v", test.name) gasConsumedForSwap := suite.Ctx.GasMeter().GasConsumed() - prevGasConsumed // We consume `types.GasFeeForSwap` directly, so the extra I/O operation mean we end up consuming more. - suite.Assert().Greater(gasConsumedForSwap, uint64(types.GasFeeForSwap)) + suite.Assert().Greater(gasConsumedForSwap, uint64(types.BalancerGasFeeForSwap)) spotPriceAfter, err := keeper.CalculateSpotPrice(suite.Ctx, poolId, test.param.tokenIn.Denom, test.param.tokenOutDenom) suite.NoError(err, "test: %v", test.name) diff --git a/x/gamm/pool-models/balancer/pool.go b/x/gamm/pool-models/balancer/pool.go index 2a34b982b48..4169067aa0e 100644 --- a/x/gamm/pool-models/balancer/pool.go +++ b/x/gamm/pool-models/balancer/pool.go @@ -584,6 +584,8 @@ func (p *Pool) SwapInAmtGivenOut( // ApplySwap. func (p *Pool) applySwap(ctx sdk.Context, tokensIn sdk.Coins, tokensOut sdk.Coins) error { + // Fixed gas consumption per swap to prevent spam + ctx.GasMeter().ConsumeGas(types.BalancerGasFeeForSwap, "balancer swap computation") // Also ensures that len(tokensIn) = 1 = len(tokensOut) inPoolAsset, outPoolAsset, err := p.parsePoolAssetsCoins(tokensIn, tokensOut) if err != nil { diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go index a794b0442f7..e503eca4f10 100644 --- a/x/gamm/types/constants.go +++ b/x/gamm/types/constants.go @@ -13,7 +13,7 @@ const ( // i.e. SigFigExponent = 8 is 10^8 which is 100000000. This gives 8 significant figures. SigFigsExponent = 8 // TODO: Turn this into a param in the future. - GasFeeForSwap = 10000 + BalancerGasFeeForSwap = 10000 ) var ( From 44abd66eb9394ba32396aca5f1da3d0686753b20 Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Mon, 11 Jul 2022 23:58:32 -0400 Subject: [PATCH 4/9] fix one more test case --- x/gamm/keeper/swap_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/gamm/keeper/swap_test.go b/x/gamm/keeper/swap_test.go index d03a706cca5..40e42c4bf7c 100644 --- a/x/gamm/keeper/swap_test.go +++ b/x/gamm/keeper/swap_test.go @@ -200,8 +200,8 @@ func (suite *KeeperTestSuite) TestBalancerPoolSimpleSwapExactAmountOut() { "test: %v\n expect_eq actual: %s, expected: %s", test.name, tokenInAmount, test.param.expectedTokenInAmount) gasConsumedForSwap := suite.Ctx.GasMeter().GasConsumed() - prevGasConsumed - // We consume 10,000 gas directly, so the extra I/O operation mean we end up consuming more. - suite.Assert().Greater(gasConsumedForSwap, uint64(10000)) + // We consume `types.GasFeeForSwap` directly, so the extra I/O operation mean we end up consuming more. + suite.Assert().Greater(gasConsumedForSwap, uint64(types.BalancerGasFeeForSwap)) spotPriceAfter, err := keeper.CalculateSpotPrice(suite.Ctx, poolId, test.param.tokenInDenom, test.param.tokenOut.Denom) suite.NoError(err, "test: %v", test.name) From 7eed0c3252401b8fd667cc6a2cdb1be2b4ba9a40 Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Wed, 13 Jul 2022 01:42:10 -0400 Subject: [PATCH 5/9] add to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b7e31401dab..40943588ca4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +* [#2016](https://github.com/osmosis-labs/osmosis/pull/2016) Add fixed 10000 gas cost for each Balancer swap ### Breaking Changes From 51909243aa5ab50980329a5d70277d7c55170c79 Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Thu, 14 Jul 2022 00:29:32 -0400 Subject: [PATCH 6/9] Update CHANGELOG.md Co-authored-by: Aleksandr Bezobchuk --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40943588ca4..d2ec2eea37b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased + * [#2016](https://github.com/osmosis-labs/osmosis/pull/2016) Add fixed 10000 gas cost for each Balancer swap ### Breaking Changes From df0f26bf86a9a0153bb79cd213748087704ab95d Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Thu, 14 Jul 2022 01:26:51 -0400 Subject: [PATCH 7/9] rm TODO now that we have issue --- x/gamm/types/constants.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go index e503eca4f10..b26dc8e027d 100644 --- a/x/gamm/types/constants.go +++ b/x/gamm/types/constants.go @@ -12,7 +12,6 @@ const ( // Raise 10 to the power of SigFigsExponent to determine number of significant figures. // i.e. SigFigExponent = 8 is 10^8 which is 100000000. This gives 8 significant figures. SigFigsExponent = 8 - // TODO: Turn this into a param in the future. BalancerGasFeeForSwap = 10000 ) From 5ea427fac94e4a4b75cd9481e9bf1dafa9049f61 Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Thu, 14 Jul 2022 01:27:14 -0400 Subject: [PATCH 8/9] underscore for clarity --- x/gamm/types/constants.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go index b26dc8e027d..6cf8d4cfbef 100644 --- a/x/gamm/types/constants.go +++ b/x/gamm/types/constants.go @@ -12,7 +12,7 @@ const ( // Raise 10 to the power of SigFigsExponent to determine number of significant figures. // i.e. SigFigExponent = 8 is 10^8 which is 100000000. This gives 8 significant figures. SigFigsExponent = 8 - BalancerGasFeeForSwap = 10000 + BalancerGasFeeForSwap = 10_000 ) var ( From 29ad32af7572aaf1d10d8fc6e8058c5cdd6a23de Mon Sep 17 00:00:00 2001 From: Richard Liu Date: Thu, 14 Jul 2022 01:39:52 -0400 Subject: [PATCH 9/9] gofmt --- x/gamm/types/constants.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/gamm/types/constants.go b/x/gamm/types/constants.go index 6cf8d4cfbef..4043af1c55b 100644 --- a/x/gamm/types/constants.go +++ b/x/gamm/types/constants.go @@ -11,7 +11,7 @@ const ( OneShareExponent = 18 // Raise 10 to the power of SigFigsExponent to determine number of significant figures. // i.e. SigFigExponent = 8 is 10^8 which is 100000000. This gives 8 significant figures. - SigFigsExponent = 8 + SigFigsExponent = 8 BalancerGasFeeForSwap = 10_000 )