From 4bc2902316b04047367a1b26fa4506e99680b976 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 21 Nov 2023 00:08:12 -0500 Subject: [PATCH 1/4] refactor(poolmanager): GetPoolDenoms method on PoolI for SQS (#6863) * refactor(poolmanager): GetPoolDenoms method on PoolI for SQS * changelog * mocks * change proto dep * transmuter pool * cfmm test * CL test (cherry picked from commit 4c54425c55f246d25f996dcb9be147c750c44ab6) # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 11 ++++++ tests/mocks/cfmm_pool.go | 42 +++++++++++++++++++++ tests/mocks/cl_pool.go | 14 +++++++ tests/mocks/pool.go | 14 +++++++ x/concentrated-liquidity/model/pool.go | 8 ++++ x/concentrated-liquidity/model/pool_test.go | 17 +++++++++ x/cosmwasmpool/model/pool.go | 7 ++++ x/cosmwasmpool/model/pool_test.go | 7 ++++ x/cosmwasmpool/model/store_model.go | 5 +++ x/gamm/pool-models/balancer/pool.go | 7 ++++ x/gamm/pool-models/balancer/pool_test.go | 20 +++++++++- x/gamm/pool-models/stableswap/pool.go | 7 ++++ x/poolmanager/types/pool.go | 3 ++ 13 files changed, 161 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 649fffc5eb0..3fdc53f0bd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### API Breaks * [#6805](https://github.com/osmosis-labs/osmosis/pull/6805) return bucket index of the current tick from LiquidityPerTickRange query +<<<<<<< HEAD +======= +* [#6530](https://github.com/osmosis-labs/osmosis/pull/6530) Improve error message when CL LP fails due to slippage bound hit. +* [#6863](https://github.com/osmosis-labs/osmosis/pull/6863) GetPoolDenoms method on PoolI interface in poolmanager + +### Bug Fixes + +* [#6840](https://github.com/osmosis-labs/osmosis/pull/6840) fix: change TypeMsgUnbondConvertAndStake value to "unbond_convert_and_stake" and improve error message when epoch currentEpochStartHeight less than zero +* [#6769](https://github.com/osmosis-labs/osmosis/pull/6769) fix: improve dust handling in EstimateTradeBasedOnPriceImpact +* [#6841](https://github.com/osmosis-labs/osmosis/pull/6841) fix: fix receive_ack response field and imporove error message of InvalidCrosschainSwapsContract and NoDenomTrace +>>>>>>> 4c54425c (refactor(poolmanager): GetPoolDenoms method on PoolI for SQS (#6863)) ## v20.0.0 diff --git a/tests/mocks/cfmm_pool.go b/tests/mocks/cfmm_pool.go index 82f435c25a4..0174447739e 100644 --- a/tests/mocks/cfmm_pool.go +++ b/tests/mocks/cfmm_pool.go @@ -185,6 +185,20 @@ func (mr *MockCFMMPoolIMockRecorder) GetId() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetId", reflect.TypeOf((*MockCFMMPoolI)(nil).GetId)) } +// GetPoolDenoms mocks base method. +func (m *MockCFMMPoolI) GetPoolDenoms(arg0 types.Context) []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPoolDenoms", arg0) + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetPoolDenoms indicates an expected call of GetPoolDenoms. +func (mr *MockCFMMPoolIMockRecorder) GetPoolDenoms(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPoolDenoms", reflect.TypeOf((*MockCFMMPoolI)(nil).GetPoolDenoms), arg0) +} + // GetSpreadFactor mocks base method. func (m *MockCFMMPoolI) GetSpreadFactor(ctx types.Context) osmomath.Dec { m.ctrl.T.Helper() @@ -569,6 +583,20 @@ func (mr *MockPoolAmountOutExtensionMockRecorder) GetId() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetId", reflect.TypeOf((*MockPoolAmountOutExtension)(nil).GetId)) } +// GetPoolDenoms mocks base method. +func (m *MockPoolAmountOutExtension) GetPoolDenoms(arg0 types.Context) []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPoolDenoms", arg0) + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetPoolDenoms indicates an expected call of GetPoolDenoms. +func (mr *MockPoolAmountOutExtensionMockRecorder) GetPoolDenoms(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPoolDenoms", reflect.TypeOf((*MockPoolAmountOutExtension)(nil).GetPoolDenoms), arg0) +} + // GetSpreadFactor mocks base method. func (m *MockPoolAmountOutExtension) GetSpreadFactor(ctx types.Context) osmomath.Dec { m.ctrl.T.Helper() @@ -950,6 +978,20 @@ func (mr *MockWeightedPoolExtensionMockRecorder) GetId() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetId", reflect.TypeOf((*MockWeightedPoolExtension)(nil).GetId)) } +// GetPoolDenoms mocks base method. +func (m *MockWeightedPoolExtension) GetPoolDenoms(arg0 types.Context) []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPoolDenoms", arg0) + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetPoolDenoms indicates an expected call of GetPoolDenoms. +func (mr *MockWeightedPoolExtensionMockRecorder) GetPoolDenoms(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPoolDenoms", reflect.TypeOf((*MockWeightedPoolExtension)(nil).GetPoolDenoms), arg0) +} + // GetSpreadFactor mocks base method. func (m *MockWeightedPoolExtension) GetSpreadFactor(ctx types.Context) osmomath.Dec { m.ctrl.T.Helper() diff --git a/tests/mocks/cl_pool.go b/tests/mocks/cl_pool.go index b294afa3a3d..3ec72628a67 100644 --- a/tests/mocks/cl_pool.go +++ b/tests/mocks/cl_pool.go @@ -193,6 +193,20 @@ func (mr *MockConcentratedPoolExtensionMockRecorder) GetLiquidity() *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLiquidity", reflect.TypeOf((*MockConcentratedPoolExtension)(nil).GetLiquidity)) } +// GetPoolDenoms mocks base method. +func (m *MockConcentratedPoolExtension) GetPoolDenoms(arg0 types.Context) []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPoolDenoms", arg0) + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetPoolDenoms indicates an expected call of GetPoolDenoms. +func (mr *MockConcentratedPoolExtensionMockRecorder) GetPoolDenoms(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPoolDenoms", reflect.TypeOf((*MockConcentratedPoolExtension)(nil).GetPoolDenoms), arg0) +} + // GetSpreadFactor mocks base method. func (m *MockConcentratedPoolExtension) GetSpreadFactor(ctx types.Context) osmomath.Dec { m.ctrl.T.Helper() diff --git a/tests/mocks/pool.go b/tests/mocks/pool.go index 7af46f53940..c73b58b3aeb 100644 --- a/tests/mocks/pool.go +++ b/tests/mocks/pool.go @@ -78,6 +78,20 @@ func (mr *MockPoolIMockRecorder) GetId() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetId", reflect.TypeOf((*MockPoolI)(nil).GetId)) } +// GetPoolDenoms mocks base method. +func (m *MockPoolI) GetPoolDenoms(arg0 types.Context) []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPoolDenoms", arg0) + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetPoolDenoms indicates an expected call of GetPoolDenoms. +func (mr *MockPoolIMockRecorder) GetPoolDenoms(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPoolDenoms", reflect.TypeOf((*MockPoolI)(nil).GetPoolDenoms), arg0) +} + // GetSpreadFactor mocks base method. func (m *MockPoolI) GetSpreadFactor(ctx types.Context) osmomath.Dec { m.ctrl.T.Helper() diff --git a/x/concentrated-liquidity/model/pool.go b/x/concentrated-liquidity/model/pool.go index 20e63f1ceee..de127966070 100644 --- a/x/concentrated-liquidity/model/pool.go +++ b/x/concentrated-liquidity/model/pool.go @@ -316,3 +316,11 @@ func (p *Pool) ApplySwap(newLiquidity osmomath.Dec, newCurrentTick int64, newCur func (p *Pool) AsSerializablePool() poolmanagertypes.PoolI { return p } + +// GetPoolDenoms implements types.ConcentratedPoolExtension. +func (p *Pool) GetPoolDenoms(ctx sdk.Context) []string { + return []string{ + p.GetToken0(), + p.GetToken1(), + } +} diff --git a/x/concentrated-liquidity/model/pool_test.go b/x/concentrated-liquidity/model/pool_test.go index 2722e321f77..4e2f67297c2 100644 --- a/x/concentrated-liquidity/model/pool_test.go +++ b/x/concentrated-liquidity/model/pool_test.go @@ -857,3 +857,20 @@ func (suite *ConcentratedPoolTestSuite) TestPoolSetMethods() { }) } } + +// Test that the right denoms are returned. +func (s *ConcentratedPoolTestSuite) TestGetPoolDenoms() { + s.Setup() + + const ( + expectedDenom1 = "bar" + expectedDenom2 = "foo" + ) + + pool := s.PrepareConcentratedPoolWithCoins(expectedDenom1, expectedDenom2) + + denoms := pool.GetPoolDenoms(s.Ctx) + s.Require().Equal(2, len(denoms)) + s.Require().Equal(expectedDenom1, denoms[0]) + s.Require().Equal(expectedDenom2, denoms[1]) +} diff --git a/x/cosmwasmpool/model/pool.go b/x/cosmwasmpool/model/pool.go index d2926f901ce..7ca2fa3f32e 100644 --- a/x/cosmwasmpool/model/pool.go +++ b/x/cosmwasmpool/model/pool.go @@ -5,6 +5,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/osmosis-labs/osmosis/osmoutils" "github.com/osmosis-labs/osmosis/v20/x/cosmwasmpool/cosmwasm/msg" "github.com/osmosis-labs/osmosis/v20/x/cosmwasmpool/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v20/x/poolmanager/types" @@ -137,6 +138,12 @@ func (p *Pool) SetWasmKeeper(wasmKeeper types.WasmKeeper) { p.WasmKeeper = wasmKeeper } +// GetPoolDenoms implements types.PoolI. +func (p *Pool) GetPoolDenoms(ctx sdk.Context) []string { + poolLiquidity := p.GetTotalPoolLiquidity(ctx) + return osmoutils.CoinsDenoms(poolLiquidity) +} + func (p Pool) AsSerializablePool() poolmanagertypes.PoolI { return &CosmWasmPool{ ContractAddress: p.ContractAddress, diff --git a/x/cosmwasmpool/model/pool_test.go b/x/cosmwasmpool/model/pool_test.go index 7bbd4712f59..1f1d424b817 100644 --- a/x/cosmwasmpool/model/pool_test.go +++ b/x/cosmwasmpool/model/pool_test.go @@ -57,3 +57,10 @@ func (s *CosmWasmPoolSuite) TestSpotPrice() { s.Require().Equal(expectedSpotPrice, actualSpotPrice) } + +// TestGetPoolDenoms validates that pool denoms are returned correctly. +func (s *CosmWasmPoolSuite) TestGetPoolDenoms() { + cwPool := s.PrepareCosmWasmPool() + poolDenoms := cwPool.GetPoolDenoms(s.Ctx) + s.Require().Equal([]string{apptesting.DefaultTransmuterDenomA, apptesting.DefaultTransmuterDenomB}, poolDenoms) +} diff --git a/x/cosmwasmpool/model/store_model.go b/x/cosmwasmpool/model/store_model.go index 5c193b3c6ab..8107f79d95e 100644 --- a/x/cosmwasmpool/model/store_model.go +++ b/x/cosmwasmpool/model/store_model.go @@ -96,3 +96,8 @@ func (p CosmWasmPool) GetStoreModel() poolmanagertypes.PoolI { func (p CosmWasmPool) SetWasmKeeper(wasmKeeper types.WasmKeeper) { panic("CosmWasmPool.SetWasmKeeeper not implemented") } + +// GetPoolDenoms implements types.PoolI. +func (p *CosmWasmPool) GetPoolDenoms(ctx sdk.Context) []string { + panic("CosmWasmPool.GetPoolDenoms not implemented") +} diff --git a/x/gamm/pool-models/balancer/pool.go b/x/gamm/pool-models/balancer/pool.go index 2dff28faf67..38d6f87569e 100644 --- a/x/gamm/pool-models/balancer/pool.go +++ b/x/gamm/pool-models/balancer/pool.go @@ -11,6 +11,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/osmosis-labs/osmosis/osmomath" + "github.com/osmosis-labs/osmosis/osmoutils" "github.com/osmosis-labs/osmosis/v20/x/gamm/pool-models/internal/cfmm_common" "github.com/osmosis-labs/osmosis/v20/x/gamm/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v20/x/poolmanager/types" @@ -982,3 +983,9 @@ func (p *Pool) ExitSwapExactAmountOut( func (p *Pool) AsSerializablePool() poolmanagertypes.PoolI { return p } + +// GetPoolDenoms implements types.CFMMPoolI. +func (p *Pool) GetPoolDenoms(ctx sdk.Context) []string { + liquidity := p.GetTotalPoolLiquidity(ctx) + return osmoutils.CoinsDenoms(liquidity) +} diff --git a/x/gamm/pool-models/balancer/pool_test.go b/x/gamm/pool-models/balancer/pool_test.go index 70356f91deb..9ba97996634 100644 --- a/x/gamm/pool-models/balancer/pool_test.go +++ b/x/gamm/pool-models/balancer/pool_test.go @@ -4,7 +4,7 @@ import ( "errors" "fmt" "testing" - time "time" + "time" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" @@ -1363,3 +1363,21 @@ func TestCalcJoinPoolNoSwapShares(t *testing.T) { }) } } + +// Test that the right denoms are returned. +func (s *KeeperTestSuite) TestGetPoolDenoms() { + const ( + expectedDenom1 = "bar" + expectedDenom2 = "foo" + ) + + poolID := s.PrepareBalancerPoolWithCoins(sdk.NewCoin(expectedDenom1, osmomath.NewInt(100)), sdk.NewCoin(expectedDenom2, osmomath.NewInt(100))) + + pool, err := s.App.PoolManagerKeeper.GetPool(s.Ctx, poolID) + s.Require().NoError(err) + + denoms := pool.GetPoolDenoms(s.Ctx) + s.Require().Equal(2, len(denoms)) + s.Require().Equal(expectedDenom1, denoms[0]) + s.Require().Equal(expectedDenom2, denoms[1]) +} diff --git a/x/gamm/pool-models/stableswap/pool.go b/x/gamm/pool-models/stableswap/pool.go index 47e124bf4f1..9da493e4df8 100644 --- a/x/gamm/pool-models/stableswap/pool.go +++ b/x/gamm/pool-models/stableswap/pool.go @@ -10,6 +10,7 @@ import ( errorsmod "cosmossdk.io/errors" "github.com/osmosis-labs/osmosis/osmomath" + "github.com/osmosis-labs/osmosis/osmoutils" "github.com/osmosis-labs/osmosis/v20/x/gamm/pool-models/internal/cfmm_common" "github.com/osmosis-labs/osmosis/v20/x/gamm/types" poolmanagertypes "github.com/osmosis-labs/osmosis/v20/x/poolmanager/types" @@ -492,3 +493,9 @@ func applyScalingFactorMultiplier(scalingFactors []uint64) ([]uint64, error) { func (p *Pool) AsSerializablePool() poolmanagertypes.PoolI { return p } + +// GetPoolDenoms implements types.CFMMPoolI. +func (p *Pool) GetPoolDenoms(ctx sdk.Context) []string { + liquidity := p.GetTotalPoolLiquidity(ctx) + return osmoutils.CoinsDenoms(liquidity) +} diff --git a/x/poolmanager/types/pool.go b/x/poolmanager/types/pool.go index 60cdad41b9d..f741726c9d4 100644 --- a/x/poolmanager/types/pool.go +++ b/x/poolmanager/types/pool.go @@ -23,6 +23,9 @@ type PoolI interface { // Returns whether the pool has swaps enabled at the moment IsActive(ctx sdk.Context) bool + // GetPoolDenoms returns the pool's denoms. + GetPoolDenoms(sdk.Context) []string + // Returns the spot price of the 'base asset' in terms of the 'quote asset' in the pool, // errors if either baseAssetDenom, or quoteAssetDenom does not exist. // For example, if this was a UniV2 50-50 pool, with 2 ETH, and 8000 UST From 85a98336dbcf007785273302a81d0ba8ac625af0 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 21 Nov 2023 00:09:40 -0500 Subject: [PATCH 2/4] Update CHANGELOG.md --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fdc53f0bd9..21e9d13494f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,8 +53,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### API Breaks * [#6805](https://github.com/osmosis-labs/osmosis/pull/6805) return bucket index of the current tick from LiquidityPerTickRange query -<<<<<<< HEAD -======= * [#6530](https://github.com/osmosis-labs/osmosis/pull/6530) Improve error message when CL LP fails due to slippage bound hit. * [#6863](https://github.com/osmosis-labs/osmosis/pull/6863) GetPoolDenoms method on PoolI interface in poolmanager From 75e47c28b181b286e73027cc30cfe882b0f12c80 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 21 Nov 2023 00:09:56 -0500 Subject: [PATCH 3/4] Update CHANGELOG.md --- CHANGELOG.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21e9d13494f..215decf9cd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,12 +56,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#6530](https://github.com/osmosis-labs/osmosis/pull/6530) Improve error message when CL LP fails due to slippage bound hit. * [#6863](https://github.com/osmosis-labs/osmosis/pull/6863) GetPoolDenoms method on PoolI interface in poolmanager -### Bug Fixes - -* [#6840](https://github.com/osmosis-labs/osmosis/pull/6840) fix: change TypeMsgUnbondConvertAndStake value to "unbond_convert_and_stake" and improve error message when epoch currentEpochStartHeight less than zero -* [#6769](https://github.com/osmosis-labs/osmosis/pull/6769) fix: improve dust handling in EstimateTradeBasedOnPriceImpact -* [#6841](https://github.com/osmosis-labs/osmosis/pull/6841) fix: fix receive_ack response field and imporove error message of InvalidCrosschainSwapsContract and NoDenomTrace ->>>>>>> 4c54425c (refactor(poolmanager): GetPoolDenoms method on PoolI for SQS (#6863)) ## v20.0.0 From 16027ef3d5350c9b4e90879102c5f430da1c2ee2 Mon Sep 17 00:00:00 2001 From: Roman Date: Tue, 21 Nov 2023 00:10:10 -0500 Subject: [PATCH 4/4] Update CHANGELOG.md --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 215decf9cd9..a581b1dbad4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### API Breaks * [#6805](https://github.com/osmosis-labs/osmosis/pull/6805) return bucket index of the current tick from LiquidityPerTickRange query -* [#6530](https://github.com/osmosis-labs/osmosis/pull/6530) Improve error message when CL LP fails due to slippage bound hit. * [#6863](https://github.com/osmosis-labs/osmosis/pull/6863) GetPoolDenoms method on PoolI interface in poolmanager