refactor(x/superfluid): improve error handling for AddNewSuperfluidAs…

…set (#2473) * refactor(x/superfluid): improve error handling for AddNewSuperfluidAsset * changelog
osmosis-labs · Aug 21, 2022 · 2279d80 · 2279d80
1 parent c1fc1f8
commit 2279d80
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -58,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * [#2227](https://github.com/osmosis-labs/osmosis/pull/2227) Enable charging fee in base denom for `CreateGauge` and `AddToGauge`.
 * [#2283](https://github.com/osmosis-labs/osmosis/pull/2283) x/incentives: refactor `CreateGauge` and `AddToGauge` fees to use txfees denom
 * [#2206](https://github.com/osmosis-labs/osmosis/pull/2283) Register all Amino interfaces and concrete types on the authz Amino codec. This will allow the authz module to properly serialize and de-serializes instances using Amino.
+* [#2473](https://github.com/osmosis-labs/osmosis/pull/2473) x/superfluid `AddNewSuperfluidAsset` now returns error, if any occurs instead of ignoring it.
 
 #### Golang API breaks
 

diff --git a/app/upgrades/v7/upgrades.go b/app/upgrades/v7/upgrades.go
@@ -52,7 +52,9 @@ func CreateUpgradeHandler(
 			Denom:     "gamm/pool/1",
 			AssetType: superfluidtypes.SuperfluidAssetTypeLPShare,
 		}
-		keepers.SuperfluidKeeper.AddNewSuperfluidAsset(ctx, superfluidAsset)
+		if err := keepers.SuperfluidKeeper.AddNewSuperfluidAsset(ctx, superfluidAsset); err != nil {
+			return newVM, err
+		}
 
 		// N.B.: This is left for historic reasons.
 		// After the v7 upgrade, there was no need for this function anymore so it was removed.

diff --git a/x/superfluid/keeper/gov/gov.go b/x/superfluid/keeper/gov/gov.go
@@ -11,7 +11,9 @@ import (
 
 func HandleSetSuperfluidAssetsProposal(ctx sdk.Context, k keeper.Keeper, ek types.EpochKeeper, p *types.SetSuperfluidAssetsProposal) error {
 	for _, asset := range p.Assets {
-		k.AddNewSuperfluidAsset(ctx, asset)
+		if err := k.AddNewSuperfluidAsset(ctx, asset); err != nil {
+			return err
+		}
 		event := sdk.NewEvent(
 			types.TypeEvtSetSuperfluidAsset,
 			sdk.NewAttribute(types.AttributeDenom, asset.Denom),

diff --git a/x/superfluid/keeper/keeper_test.go b/x/superfluid/keeper/keeper_test.go
@@ -135,16 +135,18 @@ func (suite *KeeperTestSuite) SetupGammPoolsAndSuperfluidAssets(multipliers []sd
 	for _, pool := range pools {
 		denom := gammtypes.GetPoolShareDenom(pool.GetId())
 
-		suite.App.SuperfluidKeeper.AddNewSuperfluidAsset(suite.Ctx, types.SuperfluidAsset{
+		err := suite.App.SuperfluidKeeper.AddNewSuperfluidAsset(suite.Ctx, types.SuperfluidAsset{
 			Denom:     denom,
 			AssetType: types.SuperfluidAssetTypeLPShare,
 		})
+		suite.Require().NoError(err)
 
 		// register a LP token as a superfluid asset
-		suite.App.SuperfluidKeeper.AddNewSuperfluidAsset(suite.Ctx, types.SuperfluidAsset{
+		err = suite.App.SuperfluidKeeper.AddNewSuperfluidAsset(suite.Ctx, types.SuperfluidAsset{
 			Denom:     denom,
 			AssetType: types.SuperfluidAssetTypeLPShare,
 		})
+		suite.Require().NoError(err)
 
 		denoms = append(denoms, denom)
 		poolIds = append(poolIds, pool.GetId())

diff --git a/x/superfluid/keeper/superfluid_asset.go b/x/superfluid/keeper/superfluid_asset.go
@@ -38,11 +38,11 @@ func (k Keeper) UnriskAdjustOsmoValue(ctx sdk.Context, amount sdk.Dec) sdk.Dec {
 	return amount.Quo(sdk.OneDec().Sub(minRiskFactor))
 }
 
-func (k Keeper) AddNewSuperfluidAsset(ctx sdk.Context, asset types.SuperfluidAsset) {
+func (k Keeper) AddNewSuperfluidAsset(ctx sdk.Context, asset types.SuperfluidAsset) error {
 	// initialize osmo equivalent multipliers
 	epochIdentifier := k.GetEpochIdentifier(ctx)
 	currentEpoch := k.ek.GetEpochInfo(ctx, epochIdentifier).CurrentEpoch
-	_ = osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error {
+	return osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error {
 		k.SetSuperfluidAsset(ctx, asset)
 		err := k.UpdateOsmoEquivalentMultipliers(ctx, asset, currentEpoch)
 		return err

diff --git a/x/superfluid/keeper/unpool_test.go b/x/superfluid/keeper/unpool_test.go
@@ -134,10 +134,11 @@ func (suite *KeeperTestSuite) TestUnpool() {
 			poolShareOut := bankKeeper.GetBalance(ctx, poolJoinAcc, poolDenom)
 
 			// register a LP token as a superfluid asset
-			superfluidKeeper.AddNewSuperfluidAsset(ctx, types.SuperfluidAsset{
+			err = superfluidKeeper.AddNewSuperfluidAsset(ctx, types.SuperfluidAsset{
 				Denom:     poolDenom,
 				AssetType: types.SuperfluidAssetTypeLPShare,
 			})
+			suite.Require().NoError(err)
 
 			// whitelist designated pools
 			// this should be done via `RunForkLogic` at upgrade