Skip to content

Commit

Permalink
updates (#7192) (#7194)
Browse files Browse the repository at this point in the history
  • Loading branch information
p0mvn authored Dec 23, 2023
1 parent 370b1d5 commit 1ae31ff
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 12 deletions.
17 changes: 11 additions & 6 deletions x/incentives/keeper/distribute.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func (k Keeper) doDistributionSends(ctx sdk.Context, distrs *distributionInfo) e
// the distrInfo struct. It also updates the gauge for the distribution.
// locks is expected to be the correct set of lock recipients for this gauge.
func (k Keeper) distributeSyntheticInternal(
ctx sdk.Context, gauge types.Gauge, locks []lockuptypes.PeriodLock, distrInfo *distributionInfo,
ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo,
) (sdk.Coins, error) {
qualifiedLocks := k.lk.GetLocksLongerThanDurationDenom(ctx, gauge.DistributeTo.Denom, gauge.DistributeTo.Duration)

Expand All @@ -398,12 +398,17 @@ func (k Keeper) distributeSyntheticInternal(
}
}

sortedAndTrimmedQualifiedLocks := make([]lockuptypes.PeriodLock, curIndex)
sortedAndTrimmedQualifiedLocks := make([]*lockuptypes.PeriodLock, curIndex)
// This is not an issue because we directly
// use v.index and &v.locks. However, we must be careful not to
// take the address of &v.
// nolint: exportloopref
for _, v := range qualifiedLocksMap {
v := v
if v.index < 0 {
continue
}
sortedAndTrimmedQualifiedLocks[v.index] = v.lock
sortedAndTrimmedQualifiedLocks[v.index] = &v.lock
}

return k.distributeInternal(ctx, gauge, sortedAndTrimmedQualifiedLocks, distrInfo)
Expand Down Expand Up @@ -556,7 +561,7 @@ func (k Keeper) syncVolumeSplitGroup(ctx sdk.Context, group types.Group) error {
//
// CONTRACT: gauge passed in as argument must be an active gauge.
func (k Keeper) distributeInternal(
ctx sdk.Context, gauge types.Gauge, locks []lockuptypes.PeriodLock, distrInfo *distributionInfo,
ctx sdk.Context, gauge types.Gauge, locks []*lockuptypes.PeriodLock, distrInfo *distributionInfo,
) (sdk.Coins, error) {
totalDistrCoins := sdk.NewCoins()

Expand Down Expand Up @@ -757,10 +762,10 @@ func (k Keeper) handleGroupPostDistribute(ctx sdk.Context, groupGauge types.Gaug
}

// getDistributeToBaseLocks takes a gauge along with cached period locks by denom and returns locks that must be distributed to
func (k Keeper) getDistributeToBaseLocks(ctx sdk.Context, gauge types.Gauge, cache map[string][]lockuptypes.PeriodLock) []lockuptypes.PeriodLock {
func (k Keeper) getDistributeToBaseLocks(ctx sdk.Context, gauge types.Gauge, cache map[string][]lockuptypes.PeriodLock) []*lockuptypes.PeriodLock {
// if gauge is empty, don't get the locks
if gauge.Coins.Empty() {
return []lockuptypes.PeriodLock{}
return []*lockuptypes.PeriodLock{}
}
// Confusingly, there is no way to get all synthetic lockups. Thus we use a separate method `distributeSyntheticInternal` to separately get lockSum for synthetic lockups.
// All gauges have a precondition of being ByDuration.
Expand Down
10 changes: 5 additions & 5 deletions x/incentives/keeper/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ func (k Keeper) FinishedGaugesIterator(ctx sdk.Context) sdk.Iterator {
}

// FilterLocksByMinDuration returns locks whose lock duration is greater than the provided minimum duration.
func FilterLocksByMinDuration(locks []lockuptypes.PeriodLock, minDuration time.Duration) []lockuptypes.PeriodLock {
filteredLocks := make([]lockuptypes.PeriodLock, 0, len(locks))
for _, lock := range locks {
if lock.Duration >= minDuration {
filteredLocks = append(filteredLocks, lock)
func FilterLocksByMinDuration(locks []lockuptypes.PeriodLock, minDuration time.Duration) []*lockuptypes.PeriodLock {
filteredLocks := make([]*lockuptypes.PeriodLock, 0, len(locks))
for i := range locks {
if locks[i].Duration >= minDuration {
filteredLocks = append(filteredLocks, &locks[i])
}
}
return filteredLocks
Expand Down
37 changes: 37 additions & 0 deletions x/incentives/keeper/iterator_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package keeper_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/osmosis-labs/osmosis/v21/x/incentives/keeper"
lockuptypes "github.com/osmosis-labs/osmosis/v21/x/lockup/types"
)

// This test validates that the FilterLocksByMinDuration function
// copies the correct locks when filtering.
// It helps to ensure that we do not use a wrong pointer by referencing
// a loop variable.
func TestFilterLocksByMinDuration(t *testing.T) {
const (
numLocks = 3
minDuration = 2
)

locks := make([]lockuptypes.PeriodLock, numLocks)
for i := 0; i < numLocks; i++ {
locks[i] = lockuptypes.PeriodLock{
ID: uint64(i + 1),
Duration: minDuration,
}
}

filteredLocks := keeper.FilterLocksByMinDuration(locks, minDuration)

require.Equal(t, len(locks), len(filteredLocks))

for i := 0; i < len(locks); i++ {
require.Equal(t, locks[i].ID, filteredLocks[i].ID)
}
}
2 changes: 1 addition & 1 deletion x/lockup/types/lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (p PeriodLock) SingleCoin() (sdk.Coin, error) {

// TODO: Can we use sumtree instead here?
// Assumes that caller is passing in locks that contain denom
func SumLocksByDenom(locks []PeriodLock, denom string) osmomath.Int {
func SumLocksByDenom(locks []*PeriodLock, denom string) osmomath.Int {
sumBi := big.NewInt(0)
// validate the denom once, so we can avoid the expensive validate check in the hot loop.
err := sdk.ValidateDenom(denom)
Expand Down

0 comments on commit 1ae31ff

Please sign in to comment.