Skip to content

Commit

Permalink
refactor(x/mint, x/gamm errors): move errors in mint and gamm keeper …
Browse files Browse the repository at this point in the history
…to errors.go (#2067)

* moved mint errors

* errors formatted

* rom and bez feedback

* rebased

* addressed all comments
  • Loading branch information
stackman27 authored Jul 18, 2022
1 parent 17ebcc8 commit e5ffb81
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 41 deletions.
4 changes: 2 additions & 2 deletions x/gamm/pool-models/balancer/amm.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ func getPoolAssetsByDenom(poolAssets []PoolAsset) (map[string]PoolAsset, error)
for _, poolAsset := range poolAssets {
_, ok := poolAssetsByDenom[poolAsset.Token.Denom]
if ok {
return nil, fmt.Errorf(errMsgFormatRepeatingPoolAssetsNotAllowed, poolAsset.Token.Denom)
return nil, fmt.Errorf(formatRepeatingPoolAssetsNotAllowedErrFormat, poolAsset.Token.Denom)
}

poolAssetsByDenom[poolAsset.Token.Denom] = poolAsset
Expand All @@ -174,7 +174,7 @@ func updateIntermediaryPoolAssetsLiquidity(liquidity sdk.Coins, poolAssetsByDeno
for _, coin := range liquidity {
poolAsset, ok := poolAssetsByDenom[coin.Denom]
if !ok {
return fmt.Errorf(errMsgFormatFailedInterimLiquidityUpdate, coin.Denom)
return fmt.Errorf(failedInterimLiquidityUpdateErrFormat, coin.Denom)
}

poolAsset.Token.Amount = poolAssetsByDenom[coin.Denom].Token.Amount.Add(coin.Amount)
Expand Down
6 changes: 3 additions & 3 deletions x/gamm/pool-models/balancer/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package balancer
import sdk "github.com/cosmos/cosmos-sdk/types"

const (
ErrMsgFormatRepeatingPoolAssetsNotAllowed = errMsgFormatRepeatingPoolAssetsNotAllowed
ErrMsgFormatNoPoolAssetFound = errMsgFormatNoPoolAssetFound
ErrMsgFormatRepeatingPoolAssetsNotAllowed = formatRepeatingPoolAssetsNotAllowedErrFormat
ErrMsgFormatNoPoolAssetFound = formatNoPoolAssetFoundErrFormat
)

var (
ErrMsgFormatFailedInterimLiquidityUpdate = errMsgFormatFailedInterimLiquidityUpdate
ErrMsgFormatFailedInterimLiquidityUpdate = failedInterimLiquidityUpdateErrFormat

CalcPoolSharesOutGivenSingleAssetIn = calcPoolSharesOutGivenSingleAssetIn
CalcSingleAssetInGivenPoolSharesOut = calcSingleAssetInGivenPoolSharesOut
Expand Down
32 changes: 16 additions & 16 deletions x/gamm/pool-models/balancer/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ import (

//nolint:deadcode
const (
errMsgFormatSharesAmountNotPositive = "shares amount must be positive, was %d"
errMsgFormatTokenAmountNotPositive = "token amount must be positive, was %d"
errMsgFormatTokensLargerThanMax = "%d resulted tokens is larger than the max amount of %d"
errMsgFormatSharesLargerThanMax = "%d resulted shares is larger than the max amount of %d"
errMsgFormatFailedInterimLiquidityUpdate = "failed to update interim liquidity - pool asset %s does not exist"
errMsgFormatRepeatingPoolAssetsNotAllowed = "repeating pool assets not allowed, found %s"
errMsgFormatNoPoolAssetFound = "can't find the PoolAsset (%s)"
errMsgFormatInvalidInputDenoms = "input denoms must already exist in the pool (%s)"
nonPostiveSharesAmountErrFormat = "shares amount must be positive, was %d"
nonPostiveTokenAmountErrFormat = "token amount must be positive, was %d"
sharesLargerThanMaxErrFormat = "%d resulted shares is larger than the max amount of %d"
invalidInputDenomsErrFormat = "input denoms must already exist in the pool (%s)"

failedInterimLiquidityUpdateErrFormat = "failed to update interim liquidity - pool asset %s does not exist"
formatRepeatingPoolAssetsNotAllowedErrFormat = "repeating pool assets not allowed, found %s"
formatNoPoolAssetFoundErrFormat = "can't find the PoolAsset (%s)"
)

var (
Expand Down Expand Up @@ -223,7 +223,7 @@ func (p Pool) getPoolAssetAndIndex(denom string) (int, PoolAsset, error) {
}

if len(p.PoolAssets) == 0 {
return -1, PoolAsset{}, sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, fmt.Sprintf(errMsgFormatNoPoolAssetFound, denom))
return -1, PoolAsset{}, sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, fmt.Sprintf(formatNoPoolAssetFoundErrFormat, denom))
}

i := sort.Search(len(p.PoolAssets), func(i int) bool {
Expand All @@ -234,11 +234,11 @@ func (p Pool) getPoolAssetAndIndex(denom string) (int, PoolAsset, error) {
})

if i < 0 || i >= len(p.PoolAssets) {
return -1, PoolAsset{}, sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, fmt.Sprintf(errMsgFormatNoPoolAssetFound, denom))
return -1, PoolAsset{}, sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, fmt.Sprintf(formatNoPoolAssetFoundErrFormat, denom))
}

if p.PoolAssets[i].Token.Denom != denom {
return -1, PoolAsset{}, sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, fmt.Sprintf(errMsgFormatNoPoolAssetFound, denom))
return -1, PoolAsset{}, sdkerrors.Wrapf(types.ErrDenomNotFoundInPool, fmt.Sprintf(formatNoPoolAssetFoundErrFormat, denom))
}

return i, p.PoolAssets[i], nil
Expand Down Expand Up @@ -692,7 +692,7 @@ func (p *Pool) CalcJoinPoolShares(ctx sdk.Context, tokensIn sdk.Coins, swapFee s
for _, coin := range tokensIn {
_, ok := poolAssetsByDenom[coin.Denom]
if !ok {
return sdk.ZeroInt(), sdk.NewCoins(), fmt.Errorf(errMsgFormatInvalidInputDenoms, coin.Denom)
return sdk.ZeroInt(), sdk.NewCoins(), sdkerrors.Wrapf(types.ErrDenomAlreadyInPool, invalidInputDenomsErrFormat, coin.Denom)
}
}

Expand Down Expand Up @@ -827,7 +827,7 @@ func (p *Pool) CalcTokenInShareAmountOut(
).Ceil().TruncateInt()

if !tokenInAmount.IsPositive() {
return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, errMsgFormatTokenAmountNotPositive, tokenInAmount.Int64())
return sdk.Int{}, sdkerrors.Wrapf(types.ErrNotPositiveRequireAmount, nonPostiveTokenAmountErrFormat, tokenInAmount.Int64())
}

return tokenInAmount, nil
Expand All @@ -854,7 +854,7 @@ func (p *Pool) JoinPoolTokenInMaxShareAmountOut(
).TruncateInt()

if !tokenInAmount.IsPositive() {
return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, errMsgFormatTokenAmountNotPositive, tokenInAmount.Int64())
return sdk.Int{}, sdkerrors.Wrapf(types.ErrNotPositiveRequireAmount, nonPostiveTokenAmountErrFormat, tokenInAmount.Int64())
}

poolAssetIn.Token.Amount = poolAssetIn.Token.Amount.Add(tokenInAmount)
Expand Down Expand Up @@ -886,11 +886,11 @@ func (p *Pool) ExitSwapExactAmountOut(
).TruncateInt()

if !sharesIn.IsPositive() {
return sdk.Int{}, sdkerrors.Wrapf(types.ErrInvalidMathApprox, errMsgFormatSharesAmountNotPositive, sharesIn.Int64())
return sdk.Int{}, sdkerrors.Wrapf(types.ErrNotPositiveRequireAmount, nonPostiveSharesAmountErrFormat, sharesIn.Int64())
}

if sharesIn.GT(shareInMaxAmount) {
return sdk.Int{}, sdkerrors.Wrapf(types.ErrLimitMaxAmount, errMsgFormatSharesLargerThanMax, sharesIn.Int64(), shareInMaxAmount.Uint64())
return sdk.Int{}, sdkerrors.Wrapf(types.ErrLimitMaxAmount, sharesLargerThanMaxErrFormat, sharesIn.Int64(), shareInMaxAmount.Uint64())
}

if err := p.exitPool(ctx, sdk.NewCoins(tokenOut), sharesIn); err != nil {
Expand Down
1 change: 1 addition & 0 deletions x/gamm/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var (
ErrAlreadyInvalidPool = sdkerrors.Register(ModuleName, 9, "destruction on already invalid pool")
ErrInvalidPool = sdkerrors.Register(ModuleName, 10, "attempting to create an invalid pool")
ErrDenomNotFoundInPool = sdkerrors.Register(ModuleName, 11, "denom does not exist in pool")
ErrDenomAlreadyInPool = sdkerrors.Register(ModuleName, 12, "denom already exists in the pool")

ErrEmptyRoutes = sdkerrors.Register(ModuleName, 21, "routes not defined")
ErrEmptyPoolAssets = sdkerrors.Register(ModuleName, 22, "PoolAssets not defined")
Expand Down
4 changes: 0 additions & 4 deletions x/mint/keeper/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ const (
)

var (
ErrAmountCannotBeNilOrZero = errAmountCannotBeNilOrZero
ErrDevVestingModuleAccountAlreadyCreated = errDevVestingModuleAccountAlreadyCreated
ErrDevVestingModuleAccountNotCreated = errDevVestingModuleAccountNotCreated

GetProportions = getProportions
)

Expand Down
14 changes: 4 additions & 10 deletions x/mint/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package keeper

import (
"errors"
"fmt"

"github.com/tendermint/tendermint/libs/log"
Expand All @@ -11,6 +10,7 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
)
Expand All @@ -36,12 +36,6 @@ func (e invalidRatioError) Error() string {
return fmt.Sprintf("mint allocation ratio (%s) is greater than 1", e.ActualRatio)
}

var (
errAmountCannotBeNilOrZero = errors.New("amount cannot be nil or zero")
errDevVestingModuleAccountAlreadyCreated = fmt.Errorf("%s module account already exists", types.DeveloperVestingModuleAcctName)
errDevVestingModuleAccountNotCreated = fmt.Errorf("%s module account does not exist", types.DeveloperVestingModuleAcctName)
)

// NewKeeper creates a new mint Keeper instance.
func NewKeeper(
cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace,
Expand Down Expand Up @@ -79,7 +73,7 @@ func NewKeeper(
// queries. The method returns an error if current height in ctx is greater than the v7 upgrade height.
func (k Keeper) SetInitialSupplyOffsetDuringMigration(ctx sdk.Context) error {
if !k.accountKeeper.HasAccount(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName)) {
return errDevVestingModuleAccountNotCreated
return sdkerrors.Wrapf(types.ErrModuleDoesnotExist, "%s vesting module account doesnot exist", types.DeveloperVestingModuleAcctName)
}

moduleAccBalance := k.bankKeeper.GetBalance(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName), k.GetParams(ctx).MintDenom)
Expand All @@ -96,10 +90,10 @@ func (k Keeper) SetInitialSupplyOffsetDuringMigration(ctx sdk.Context) error {
// - developer vesting module account is already created prior to calling this method.
func (k Keeper) CreateDeveloperVestingModuleAccount(ctx sdk.Context, amount sdk.Coin) error {
if amount.IsNil() || amount.Amount.IsZero() {
return errAmountCannotBeNilOrZero
return sdkerrors.Wrap(types.ErrAmountNilOrZero, "amount cannot be nil or zero")
}
if k.accountKeeper.HasAccount(ctx, k.accountKeeper.GetModuleAddress(types.DeveloperVestingModuleAcctName)) {
return errDevVestingModuleAccountAlreadyCreated
return sdkerrors.Wrapf(types.ErrModuleAccountAlreadyExist, "%s vesting module account already exist", types.DeveloperVestingModuleAcctName)
}

moduleAcc := authtypes.NewEmptyModuleAccount(
Expand Down
12 changes: 6 additions & 6 deletions x/mint/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,18 +315,18 @@ func (suite *KeeperTestSuite) TestCreateDeveloperVestingModuleAccount() {
},
"nil amount": {
blockHeight: 0,
expectedError: keeper.ErrAmountCannotBeNilOrZero,
expectedError: sdkerrors.Wrap(types.ErrAmountNilOrZero, "amount cannot be nil or zero"),
},
"zero amount": {
blockHeight: 0,
amount: sdk.NewCoin("stake", sdk.NewInt(0)),
expectedError: keeper.ErrAmountCannotBeNilOrZero,
expectedError: sdkerrors.Wrap(types.ErrAmountNilOrZero, "amount cannot be nil or zero"),
},
"module account is already created": {
blockHeight: 0,
amount: sdk.NewCoin("stake", sdk.NewInt(keeper.DeveloperVestingAmount)),
isDeveloperModuleAccountCreated: true,
expectedError: keeper.ErrDevVestingModuleAccountAlreadyCreated,
expectedError: sdkerrors.Wrapf(types.ErrModuleAccountAlreadyExist, "%s vesting module account already exist", types.DeveloperVestingModuleAcctName),
},
}

Expand All @@ -340,7 +340,7 @@ func (suite *KeeperTestSuite) TestCreateDeveloperVestingModuleAccount() {

if tc.expectedError != nil {
suite.Error(actualError)
suite.Equal(actualError, tc.expectedError)
suite.ErrorIs(actualError, tc.expectedError)
return
}
suite.NoError(actualError)
Expand All @@ -361,7 +361,7 @@ func (suite *KeeperTestSuite) TestSetInitialSupplyOffsetDuringMigration() {
},
"dev vesting module account does not exist": {
blockHeight: 1,
expectedError: keeper.ErrDevVestingModuleAccountNotCreated,
expectedError: sdkerrors.Wrapf(types.ErrModuleDoesnotExist, "%s vesting module account doesnot exist", types.DeveloperVestingModuleAcctName),
},
}

Expand All @@ -380,7 +380,7 @@ func (suite *KeeperTestSuite) TestSetInitialSupplyOffsetDuringMigration() {

if tc.expectedError != nil {
suite.Error(actualError)
suite.Equal(actualError, tc.expectedError)
suite.ErrorIs(actualError, tc.expectedError)

suite.Equal(supplyWithOffsetBefore.Amount, bankKeeper.GetSupplyWithOffset(ctx, sdk.DefaultBondDenom).Amount)
suite.Equal(supplyOffsetBefore, bankKeeper.GetSupplyOffset(ctx, sdk.DefaultBondDenom))
Expand Down
11 changes: 11 additions & 0 deletions x/mint/types/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package types

import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

var (
ErrAmountNilOrZero = sdkerrors.Register(ModuleName, 2, "amount cannot be nil or zero")
ErrModuleAccountAlreadyExist = sdkerrors.Register(ModuleName, 3, "module account already exists")
ErrModuleDoesnotExist = sdkerrors.Register(ModuleName, 4, "module account does not exist")
)

0 comments on commit e5ffb81

Please sign in to comment.