From 06fc76c46690e9056bcdc9e0d90d238475742adf Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Mon, 5 Feb 2024 13:18:33 -0600 Subject: [PATCH] De-duplicate fetching intermediate accounts (#7411) * De-duplicate fetching intermediate accounts * Fix build * De-duplicate BondDenom fetch (cherry picked from commit fda7971b9a39c1a1f2b327edff5e99d130fb8577) --- x/incentives/keeper/distribute.go | 2 +- x/superfluid/keeper/epoch.go | 13 +++++++------ x/superfluid/keeper/epoch_test.go | 6 ++++-- x/superfluid/keeper/hooks.go | 3 ++- x/superfluid/keeper/stake.go | 3 +-- x/superfluid/keeper/stake_test.go | 6 ++++-- 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index 22240ff2c46..4c27d926f8d 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -799,7 +799,7 @@ func (k Keeper) Distribute(ctx sdk.Context, gauges []types.Gauge) (sdk.Coins, er locksByDenomCache := make(map[string][]lockuptypes.PeriodLock) totalDistributedCoins := sdk.NewCoins() - scratchSlice := make([]*lockuptypes.PeriodLock, 0, 10000) + scratchSlice := make([]*lockuptypes.PeriodLock, 0, 50000) for _, gauge := range gauges { var gaugeDistributedCoins sdk.Coins diff --git a/x/superfluid/keeper/epoch.go b/x/superfluid/keeper/epoch.go index 2f4e5ac1a52..017ccb506be 100644 --- a/x/superfluid/keeper/epoch.go +++ b/x/superfluid/keeper/epoch.go @@ -27,9 +27,10 @@ func (k Keeper) AfterEpochStartBeginBlock(ctx sdk.Context) { // the supplied epoch number is wrong at time of commit. hence we get from the info. curEpoch := k.ek.GetEpochInfo(ctx, k.GetEpochIdentifier(ctx)).CurrentEpoch + accs := k.GetAllIntermediaryAccounts(ctx) // Move delegation rewards to perpetual gauge ctx.Logger().Info("Move delegation rewards to gauges") - k.MoveSuperfluidDelegationRewardToGauges(ctx) + k.MoveSuperfluidDelegationRewardToGauges(ctx, accs) ctx.Logger().Info("Distribute Superfluid gauges") //nolint:errcheck @@ -56,11 +57,11 @@ func (k Keeper) AfterEpochStartBeginBlock(ctx sdk.Context) { // Refresh intermediary accounts' delegation amounts, // making staking rewards follow the updated multiplier numbers. ctx.Logger().Info("Refresh all superfluid delegation amounts") - k.RefreshIntermediaryDelegationAmounts(ctx) + k.RefreshIntermediaryDelegationAmounts(ctx, accs) } -func (k Keeper) MoveSuperfluidDelegationRewardToGauges(ctx sdk.Context) { - accs := k.GetAllIntermediaryAccounts(ctx) +func (k Keeper) MoveSuperfluidDelegationRewardToGauges(ctx sdk.Context, accs []types.SuperfluidIntermediaryAccount) { + bondDenom := k.sk.BondDenom(ctx) for _, acc := range accs { addr := acc.GetAccAddress() valAddr, err := sdk.ValAddressFromBech32(acc.ValAddr) @@ -74,7 +75,8 @@ func (k Keeper) MoveSuperfluidDelegationRewardToGauges(ctx sdk.Context) { _, err := k.ck.WithdrawDelegationRewards(cacheCtx, addr, valAddr) if errors.Is(err, distributiontypes.ErrEmptyDelegationDistInfo) { ctx.Logger().Debug("no swaps occurred in this pool between last epoch and this epoch") - return nil + } else if err != nil { + return err } return err }) @@ -83,7 +85,6 @@ func (k Keeper) MoveSuperfluidDelegationRewardToGauges(ctx sdk.Context) { _ = osmoutils.ApplyFuncIfNoError(ctx, func(cacheCtx sdk.Context) error { // Note! We only send the bond denom (osmo), to avoid attack vectors where people // send many different denoms to the intermediary account, and make a resource exhaustion attack on end block. - bondDenom := k.sk.BondDenom(cacheCtx) balance := k.bk.GetBalance(cacheCtx, addr, bondDenom) if balance.IsZero() { return nil diff --git a/x/superfluid/keeper/epoch_test.go b/x/superfluid/keeper/epoch_test.go index 62a48218fe0..db512618e51 100644 --- a/x/superfluid/keeper/epoch_test.go +++ b/x/superfluid/keeper/epoch_test.go @@ -215,7 +215,8 @@ func (s *KeeperTestSuite) TestMoveSuperfluidDelegationRewardToGauges() { } // move intermediary account delegation rewards to gauges - s.App.SuperfluidKeeper.MoveSuperfluidDelegationRewardToGauges(s.Ctx) + accs := s.App.SuperfluidKeeper.GetAllIntermediaryAccounts(s.Ctx) + s.App.SuperfluidKeeper.MoveSuperfluidDelegationRewardToGauges(s.Ctx, accs) // check invariant is fine reason, broken := keeper.AllInvariants(*s.App.SuperfluidKeeper)(s.Ctx) @@ -292,7 +293,8 @@ func (s *KeeperTestSuite) TestDistributeSuperfluidGauges() { } // move intermediary account delegation rewards to gauges - s.App.SuperfluidKeeper.MoveSuperfluidDelegationRewardToGauges(s.Ctx) + accs := s.App.SuperfluidKeeper.GetAllIntermediaryAccounts(s.Ctx) + s.App.SuperfluidKeeper.MoveSuperfluidDelegationRewardToGauges(s.Ctx, accs) // move gauges to active gauge by declaring epoch end s.Ctx = s.Ctx.WithBlockTime(s.Ctx.BlockTime().Add(time.Minute)) diff --git a/x/superfluid/keeper/hooks.go b/x/superfluid/keeper/hooks.go index 9f4f300b183..28e6929d84e 100644 --- a/x/superfluid/keeper/hooks.go +++ b/x/superfluid/keeper/hooks.go @@ -115,5 +115,6 @@ func (h Hooks) AfterValidatorSlashed(ctx sdk.Context, valAddr sdk.ValAddress, in if slashFactor.IsZero() { return } - h.k.RefreshIntermediaryDelegationAmounts(ctx) + accs := h.k.GetAllIntermediaryAccounts(ctx) + h.k.RefreshIntermediaryDelegationAmounts(ctx, accs) } diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index d4c08ac75fd..beb2832eef0 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -45,9 +45,8 @@ func (k Keeper) GetExpectedDelegationAmount(ctx sdk.Context, acc types.Superflui // RefreshIntermediaryDelegationAmounts refreshes the amount of delegation for all intermediary accounts. // This method includes minting new osmo if the refreshed delegation amount has increased, and // instantly undelegating and burning if the refreshed delegation has decreased. -func (k Keeper) RefreshIntermediaryDelegationAmounts(ctx sdk.Context) { +func (k Keeper) RefreshIntermediaryDelegationAmounts(ctx sdk.Context, accs []types.SuperfluidIntermediaryAccount) { // iterate over all intermedairy accounts - every (denom, validator) pair - accs := k.GetAllIntermediaryAccounts(ctx) for _, acc := range accs { mAddr := acc.GetAccAddress() diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index d64bfb2ac12..475ca5137af 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -975,7 +975,8 @@ func (s *KeeperTestSuite) TestRefreshIntermediaryDelegationAmounts() { s.App.SuperfluidKeeper.SetOsmoEquivalentMultiplier(s.Ctx, 2, denom, multiplier) } - s.App.SuperfluidKeeper.RefreshIntermediaryDelegationAmounts(s.Ctx) + accs := s.App.SuperfluidKeeper.GetAllIntermediaryAccounts(s.Ctx) + s.App.SuperfluidKeeper.RefreshIntermediaryDelegationAmounts(s.Ctx, accs) originalMultiplier := osmomath.NewDec(20) for interAccIndex, intermediaryAcc := range intermediaryAccs { @@ -1022,7 +1023,8 @@ func (s *KeeperTestSuite) TestRefreshIntermediaryDelegationAmounts() { } // refresh intermediary account delegations - s.App.SuperfluidKeeper.RefreshIntermediaryDelegationAmounts(s.Ctx) + accs = s.App.SuperfluidKeeper.GetAllIntermediaryAccounts(s.Ctx) + s.App.SuperfluidKeeper.RefreshIntermediaryDelegationAmounts(s.Ctx, accs) for _, intermediaryAcc := range intermediaryAccs { // check unbonded amount is removed after refresh operation