From 59488ebf5a8716b43a70b8b376116f6ef3339620 Mon Sep 17 00:00:00 2001 From: RafilxTenfen Date: Thu, 19 Dec 2024 17:42:52 -0300 Subject: [PATCH] chore: reduced duplicated code and update comments --- test/e2e/btc_rewards_distribution_e2e_test.go | 10 ---- test/e2e/configurer/chain/chain.go | 4 -- x/finality/keeper/power_dist_change.go | 57 ++++++++++--------- x/incentive/keeper/btc_staking_gauge_test.go | 1 - x/incentive/keeper/reward_tracker.go | 20 ++++--- x/incentive/keeper/reward_tracker_store.go | 3 +- 6 files changed, 43 insertions(+), 52 deletions(-) diff --git a/test/e2e/btc_rewards_distribution_e2e_test.go b/test/e2e/btc_rewards_distribution_e2e_test.go index 9c3a47107..d66388c9f 100644 --- a/test/e2e/btc_rewards_distribution_e2e_test.go +++ b/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) diff --git a/test/e2e/configurer/chain/chain.go b/test/e2e/configurer/chain/chain.go index d27ce0c76..76ac1f72c 100644 --- a/test/e2e/configurer/chain/chain.go +++ b/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 } diff --git a/x/finality/keeper/power_dist_change.go b/x/finality/keeper/power_dist_change.go index 3e4af4e7a..cfed42b5d 100644 --- a/x/finality/keeper/power_dist_change.go +++ b/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) } } diff --git a/x/incentive/keeper/btc_staking_gauge_test.go b/x/incentive/keeper/btc_staking_gauge_test.go index d3aa72494..526e02971 100644 --- a/x/incentive/keeper/btc_staking_gauge_test.go +++ b/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) diff --git a/x/incentive/keeper/reward_tracker.go b/x/incentive/keeper/reward_tracker.go index 06e8ca305..afee4994b 100644 --- a/x/incentive/keeper/reward_tracker.go +++ b/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 diff --git a/x/incentive/keeper/reward_tracker_store.go b/x/incentive/keeper/reward_tracker_store.go index 65de0413b..1414bf46b 100644 --- a/x/incentive/keeper/reward_tracker_store.go +++ b/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