Skip to content

Commit

Permalink
Revert "remove second validation"
Browse files Browse the repository at this point in the history
This reverts commit 7f68d27.
  • Loading branch information
czarcas7ic committed Dec 6, 2022
1 parent e7a9fee commit c282280
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 84 deletions.
45 changes: 3 additions & 42 deletions x/concentrated-liquidity/keeper.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package concentrated_liquidity

import (
"errors"

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"

Expand All @@ -26,44 +24,7 @@ func NewKeeper(cdc codec.BinaryCodec, storeKey sdk.StoreKey, bankKeeper types.Ba
}
}

// TODO: implement minting, spec, tests
func (k Keeper) InitializePool(ctx sdk.Context, poolI swaproutertypes.PoolI, creatorAddress sdk.AccAddress) error {
pool, ok := poolI.(types.ConcentratedPoolExtension)
if !ok {
return errors.New("invalid pool type when setting concentrated pool")
}
//poolId := pool.GetId()

// Add the share token's meta data to the bank keeper.
// poolShareBaseDenom := types.GetPoolShareDenom(poolId)
// poolShareDisplayDenom := fmt.Sprintf("GAMM-%d", poolId)
// k.bankKeeper.SetDenomMetaData(ctx, banktypes.Metadata{
// Description: fmt.Sprintf("The share token of the gamm pool %d", poolId),
// DenomUnits: []*banktypes.DenomUnit{
// {
// Denom: poolShareBaseDenom,
// Exponent: 0,
// Aliases: []string{
// "attopoolshare",
// },
// },
// {
// Denom: poolShareDisplayDenom,
// Exponent: types.OneShareExponent,
// Aliases: nil,
// },
// },
// Base: poolShareBaseDenom,
// Display: poolShareDisplayDenom,
// })

// // Mint the initial pool shares share token to the sender
// err := k.MintPoolShareToAccount(ctx, pool, creatorAddress, pool.GetTotalShares())
// if err != nil {
// return err
// }

// k.RecordTotalLiquidityIncrease(ctx, pool.GetTotalPoolLiquidity(ctx))

return k.setPool(ctx, pool)
// TODO: spec, tests, implementation
func (k Keeper) InitializePool(ctx sdk.Context, pool swaproutertypes.PoolI, creatorAddress sdk.AccAddress) error {
return nil
}
2 changes: 1 addition & 1 deletion x/concentrated-liquidity/model/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (msg MsgCreateConcentratedPool) Validate(ctx sdk.Context) error {
}

func (msg MsgCreateConcentratedPool) InitialLiquidity() sdk.Coins {
return sdk.Coins{}
return nil
}

func (msg MsgCreateConcentratedPool) CreatePool(ctx sdk.Context, poolID uint64) (swaproutertypes.PoolI, error) {
Expand Down
10 changes: 1 addition & 9 deletions x/concentrated-liquidity/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,7 @@ import (

// GetPool returns a pool with a given id.
func (k Keeper) GetPool(ctx sdk.Context, poolId uint64) (swaproutertypes.PoolI, error) {
concentratedPool, err := k.getPoolById(ctx, poolId)
if err != nil {
return nil, types.PoolNotFoundError{PoolId: poolId}
}
pool, ok := concentratedPool.(swaproutertypes.PoolI)
if !ok {
return nil, errors.New("invalid pool type when setting concentrated pool")
}
return pool, nil
return nil, errors.New("not implemented")
}

// getPoolById returns a concentratedPoolExtension that corresponds to the requested pool id. Returns error if pool id is not found.
Expand Down
1 change: 0 additions & 1 deletion x/gamm/pool-models/balancer/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ func (msg MsgCreateBalancerPool) ValidateBasic() error {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "Invalid sender address (%s)", err)
}

// validates number of assets meets both minimum and maximum criteria
err = validateUserSpecifiedPoolAssets(msg.PoolAssets)
if err != nil {
return err
Expand Down
6 changes: 1 addition & 5 deletions x/gamm/pool-models/balancer/pool_asset.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@ import (
"sort"
"strings"

"gopkg.in/yaml.v2"

"github.com/osmosis-labs/osmosis/v13/x/gamm/types"
"gopkg.in/yaml.v2"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
Expand Down Expand Up @@ -68,9 +67,6 @@ func sortPoolAssetsByDenom(assets []PoolAsset) {
})
}

// validateUserSpecifiedPoolAssets validates that the provided pool assets meet certain criteria.
// It checks that the number of assets is at least 2 and no more than 8, that each asset's weight is valid,
// and that each asset's token is valid and positive. If any of these checks fail, the corresponding error is returned.
func validateUserSpecifiedPoolAssets(assets []PoolAsset) error {
// The pool must be swapping between at least two assets
if len(assets) < 2 {
Expand Down
42 changes: 41 additions & 1 deletion x/swaprouter/creator.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
// the form of <swap module name>/pool/{poolID}. In addition, the x/bank metadata is updated
// to reflect the newly created GAMM share denomination.
func (k Keeper) CreatePool(ctx sdk.Context, msg types.CreatePoolMsg) (uint64, error) {
err := msg.Validate(ctx)
err := validateCreatePoolMsg(ctx, msg)
if err != nil {
return 0, err
}
Expand Down Expand Up @@ -72,6 +72,7 @@ func (k Keeper) CreatePool(ctx sdk.Context, msg types.CreatePoolMsg) (uint64, er
if poolType != types.Concentrated {
k.poolCreationListeners.AfterPoolCreated(ctx, sender, pool.GetId())
}

return pool.GetId(), nil
}

Expand All @@ -82,6 +83,29 @@ func (k Keeper) getNextPoolIdAndIncrement(ctx sdk.Context) uint64 {
return nextPoolId
}

func validateCreatePoolMsg(ctx sdk.Context, msg types.CreatePoolMsg) error {
err := msg.Validate(ctx)
if err != nil {
return err
}

initialPoolLiquidity := msg.InitialLiquidity()
numAssets := initialPoolLiquidity.Len()

if msg.GetPoolType() != types.Concentrated {
if numAssets < types.MinPoolAssets {
return types.ErrTooFewPoolAssets
}
if numAssets > types.MaxPoolAssets {
return errors.Wrapf(
types.ErrTooManyPoolAssets,
"pool has too many PoolAssets (%d)", numAssets,
)
}
}
return nil
}

func (k Keeper) validateCreatedPool(
ctx sdk.Context,
initialPoolLiquidity sdk.Coins,
Expand All @@ -97,6 +121,22 @@ func (k Keeper) validateCreatedPool(
return errors.Wrapf(types.ErrInvalidPool,
"Pool was attempted to be created with incorrect pool address.")
}

// Check the total pool liquidity/shares if pool is not a concentrated liquidity pool
if poolType != types.Concentrated {
// Notably we use the initial pool liquidity at the start of the messages definition
// just in case CreatePool was mutative.
if !pool.GetTotalPoolLiquidity(ctx).IsEqual(initialPoolLiquidity) {
return errors.Wrap(types.ErrInvalidPool,
"Pool was attempted to be created, with initial liquidity not equal to what was specified.")
}
// TODO: this check should be moved
// This check can be removed later, and replaced with a minimum.
if !pool.GetTotalShares().Equal(gammtypes.InitPoolSharesSupply) {
return errors.Wrap(types.ErrInvalidPool,
"Pool was attempted to be created with incorrect number of initial shares.")
}
}
return nil
}

Expand Down
40 changes: 15 additions & 25 deletions x/swaprouter/creator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

clmodel "github.com/osmosis-labs/osmosis/v13/x/concentrated-liquidity/model"
"github.com/osmosis-labs/osmosis/v13/x/gamm/pool-models/balancer"
gammtypes "github.com/osmosis-labs/osmosis/v13/x/gamm/types"
"github.com/osmosis-labs/osmosis/v13/x/swaprouter/types"
)

Expand All @@ -28,26 +27,23 @@ func (suite *KeeperTestSuite) TestCreatePool() {
validConcentratedPoolMsg := clmodel.NewMsgCreateConcentratedPool(suite.TestAccs[0], "eth", "usdc")

tests := []struct {
name string
creatorFundAmount sdk.Coins
msg types.CreatePoolMsg
expectedModuleType reflect.Type
expectedInitialPoolShareSupply sdk.Int
expectError bool
name string
creatorFundAmount sdk.Coins
msg types.CreatePoolMsg
expectedModuleType reflect.Type
expectError bool
}{
{
name: "first balancer pool - success",
creatorFundAmount: sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount.Mul(sdk.NewInt(2))), sdk.NewCoin(bar, defaultInitPoolAmount.Mul(sdk.NewInt(2)))),
msg: validBalancerPoolMsg,
expectedModuleType: gammKeeperType,
expectedInitialPoolShareSupply: gammtypes.InitPoolSharesSupply,
name: "first balancer pool - success",
creatorFundAmount: sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount.Mul(sdk.NewInt(2))), sdk.NewCoin(bar, defaultInitPoolAmount.Mul(sdk.NewInt(2)))),
msg: validBalancerPoolMsg,
expectedModuleType: gammKeeperType,
},
{
name: "second balancer pool - success",
creatorFundAmount: sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount.Mul(sdk.NewInt(2))), sdk.NewCoin(bar, defaultInitPoolAmount.Mul(sdk.NewInt(2)))),
msg: validBalancerPoolMsg,
expectedModuleType: gammKeeperType,
expectedInitialPoolShareSupply: gammtypes.InitPoolSharesSupply,
name: "second balancer pool - success",
creatorFundAmount: sdk.NewCoins(sdk.NewCoin(foo, defaultInitPoolAmount.Mul(sdk.NewInt(2))), sdk.NewCoin(bar, defaultInitPoolAmount.Mul(sdk.NewInt(2)))),
msg: validBalancerPoolMsg,
expectedModuleType: gammKeeperType,
},
{
name: "concentrated liquidity pool - success",
Expand All @@ -64,9 +60,8 @@ func (suite *KeeperTestSuite) TestCreatePool() {

swaprouterKeeper := suite.App.SwapRouterKeeper
ctx := suite.Ctx
poolId := uint64(i + 1)

poolCreationFee := swaprouterKeeper.GetParams(ctx).PoolCreationFee
poolCreationFee := swaprouterKeeper.GetParams(suite.Ctx).PoolCreationFee
suite.FundAcc(suite.TestAccs[0], append(tc.creatorFundAmount, poolCreationFee...))

poolId, err := swaprouterKeeper.CreatePool(ctx, tc.msg)
Expand All @@ -76,17 +71,12 @@ func (suite *KeeperTestSuite) TestCreatePool() {
return
}

swapModule, err := swaprouterKeeper.GetSwapModule(ctx, poolId)
pool, err := swapModule.GetPool(ctx, poolId)
suite.Require().NoError(err)

// Validate pool.
suite.Require().NoError(err)
suite.Require().Equal(uint64(i+1), poolId)
suite.Require().Equal(tc.msg.InitialLiquidity().String(), pool.GetTotalPoolLiquidity(ctx).String())
suite.Require().Equal(tc.expectedInitialPoolShareSupply.String(), pool.GetTotalShares().String())

// Validate that mapping pool id -> module type has been persisted.
swapModule, err := swaprouterKeeper.GetSwapModule(ctx, poolId)
suite.Require().NoError(err)
suite.Require().Equal(tc.expectedModuleType, reflect.TypeOf(swapModule))
})
Expand Down

0 comments on commit c282280

Please sign in to comment.