v0.21.1 - correct upgrade handler (#268)

* dev: only remove negative delegations in store, improve logging * chore: bump various deps * dev: remove polytone and cosmwasm conformance (will replace & tested using integration) * chore: bump ict deps * chore: ict core application coverage * chore: ict deps * chore: ict deps * chore: ict deps * chore: ict bump * chore: remove ci ict test (will be added back in future) --------- Co-authored-by: hard-nett <[email protected]>
bitsongofficial · Feb 13, 2025 · f18e2ae · f18e2ae
1 parent 8928250
commit f18e2ae
Show file tree

Hide file tree

Showing 15 changed files with 2,777 additions and 1,395 deletions.
diff --git a/.github/workflows/interchaintest-e2e.yml b/.github/workflows/interchaintest-e2e.yml
@@ -60,7 +60,7 @@ jobs:
         test:
           - "e2e-basic"
           - "e2e-pfm"
-          - "e2e-polytone"
+          # - "e2e-polytone"
           # - "e2e-upgrade"
       fail-fast: false
 

diff --git a/app/upgrades/v020/upgrades.go b/app/upgrades/v020/upgrades.go
@@ -124,37 +124,45 @@ func V018ManualDelegationRewardsPatch(sdkCtx sdk.Context, rewardsRaw, outstandin
 	// add coins to user account
 	if !finalRewards.IsZero() {
 		withdrawAddr, err := k.DistrKeeper.GetDelegatorWithdrawAddr(sdkCtx, sdk.AccAddress(del.GetDelegatorAddr()))
+		if err != nil {
+			return err
+		}
 		err = k.BankKeeper.SendCoinsFromModuleToAccount(sdkCtx, distrtypes.ModuleName, withdrawAddr, finalRewards)
 		if err != nil {
 			return err
 		}
+		sdkCtx.Logger().Info(fmt.Sprintf("Rewards %v manually claimed for: %q", finalRewards, del.GetDelegatorAddr()))
 	}
 
 	// update the outstanding rewards and the community pool only if the
 	// transaction was successful
 	k.DistrKeeper.SetValidatorOutstandingRewards(sdkCtx, sdk.ValAddress(valAddr), distrtypes.ValidatorOutstandingRewards{Rewards: outstanding.Sub(rewards)})
-	feePool, _ := k.DistrKeeper.FeePool.Get(sdkCtx)
+	feePool, err := k.DistrKeeper.FeePool.Get(sdkCtx)
+	if err != nil {
+		return err
+	}
 	feePool.CommunityPool = feePool.CommunityPool.Add(remainder...)
-	k.DistrKeeper.FeePool.Set(sdkCtx, feePool)
+	err = k.DistrKeeper.FeePool.Set(sdkCtx, feePool)
+	if err != nil {
+		return err
+	}
 
 	// decrement reference count of starting period
-	startingInfo, _ := k.DistrKeeper.GetDelegatorStartingInfo(sdkCtx, sdk.ValAddress(del.GetValidatorAddr()), sdk.AccAddress(del.GetDelegatorAddr()))
+	startingInfo, err := k.DistrKeeper.GetDelegatorStartingInfo(sdkCtx, sdk.ValAddress(del.GetValidatorAddr()), sdk.AccAddress(del.GetDelegatorAddr()))
+	if err != nil {
+		return err
+	}
 	startingPeriod := startingInfo.PreviousPeriod
 	customDecrementReferenceCount(sdkCtx, k, sdk.ValAddress(del.GetValidatorAddr()), startingPeriod)
 
 	// remove delegator starting info
 	k.DistrKeeper.DeleteDelegatorStartingInfo(sdkCtx, sdk.ValAddress(del.GetValidatorAddr()), sdk.AccAddress(del.GetDelegatorAddr()))
 
 	if finalRewards.IsZero() {
-		baseDenom, _ := sdk.GetBaseDenom()
-		if baseDenom == "" {
-			baseDenom = sdk.DefaultBondDenom
-		}
-
 		// Note, we do not call the NewCoins constructor as we do not want the zero
 		// coin removed.
-		finalRewards = sdk.Coins{sdk.NewCoin(baseDenom, math.ZeroInt())}
-		sdkCtx.Logger().Info("No final rewards", finalRewards)
+		sdkCtx.Logger().Info("~=~=~=~=~~=~=~=~=~~=~=~=~=~~=~=~=~=~~=~=~=~=~~=~=~=~=~~=~=~=~=~~=~=~=~=~~=~=~=~=~~=~=~=~=~")
+		sdkCtx.Logger().Info(fmt.Sprintf("No final rewards: %q %v", val.GetOperator(), del.GetDelegatorAddr()))
 	}
 
 	// reinitialize the delegation

diff --git a/app/upgrades/v021/upgrade_test.go b/app/upgrades/v021/upgrade_test.go
@@ -12,7 +12,6 @@ import (
 	upgradetypes "cosmossdk.io/x/upgrade/types"
 	apptesting "github.com/bitsongofficial/go-bitsong/app/testing"
 	addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
-	distrtypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 
 	sdktypes "github.com/cosmos/cosmos-sdk/types"
@@ -38,13 +37,13 @@ func TestUpgradeTestSuite(t *testing.T) {
 func (s *UpgradeTestSuite) TestUpgrade() {
 	upgradeSetup := func(shares, slash math.LegacyDec, jailed bool) {
 		vals, _ := s.App.AppKeepers.StakingKeeper.GetAllValidators(s.Ctx)
+		fmt.Printf("# OF VALS: %d\n", len(vals))
 		for _, val := range vals {
-			// save delegation to distribution store
-			err := s.App.AppKeepers.DistrKeeper.SetDelegatorStartingInfo(s.Ctx, sdktypes.ValAddress(val.OperatorAddress), s.TestAccs[0], distrtypes.NewDelegatorStartingInfo(1, shares, uint64(s.Ctx.BlockHeight())))
-			s.Require().NoError(err)
+			// delAddrStr, err := s.App.AppKeepers.AccountKeeper.AddressCodec().BytesToString(s.TestAccs[0])
 
-			// create delegation with smallest non 0 value
-			s.App.AppKeepers.StakingKeeper.SetDelegation(s.Ctx, stakingtypes.NewDelegation(s.TestAccs[0].String(), val.OperatorAddress, shares))
+			s.FundAcc(s.TestAccs[0], sdktypes.NewCoins(sdktypes.NewCoin("stake", math.NewInt(1000000))))
+			_, err := s.App.AppKeepers.StakingKeeper.Delegate(s.Ctx, s.TestAccs[0], math.NewInt(1000000), stakingtypes.Unbonded, val, true)
+			s.Require().NoError(err)
 
 			if !slash.IsZero() {
 				val.Tokens = math.LegacyNewDecFromInt(val.Tokens).MulTruncate(math.LegacyOneDec().Sub(slash)).RoundInt() // 1 % slash
@@ -54,6 +53,13 @@ func (s *UpgradeTestSuite) TestUpgrade() {
 			}
 			err = s.App.AppKeepers.StakingKeeper.SetValidator(s.Ctx, val)
 			s.Require().NoError(err)
+
+			// get delegations
+			dels, err := s.App.AppKeepers.StakingKeeper.GetValidatorDelegations(s.Ctx, sdktypes.ValAddress(val.OperatorAddress))
+			s.Require().NoError(err)
+			fmt.Printf("# OF DELS: %d\n", len(dels))
+
+			// todo: fix staking helper to propoerly manage staking in simulation tests
 		}
 
 	}
@@ -67,7 +73,7 @@ func (s *UpgradeTestSuite) TestUpgrade() {
 		{
 			"test app module params",
 			func() {
-				upgradeSetup(math.LegacyOneDec(), math.LegacyZeroDec(), true)
+				upgradeSetup(math.LegacyOneDec(), math.LegacyZeroDec(), false)
 			},
 			func() {
 				dummyUpgrade(s)

diff --git a/app/upgrades/v021/upgrades.go b/app/upgrades/v021/upgrades.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"time"
 
-	"cosmossdk.io/errors"
 	"cosmossdk.io/math"
 	upgradetypes "cosmossdk.io/x/upgrade/types"
 	"github.com/bitsongofficial/go-bitsong/app/keepers"
@@ -15,7 +14,6 @@ import (
 	sca "github.com/bitsongofficial/go-bitsong/x/smart-account/types"
 	cmtproto "github.com/cometbft/cometbft/proto/tendermint/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkErr "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/types/module"
 	icqkeeper "github.com/cosmos/ibc-apps/modules/async-icq/v8/keeper"
 	wasmlctypes "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
@@ -40,29 +38,32 @@ func CreateV021UpgradeHandler(mm *module.Manager, configurator module.Configurat
 		for _, val := range vals {
 			valAddr := sdk.ValAddress(val.OperatorAddress)
 			dels, _ := k.StakingKeeper.GetValidatorDelegations(sdkCtx, valAddr)
-
 			for _, del := range dels {
-				if del.Shares.LTE(math.LegacyNewDecWithPrec(13, -17)) {
+				if del.Shares.LTE(math.LegacyZeroDec()) {
+					sdkCtx.Logger().Info(fmt.Sprintf("removing negative delegation from store: %q %v", val.GetOperator(), del.GetDelegatorAddr())) // remove reward information from distribution store
+					if exists, _ := k.DistrKeeper.HasDelegatorStartingInfo(sdkCtx, valAddr, sdk.AccAddress(del.DelegatorAddress)); exists {
+						sdkCtx.Logger().Info("delegation info found, deleting...")
+						if err := k.DistrKeeper.DeleteDelegatorStartingInfo(sdkCtx, valAddr, sdk.AccAddress(del.DelegatorAddress)); err != nil {
+							return nil, err
+						}
+						sdkCtx.Logger().Info("removed negative delegation from store successfully!")
+					}
 					// remove delegation from staking store
+					sdkCtx.Logger().Info("~-~-~~-~-~~-~-~~-~-~~-~-~~-~-~~-~-~")
+					sdkCtx.Logger().Info("removing negative delegation from staking keeper store")
 					if err := k.StakingKeeper.RemoveDelegation(sdkCtx, del); err != nil {
 						return nil, err
-					}
-					// remove reward information from distribution store
-					if exists, err := k.DistrKeeper.HasDelegatorStartingInfo(sdkCtx, valAddr, sdk.AccAddress(del.DelegatorAddress)); err != nil {
-						return nil, err
-					} else if !exists {
-						return nil, errors.Wrapf(sdkErr.ErrNotFound, "delegator starting info not found for delegator %s", del.DelegatorAddress)
-					}
-					if err := k.DistrKeeper.DeleteDelegatorStartingInfo(sdkCtx, valAddr, sdk.AccAddress(del.DelegatorAddress)); err != nil {
-						return nil, err
+					} else {
+						sdkCtx.Logger().Info("removed negative delegation store value successfully!")
 					}
 				} else {
-					// check if we need to patch distribution by manually claiming rewards again
+					// check if we need to patch distribution by manually claiming rewards for any impaced delegations once again...
 					hasInfo, err := k.DistrKeeper.HasDelegatorStartingInfo(sdkCtx, sdk.ValAddress(valAddr), sdk.AccAddress(del.GetDelegatorAddr()))
 					if err != nil {
 						return nil, err
 					}
 					if !hasInfo {
+						sdkCtx.Logger().Info(fmt.Sprintf("delegation does not have starting info: val: %q, del: %v", val.GetOperator(), del.GetDelegatorAddr()))
 						continue
 					}
 					// calculate rewards
@@ -85,7 +86,28 @@ func CreateV021UpgradeHandler(mm *module.Manager, configurator module.Configurat
 					}
 				}
 			}
+		}
 
+		/*  ensure no delegations exist without starting info*/
+		allDels, err := k.StakingKeeper.GetAllDelegations(sdkCtx)
+		if err != nil {
+			panic(err)
+		}
+		for _, del := range allDels {
+			/* ensure all rewards are patched */
+			val, err := k.StakingKeeper.Validator(sdkCtx, sdk.ValAddress(del.GetValidatorAddr()))
+			if err != nil {
+				panic(err)
+			}
+			endingPeriod, err := k.DistrKeeper.IncrementValidatorPeriod(sdkCtx, val)
+			if err != nil {
+				panic(err)
+			}
+			/* will error if still broken */
+			_, err = k.DistrKeeper.CalculateDelegationRewards(sdkCtx, val, del, endingPeriod)
+			if err != nil {
+				panic(err)
+			}
 		}
 
 		// setup vote extension