Miscellaneous small fixes to superfluid logic (#865)

* WIP: superfluid spec

* Make unified method for severe errors

* Fix immediate epoch number problem

* Refactor updateEpochTwap into its own function

* More function moves, add epoch item to spec

* List the flow of whats expected to happen at epoch end

* Reorder epoch end order of operations

* Restore prior functionality of serializing twap at the new epoch numberf

* basic twap docs

* companion to prior commit

* Move TWAP calculation into its own spot

x/superfluid/keeper/gov.go

@@ -1,34 +1,38 @@
 package keeper
 
 import (
+	"fmt"
+
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/osmosis-labs/osmosis/x/superfluid/types"
 )
 
 func (k Keeper) HandleSetSuperfluidAssetsProposal(ctx sdk.Context, p *types.SetSuperfluidAssetsProposal) error {
 	for _, asset := range p.Assets {
 		k.SetSuperfluidAsset(ctx, asset)
-		ctx.EventManager().EmitEvents(sdk.Events{
-			sdk.NewEvent(
-				types.TypeEvtSetSuperfluidAsset,
-				sdk.NewAttribute(types.AttributeDenom, asset.Denom),
-				sdk.NewAttribute(types.AttributeSuperfluidAssetType, asset.AssetType.String()),
-			),
-		})
+		event := sdk.NewEvent(
+			types.TypeEvtSetSuperfluidAsset,
+			sdk.NewAttribute(types.AttributeDenom, asset.Denom),
+			sdk.NewAttribute(types.AttributeSuperfluidAssetType, asset.AssetType.String()),
+		)
+		ctx.EventManager().EmitEvent(event)
 	}
 	return nil
 }
 
 func (k Keeper) HandleRemoveSuperfluidAssetsProposal(ctx sdk.Context, p *types.RemoveSuperfluidAssetsProposal) error {
 	for _, denom := range p.SuperfluidAssetDenoms {
-		k.SetEpochOsmoEquivalentTWAP(ctx, 0, denom, sdk.ZeroDec())
-		k.DeleteSuperfluidAsset(ctx, denom)
-		ctx.EventManager().EmitEvents(sdk.Events{
-			sdk.NewEvent(
-				types.TypeEvtRemoveSuperfluidAsset,
-				sdk.NewAttribute(types.AttributeDenom, denom),
-			),
-		})
+		asset := k.GetSuperfluidAsset(ctx, denom)
+		dummyAsset := types.SuperfluidAsset{}
+		if asset == dummyAsset {
+			return fmt.Errorf("superfluid asset %s doesn't exist", denom)
+		}
+		k.BeginUnwindSuperfluidAsset(ctx, 0, asset)
+		event := sdk.NewEvent(
+			types.TypeEvtRemoveSuperfluidAsset,
+			sdk.NewAttribute(types.AttributeDenom, denom),
+		)
+		ctx.EventManager().EmitEvent(event)
 	}
 	return nil
 }

x/superfluid/keeper/gov_test.go

@@ -23,6 +23,7 @@ func (suite *KeeperTestSuite) TestHandleSetSuperfluidAssetsProposal() {
 		isAdd          bool
 		assets         []types.SuperfluidAsset
 		expectedAssets []types.SuperfluidAsset
+		expectErr      bool
 	}
 	testCases := []struct {
 		name    string
@@ -32,13 +33,21 @@ func (suite *KeeperTestSuite) TestHandleSetSuperfluidAssetsProposal() {
 			"happy path flow",
 			[]Action{
 				{
-					true, []types.SuperfluidAsset{asset1, asset2}, []types.SuperfluidAsset{asset1, asset2},
+					true, []types.SuperfluidAsset{asset1, asset2}, []types.SuperfluidAsset{asset1, asset2}, false,
 				},
 				{
-					false, []types.SuperfluidAsset{asset2}, []types.SuperfluidAsset{asset1},
+					false, []types.SuperfluidAsset{asset2}, []types.SuperfluidAsset{asset1}, false,
 				},
+			},
+		},
+		{
+			"token does not exist",
+			[]Action{
 				{
-					false, []types.SuperfluidAsset{asset3}, []types.SuperfluidAsset{asset1},
+					true, []types.SuperfluidAsset{asset1, asset2}, []types.SuperfluidAsset{asset1, asset2}, false,
+				},
+				{
+					false, []types.SuperfluidAsset{asset3}, []types.SuperfluidAsset{asset1, asset2}, true,
 				},
 			},
 		},
@@ -55,15 +64,14 @@ func (suite *KeeperTestSuite) TestHandleSetSuperfluidAssetsProposal() {
 			suite.Require().NoError(err)
 			suite.Require().Len(resp.Assets, 0)
 
-			for _, action := range tc.actions {
+			for i, action := range tc.actions {
 				if action.isAdd {
 					// set superfluid assets via proposal
 					err = suite.app.SuperfluidKeeper.HandleSetSuperfluidAssetsProposal(suite.ctx, &types.SetSuperfluidAssetsProposal{
 						Title:       "title",
 						Description: "description",
 						Assets:      action.assets,
 					})
-					suite.Require().NoError(err)
 				} else {
 					assetDenoms := []string{}
 					for _, asset := range action.assets {
@@ -75,14 +83,18 @@ func (suite *KeeperTestSuite) TestHandleSetSuperfluidAssetsProposal() {
 						Description:           "description",
 						SuperfluidAssetDenoms: assetDenoms,
 					})
+				}
+				if action.expectErr {
+					suite.Require().Error(err)
+				} else {
 					suite.Require().NoError(err)
 				}
 
 				// check assets individually
 				for _, asset := range action.expectedAssets {
 					res, err := suite.app.SuperfluidKeeper.AssetType(sdk.WrapSDKContext(suite.ctx), &types.AssetTypeRequest{Denom: asset.Denom})
 					suite.Require().NoError(err)
-					suite.Require().Equal(res.AssetType, asset.AssetType)
+					suite.Require().Equal(res.AssetType, asset.AssetType, "tcname %s, action num %d", tc.name, i)
 				}
 
 				// check assets

x/superfluid/keeper/hooks.go

@@ -1,6 +1,7 @@
 package keeper
 
 import (
+	"errors"
 	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -10,50 +11,72 @@ import (
 	"github.com/osmosis-labs/osmosis/x/superfluid/types"
 )
 
-func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, epochNumber int64) {
+func (k Keeper) BeforeEpochStart(ctx sdk.Context, epochIdentifier string, _ int64) {
 }
 
-func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumber int64) {
+func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, _ int64) {
 	params := k.GetParams(ctx)
 	if epochIdentifier == params.RefreshEpochIdentifier {
-		// Slash all module accounts' LP token based on slash amount before twap update
-		for _, asset := range k.GetAllSuperfluidAssets(ctx) {
-			if asset.AssetType == types.SuperfluidAssetTypeLPShare {
-				// LP_token_Osmo_equivalent = OSMO_amount_on_pool / LP_token_supply
-				poolId := gammtypes.MustGetPoolIdFromShareDenom(asset.Denom)
-				pool, err := k.gk.GetPool(ctx, poolId)
-				if err != nil {
-					k.Logger(ctx).Error(err.Error())
-					k.SetEpochOsmoEquivalentTWAP(ctx, epochNumber, asset.Denom, sdk.NewDec(0))
-					continue
-				}
-
-				// get OSMO amount
-				bondDenom := k.sk.BondDenom(ctx)
-				osmoPoolAsset, err := pool.GetPoolAsset(bondDenom)
-				if err != nil {
-					k.Logger(ctx).Error(err.Error())
-					k.SetEpochOsmoEquivalentTWAP(ctx, epochNumber, asset.Denom, sdk.NewDec(0))
-					continue
-				}
-
-				twap := osmoPoolAsset.Token.Amount.ToDec().Quo(pool.GetTotalShares().Amount.ToDec())
-				k.SetEpochOsmoEquivalentTWAP(ctx, epochNumber, asset.Denom, twap)
-			} else if asset.AssetType == types.SuperfluidAssetTypeNative {
-				// TODO: should get twap price from gamm module and use the price
-				// which pool should it use to calculate native token price?
-				k.Logger(ctx).Error("unsupported superfluid asset type")
-			}
-		}
+		// cref [#830](https://github.com/osmosis-labs/osmosis/issues/830),
+		// the supplied epoch number is wrong at time of commit. hence we get from the info.
+		endedEpochNumber := k.ek.GetEpochInfo(ctx, epochIdentifier).CurrentEpoch
 
 		// Move delegation rewards to perpetual gauge
 		k.MoveSuperfluidDelegationRewardToGauges(ctx)
 
-		// Refresh intermediary accounts' delegation amounts
+		// Update all LP tokens TWAP's for the upcoming epoch.
+		// This affects staking reward distribution until the next epochs rewards.
+		// Exclusive of current epoch's rewards, inclusive of next epoch's rewards.
+		for _, asset := range k.GetAllSuperfluidAssets(ctx) {
+			err := k.updateEpochTwap(ctx, asset, endedEpochNumber)
+			if err != nil {
+				// TODO: Revisit what we do here. (halt all distr, only skip this asset)
+				// Since at MVP of feature, we only have one pool of superfluid staking,
+				// we can punt this question.
+				// each of the errors feels like significant misconfig
+				return
+			}
+		}
+
+		// Refresh intermediary accounts' delegation amounts,
+		// making staking rewards follow the updated TWAP numbers.
 		k.RefreshIntermediaryDelegationAmounts(ctx)
 	}
 }
 
+func (k Keeper) updateEpochTwap(ctx sdk.Context, asset types.SuperfluidAsset, endedEpochNumber int64) error {
+	if asset.AssetType == types.SuperfluidAssetTypeLPShare {
+		// LP_token_Osmo_equivalent = OSMO_amount_on_pool / LP_token_supply
+		poolId := gammtypes.MustGetPoolIdFromShareDenom(asset.Denom)
+		pool, err := k.gk.GetPool(ctx, poolId)
+		if err != nil {
+			// Pool has been unexpectedly deleted
+			k.Logger(ctx).Error(err.Error())
+			k.BeginUnwindSuperfluidAsset(ctx, 0, asset)
+			return err
+		}
+
+		// get OSMO amount
+		bondDenom := k.sk.BondDenom(ctx)
+		osmoPoolAsset, err := pool.GetPoolAsset(bondDenom)
+		if err != nil {
+			// Pool has unexpectedly removed Osmo from its assets.
+			k.Logger(ctx).Error(err.Error())
+			k.BeginUnwindSuperfluidAsset(ctx, 0, asset)
+			return err
+		}
+
+		twap := k.calculateOsmoBackingPerShare(pool, osmoPoolAsset)
+		beginningEpochNumber := endedEpochNumber + 1
+		k.SetEpochOsmoEquivalentTWAP(ctx, beginningEpochNumber, asset.Denom, twap)
+	} else if asset.AssetType == types.SuperfluidAssetTypeNative {
+		// TODO: Consider deleting superfluid asset type native
+		k.Logger(ctx).Error("unsupported superfluid asset type")
+		return errors.New("SuperfluidAssetTypeNative is unspported")
+	}
+	return nil
+}
+
 // ___________________________________________________________________________________________________
 
 // Hooks wrapper struct for incentives keeper

x/superfluid/keeper/slash.go

@@ -12,8 +12,10 @@ func (k Keeper) SlashLockupsForUnbondingDelegationSlash(ctx sdk.Context, delAddr
 	if err != nil {
 		panic(err)
 	}
+
 	acc := k.GetIntermediaryAccount(ctx, delAddr)
-	if acc.Denom == "" { // if delAddr is not intermediary account, pass
+	// if delAddr is not intermediary account, pass
+	if acc.Denom == "" {
 		return
 	}
 

x/superfluid/keeper/superfluid_asset.go

@@ -0,0 +1,26 @@
+package keeper
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/osmosis-labs/osmosis/x/superfluid/types"
+)
+
+// BeginUnwindSuperfluidAsset starts the deletion process for a superfluid asset.
+// This current method is a stub, but is called when:
+// * Governance removes a superfluid asset
+// * A severe error in gamm occurs
+//
+// It should eventually begin unwinding all of the synthetic lockups for that asset
+// and queue them for deletion.
+// See https://github.com/osmosis-labs/osmosis/issues/864
+func (k Keeper) BeginUnwindSuperfluidAsset(ctx sdk.Context, epochNum int64, asset types.SuperfluidAsset) {
+	// Right now set the TWAP to 0, and delete the asset.
+	k.SetEpochOsmoEquivalentTWAP(ctx, epochNum, asset.Denom, sdk.ZeroDec())
+	k.DeleteSuperfluidAsset(ctx, asset.Denom)
+}
+
+func (k Keeper) GetRiskAdjustedOsmoValue(ctx sdk.Context, asset types.SuperfluidAsset, amount sdk.Int) sdk.Int {
+	// TODO: we need to figure out how to do this later.
+	minRiskFactor := k.GetParams(ctx).MinimumRiskFactor
+	return amount.Sub(amount.ToDec().Mul(minRiskFactor).RoundInt())
+}

x/superfluid/keeper/superfluid.go → x/superfluid/keeper/superfluid_asset_store.go

@@ -1,5 +1,7 @@
 package keeper
 
+// This file handles
+
 import (
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -57,9 +59,3 @@ func (k Keeper) GetAllSuperfluidAssets(ctx sdk.Context) []types.SuperfluidAsset
 	}
 	return assets
 }
-
-func (k Keeper) GetRiskAdjustedOsmoValue(ctx sdk.Context, asset types.SuperfluidAsset, amount sdk.Int) sdk.Int {
-	// TODO: we need to figure out how to do this later.
-	minRiskFactor := k.GetParams(ctx).MinimumRiskFactor
-	return amount.Sub(amount.ToDec().Mul(minRiskFactor).RoundInt())
-}

x/superfluid/keeper/twap_price.go

@@ -4,9 +4,18 @@ import (
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/gogo/protobuf/proto"
+	gammtypes "github.com/osmosis-labs/osmosis/x/gamm/types"
 	"github.com/osmosis-labs/osmosis/x/superfluid/types"
 )
 
+// This function calculates the osmo equivalent worth of an LP share.
+// It is intended to eventually use the TWAP of the worth of an LP share
+// once that is exposed from the gamm module.
+func (k Keeper) calculateOsmoBackingPerShare(pool gammtypes.PoolI, osmoInPool gammtypes.PoolAsset) sdk.Dec {
+	twap := osmoInPool.Token.Amount.ToDec().Quo(pool.GetTotalShares().Amount.ToDec())
+	return twap
+}
+
 func (k Keeper) SetEpochOsmoEquivalentTWAP(ctx sdk.Context, epoch int64, denom string, price sdk.Dec) {
 	store := ctx.KVStore(k.storeKey)
 	prefixStore := prefix.NewStore(store, types.KeyPrefixTokenPriceTwap)

x/superfluid/spec/01_concepts.md

@@ -4,3 +4,21 @@ order: 1
 
 # Concepts
 
+Lets fill out this spec, and think about the entire state machine.
+
+Its a state machine which has inputs:
+
+* Messages
+* Parameters / param changes
+* Epoch
+* Slashes
+* Gov props to add/remove superfluid assets
+* ??? Staking weight changes? (Validators going inactive)
+
+It has state:
+
+* Superfluid assets
+* Intermediary Accounts
+* Dedicated Gauges
+* Synthetic Locks
+* Historical osmo equivalent twaps?

x/superfluid/spec/02_state.md

@@ -4,6 +4,29 @@ order: 2
 
 # State
 
+## SuperfluidAsset
+
+A superfluid asset is ...
+
+It can only be updated by governance proposals. We validate at proposal creation time that the denom + pool exists.
+(Are we going to ignore edge cases around a reference pool getting deleted it)
+
+## Intermediary Accounts
+
+Lots of questions to be answered here
+
+## Synthetic Lockups created
+
+Many questions to be answered here
+
+## TWAP
+
+** Clarify what we mean by TWAP. Namely the time-weighted average of the osmo backing of an LP share.
+
+The TWAP for an epoch is set at the beginning of the epoch. We refer to the TWAP for use in epoch N as the 24hr TWAP for the entire prior epoch (N-1). This is then set at the beginning of epoch N.
+
+Note, that we don't actually use the TWAP. We just use the spot price at the start of epoch N for now.
+
 ## Latest OSMO equivalent TWAP
 
 Snapshot of OSMO equivalent that is updated at the end of epoch, can be queried by `GetLatestEpochOsmoEquivalentTWAP`.

x/superfluid/spec/04_epoch.md

@@ -0,0 +1,15 @@
+<!--
+order: 4
+-->
+
+# Epochs
+
+At the Osmosis rewards distribution epoch time, all superfluid staking rewards get distributed.
+The envisioned flow of how this works is as follows:
+
+* (Epochs) AfterEpochEnd hook runs for epoch N
+* (Mint) distributes rewards to all stakers at the epoch that just endeds prices
+* (Superfluid) Claim all staking rewards to every intermediary module accounts
+* (Superfluid) Update all TWAP values [updateEpochEnd][./../keeper/hooks.go]
+  * Here we are setting the TWAP value for epoch N+1, as the TWAP from the duration of epoch N.
+* (Superfluid) Update all the intermediary accounts staked amounts. (Mint/Burn coins as needed as well)

x/superfluid/spec/04_events.md → x/superfluid/spec/05_events.md

@@ -1,5 +1,5 @@
 <!--
-order: 4
+order: 5
 -->
 
 # Events