chore: reduced duplicated code and update comments

test/e2e/btc_rewards_distribution_e2e_test.go

@@ -227,9 +227,6 @@ func (s *BtcRewardsDistribution) Test4CommitPublicRandomnessAndSealed() {
 	n2, err := chainA.GetNodeAtIndex(2)
 	s.NoError(err)
 
-	/*
-		commit a number of public randomness
-	*/
 	// commit public randomness list
 	commitStartHeight := uint64(1)
 
@@ -259,7 +256,6 @@ func (s *BtcRewardsDistribution) Test4CommitPublicRandomnessAndSealed() {
 
 	n1.WaitUntilCurrentEpochIsSealedAndFinalized(1)
 
-	// activated height is never returned
 	s.finalityBlockHeightVoted = n1.WaitFinalityIsActivated()
 
 	// submit finality signature
@@ -403,12 +399,6 @@ func (s *BtcRewardsDistribution) Test7CheckRewards() {
 	// (fp2)  => 8_00000000
 	// (del1) => 4_00000000
 	// (del2) => 10_00000000
-
-	// Current rewards for each addr
-	// fp1 ~5364ubbn
-	// fp2 ~1787ubbn
-	// del1 ~11625ubbn
-	// del2 ~16989ubbn
 	fp1RewardGaugePrev, fp2RewardGaugePrev, btcDel1RewardGaugePrev, btcDel2RewardGaugePrev := s.QueryRewardGauges(n2)
 	// wait a few block of rewards to calculate the difference
 	n2.WaitForNextBlocks(2)

test/e2e/configurer/chain/chain.go

@@ -3,7 +3,6 @@ package chain
 import (
 	"encoding/hex"
 	"fmt"
-	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -77,9 +76,6 @@ func (c *Config) CreateNode(initNode *initialization.Node) *NodeConfig {
 		t:                c.t,
 	}
 	c.NodeConfigs = append(c.NodeConfigs, nodeConfig)
-	sort.Slice(c.NodeConfigs, func(i, j int) bool {
-		return c.NodeConfigs[i].Name > c.NodeConfigs[j].Name
-	})
 	return nodeConfig
 }
 

x/finality/keeper/power_dist_change.go

@@ -187,7 +187,7 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents(
 	unjailedFPs := map[string]struct{}{}
 
 	// simple cache to load fp by his btc pk hex
-	cacheFpByBtcPkHex := map[string]*types.FinalityProvider{}
+	fpByBtcPkHex := map[string]*types.FinalityProvider{}
 
 	/*
 		filter and classify all events into new/expired BTC delegations and jailed/slashed FPs
@@ -213,39 +213,30 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents(
 					activedSatsByFpBtcPk[fpBTCPKHex] = append(activedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat)
 				}
 
-				k.processRewardTracker(ctx, cacheFpByBtcPkHex, btcDel, func(fp, del sdk.AccAddress, sats uint64) {
+				k.processRewardTracker(ctx, fpByBtcPkHex, btcDel, func(fp, del sdk.AccAddress, sats uint64) {
 					k.MustProcessBtcDelegationActivated(ctx, fp, del, sats)
 				})
 			case types.BTCDelegationStatus_UNBONDED:
 				// add the unbonded BTC delegation to the map
-				// unbondedBTCDels[delStkTxHash] = struct{}{}
-				for _, fpBTCPK := range btcDel.FpBtcPkList {
-					fpBTCPKHex := fpBTCPK.MarshalHex()
-					unbondedSatsByFpBtcPk[fpBTCPKHex] = append(unbondedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat)
-				}
-				k.processRewardTracker(ctx, cacheFpByBtcPkHex, btcDel, func(fp, del sdk.AccAddress, sats uint64) {
-					k.MustProcessBtcDelegationUnbonded(ctx, fp, del, sats)
-				})
+				k.processPowerDistUpdateEventUnbond(ctx, fpByBtcPkHex, btcDel, unbondedSatsByFpBtcPk)
 			case types.BTCDelegationStatus_EXPIRED:
 				types.EmitExpiredDelegationEvent(sdkCtx, delStkTxHash)
 
 				if !btcDel.IsUnbondedEarly() {
 					// only adds to the new unbonded list if it hasn't
 					// previously unbonded with types.BTCDelegationStatus_UNBONDED
-					// unbondedBTCDels[delStkTxHash] = struct{}{}
-					for _, fpBTCPK := range btcDel.FpBtcPkList {
-						fpBTCPKHex := fpBTCPK.MarshalHex()
-						unbondedSatsByFpBtcPk[fpBTCPKHex] = append(unbondedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat)
-					}
-					k.processRewardTracker(ctx, cacheFpByBtcPkHex, btcDel, func(fp, del sdk.AccAddress, sats uint64) {
-						k.MustProcessBtcDelegationUnbonded(ctx, fp, del, sats)
-					})
+					k.processPowerDistUpdateEventUnbond(ctx, fpByBtcPkHex, btcDel, unbondedSatsByFpBtcPk)
 				}
 			}
 		case *types.EventPowerDistUpdate_SlashedFp:
 			// record slashed fps
 			types.EmitSlashedFPEvent(sdkCtx, typedEvent.SlashedFp.Pk)
-			slashedFPs[typedEvent.SlashedFp.Pk.MarshalHex()] = struct{}{}
+			fpBTCPKHex := typedEvent.SlashedFp.Pk.MarshalHex()
+			slashedFPs[fpBTCPKHex] = struct{}{}
+			fp := k.loadFP(ctx, fpByBtcPkHex, fpBTCPKHex)
+			if err := k.IncentiveKeeper.FpSlashed(ctx, fp.Address()); err != nil {
+				panic(err)
+			}
 		case *types.EventPowerDistUpdate_JailedFp:
 			// record jailed fps
 			types.EmitJailedFPEvent(sdkCtx, typedEvent.JailedFp.Pk)
@@ -276,9 +267,6 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents(
 		// assigning delegation to it
 		_, isSlashed := slashedFPs[fpBTCPKHex]
 		if isSlashed {
-			if err := k.IncentiveKeeper.FpSlashed(ctx, fp.GetAddress()); err != nil {
-				panic(err)
-			}
 			fp.IsSlashed = true
 			continue
 		}
@@ -341,7 +329,7 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents(
 	// for each new finality provider, apply the new BTC delegations to the new dist cache
 	for _, fpBTCPKHex := range fpActiveBtcPkHexList {
 		// get the finality provider and initialise its dist info
-		newFP := k.loadFP(ctx, cacheFpByBtcPkHex, fpBTCPKHex)
+		newFP := k.loadFP(ctx, fpByBtcPkHex, fpBTCPKHex)
 		fpDistInfo := ftypes.NewFinalityProviderDistInfo(newFP)
 
 		// add each BTC delegation
@@ -369,6 +357,21 @@ func (k Keeper) ProcessAllPowerDistUpdateEvents(
 	return newDc
 }
 
+func (k Keeper) processPowerDistUpdateEventUnbond(
+	ctx context.Context,
+	cacheFpByBtcPkHex map[string]*types.FinalityProvider,
+	btcDel *types.BTCDelegation,
+	unbondedSatsByFpBtcPk map[string][]uint64,
+) {
+	for _, fpBTCPK := range btcDel.FpBtcPkList {
+		fpBTCPKHex := fpBTCPK.MarshalHex()
+		unbondedSatsByFpBtcPk[fpBTCPKHex] = append(unbondedSatsByFpBtcPk[fpBTCPKHex], btcDel.TotalSat)
+	}
+	k.processRewardTracker(ctx, cacheFpByBtcPkHex, btcDel, func(fp, del sdk.AccAddress, sats uint64) {
+		k.MustProcessBtcDelegationUnbonded(ctx, fp, del, sats)
+	})
+}
+
 func (k Keeper) SetVotingPowerDistCache(ctx context.Context, height uint64, dc *ftypes.VotingPowerDistCache) {
 	store := k.votingPowerDistCacheStore(ctx)
 	store.Set(sdk.Uint64ToBigEndian(height), k.cdc.MustMarshal(dc))
@@ -404,16 +407,14 @@ func (k Keeper) votingPowerDistCacheStore(ctx context.Context) prefix.Store {
 // and satoshi amounts.
 func (k Keeper) processRewardTracker(
 	ctx context.Context,
-	cacheFpByBtcPkHex map[string]*types.FinalityProvider,
+	fpByBtcPkHex map[string]*types.FinalityProvider,
 	btcDel *types.BTCDelegation,
 	f func(fp, del sdk.AccAddress, sats uint64),
 ) {
 	delAddr := sdk.MustAccAddressFromBech32(btcDel.StakerAddr)
 	for _, fpBTCPK := range btcDel.FpBtcPkList {
-		fp := k.loadFP(ctx, cacheFpByBtcPkHex, fpBTCPK.MarshalHex())
-		fpAddr := sdk.MustAccAddressFromBech32(fp.Addr)
-
-		f(fpAddr, delAddr, btcDel.TotalSat)
+		fp := k.loadFP(ctx, fpByBtcPkHex, fpBTCPK.MarshalHex())
+		f(fp.Address(), delAddr, btcDel.TotalSat)
 	}
 }
 

x/incentive/keeper/btc_staking_gauge_test.go

@@ -24,7 +24,6 @@ func FuzzRewardBTCStaking(f *testing.F) {
 		// mock bank keeper
 		bankKeeper := types.NewMockBankKeeper(ctrl)
 
-		// create incentive k
 		k, ctx := testkeeper.IncentiveKeeper(t, bankKeeper, nil, nil)
 		height := datagen.RandomInt(r, 1000)
 		ctx = datagen.WithCtxHeight(ctx, height)

x/incentive/keeper/reward_tracker.go

@@ -247,7 +247,7 @@ func (k Keeper) IncrementFinalityProviderPeriod(ctx context.Context, fp sdk.AccA
 		return 0, err
 	}
 
-	// initiates a new period with empty rewards and the same amount of active sat (this value should be updated latter if needed)
+	// initiates a new period with empty rewards and the same amount of active sat
 	newCurrentRwd := types.NewFinalityProviderCurrentRewards(sdk.NewCoins(), fpCurrentRwd.Period+1, fpCurrentRwd.TotalActiveSat)
 	if err := k.setFinalityProviderCurrentRewards(ctx, fp, newCurrentRwd); err != nil {
 		return 0, err
@@ -257,7 +257,7 @@ func (k Keeper) IncrementFinalityProviderPeriod(ctx context.Context, fp sdk.AccA
 }
 
 // initializeFinalityProvider initializes a new finality provider current rewards at period 1, empty rewards and zero sats
-// and also a historical rewards at period 0 and zero rewards as well.
+// and also creates a new historical rewards at period 0 and zero rewards as well.
 // It does not verifies if it exists prior to overwrite, who calls it needs to verify.
 func (k Keeper) initializeFinalityProvider(ctx context.Context, fp sdk.AccAddress) (types.FinalityProviderCurrentRewards, error) {
 	// historical rewards starts at the period 0
@@ -271,14 +271,18 @@ func (k Keeper) initializeFinalityProvider(ctx context.Context, fp sdk.AccAddres
 	return newFp, k.setFinalityProviderCurrentRewards(ctx, fp, newFp)
 }
 
-// initializeBTCDelegation creates a new BTCDelegationRewardsTracker from the previous acumulative rewards
-// period of the finality provider and it should be called right after a BTC delegator withdraw his rewards
-// (in our case send the rewards to the reward gauge). Reminder that at every new modification to the amount
-// of satoshi staked from this btc delegator to this finality provider (activivation or unbonding) of BTC
-// delegations, it should withdraw all rewards (send to gauge) and initialize a new BTCDelegationRewardsTracker.
+// initializeBTCDelegation creates a new BTCDelegationRewardsTracker from the
+// previous acumulative rewards period of the finality provider. This function
+// should be called right after a BTC delegator withdraw his rewards (in our
+// case send the rewards to the reward gauge). Reminder that at every new
+// modification to the amount of satoshi staked from this btc delegator to
+// this finality provider (activivation or unbonding) of BTC delegations, it
+// should withdraw all rewards (send to gauge) and initialize a new BTCDelegationRewardsTracker.
 // TODO: add reference count to keep track of possible prunning state of val rewards
 func (k Keeper) initializeBTCDelegation(ctx context.Context, fp, del sdk.AccAddress) error {
-	// period has already been incremented - we want to store the period ended by this delegation action
+	// period has already been incremented prior to call this function
+	// it is needed to store the period ended by this delegation action
+	// as a starting point of the delegation rewards calculation
 	valCurrentRewards, err := k.GetFinalityProviderCurrentRewards(ctx, fp)
 	if err != nil {
 		return err

x/incentive/keeper/reward_tracker_store.go

@@ -272,7 +272,8 @@ func (k Keeper) addFinalityProviderStaked(ctx context.Context, fp sdk.AccAddress
 			return err
 		}
 
-		// this is needed as the amount of sats for the FP is inside the FpCurrentRewards
+		// needs to initialize at this point due to the amount of
+		// sats for the FP is inside the FinalityProviderCurrentRewards
 		fpCurrentRwd, err = k.initializeFinalityProvider(ctx, fp)
 		if err != nil {
 			return err