Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor createGauge to accomodate for CL gauges #5383

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions proto/osmosis/lockup/lock.proto
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,17 @@ message PeriodLock {
enum LockQueryType {
option (gogoproto.goproto_enum_prefix) = false;

ByDuration = 0;
UNSPECIFIED = 0;
ByTime = 1;
ByDuration = 2;
}

// QueryCondition is a struct used for querying locks upon different conditions.
// Duration field and timestamp fields could be optional, depending on the
// LockQueryType.
message QueryCondition {
// LockQueryType is a type of lock query, ByLockDuration | ByLockTime
LockQueryType lock_query_type = 1;
LockQueryType lock_query_type = 1 [ (gogoproto.nullable) = true ];
// Denom represents the token denomination we are looking to lock up
string denom = 2;
// Duration is used to query locks with longer duration than the specified
Expand Down
31 changes: 16 additions & 15 deletions x/incentives/keeper/gauge.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,25 +96,26 @@ func (k Keeper) SetGaugeWithRefKey(ctx sdk.Context, gauge *types.Gauge) error {
// CreateGauge creates a gauge and sends coins to the gauge.
func (k Keeper) CreateGauge(ctx sdk.Context, isPerpetual bool, owner sdk.AccAddress, coins sdk.Coins, distrTo lockuptypes.QueryCondition, startTime time.Time, numEpochsPaidOver uint64) (uint64, error) {
// Ensure that this gauge's duration is one of the allowed durations on chain
durations := k.GetLockableDurations(ctx)
if distrTo.LockQueryType == lockuptypes.ByDuration {
durationOk := false
for _, duration := range durations {
if duration == distrTo.Duration {
durationOk = true
break
if distrTo.LockQueryType != lockuptypes.UNSPECIFIED {
durations := k.GetLockableDurations(ctx)
if distrTo.LockQueryType == lockuptypes.ByDuration {
durationOk := false
for _, duration := range durations {
if duration == distrTo.Duration {
durationOk = true
break
}
}
if !durationOk {
return 0, fmt.Errorf("invalid duration: %d", distrTo.Duration)
}
}
if !durationOk {
return 0, fmt.Errorf("invalid duration: %d", distrTo.Duration)
}
}

// check if denom this gauge pays out to exists on-chain
if !k.bk.HasSupply(ctx, distrTo.Denom) && !strings.Contains(distrTo.Denom, "osmovaloper") {
return 0, fmt.Errorf("denom does not exist: %s", distrTo.Denom)
// check if denom this gauge pays out to exists on-chain
if !k.bk.HasSupply(ctx, distrTo.Denom) && !strings.Contains(distrTo.Denom, "osmovaloper") {
return 0, fmt.Errorf("denom does not exist: %s", distrTo.Denom)
}
}

gauge := types.Gauge{
Id: k.GetLastGaugeID(ctx) + 1,
IsPerpetual: isPerpetual,
Expand Down
4 changes: 0 additions & 4 deletions x/incentives/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,6 @@ func (m MsgCreateGauge) ValidateBasic() error {
return errors.New("distribution period should be 1 epoch for perpetual gauge")
}

if lockuptypes.LockQueryType_name[int32(m.DistributeTo.LockQueryType)] != "ByDuration" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why was this removed as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In CreateGauge for CL pool we don't want to limit just using ByDuration right? Or lmk if im wrong

Copy link
Contributor

@stackman27 stackman27 Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add this back and add a clause that if lockTypes are used it has to be "byDuration"?

return errors.New("only duration query condition is allowed. Start time distr conditions is an obsolete codepath slated for deletion")
}

return nil
}

Expand Down
99 changes: 52 additions & 47 deletions x/lockup/types/lock.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 1 addition & 5 deletions x/pool-incentives/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

appParams "github.com/osmosis-labs/osmosis/v16/app/params"
poolmanagertypes "github.com/osmosis-labs/osmosis/v16/x/poolmanager/types"
)

Expand Down Expand Up @@ -108,10 +107,7 @@ func (k Keeper) CreateConcentratedLiquidityPoolGauge(ctx sdk.Context, poolId uin
// dummy variable so that the existing logic does not break
// CreateGauge checks if LockQueryType is `ByDuration` or not, we bypass this check by passing
// lockQueryType as byTime. Although we do not need this check, we still cannot pass empty struct.
lockuptypes.QueryCondition{
LockQueryType: lockuptypes.ByTime,
Denom: appParams.BaseCoinUnit,
},
lockuptypes.QueryCondition{},
ctx.BlockTime(),
1,
)
Expand Down
3 changes: 1 addition & 2 deletions x/pool-incentives/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/stretchr/testify/suite"

"github.com/osmosis-labs/osmosis/v16/app/apptesting"
appParams "github.com/osmosis-labs/osmosis/v16/app/params"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming this was removed accidentally

gammtypes "github.com/osmosis-labs/osmosis/v16/x/gamm/types"
incentivestypes "github.com/osmosis-labs/osmosis/v16/x/incentives/types"
"github.com/osmosis-labs/osmosis/v16/x/pool-incentives/types"
Expand Down Expand Up @@ -220,7 +219,7 @@ func (s *KeeperTestSuite) TestCreateConcentratedLiquidityPoolGauge() {
s.Require().True(gaugeInfo.IsPerpetual)
s.Require().Empty(gaugeInfo.Coins)
s.Require().Equal(s.Ctx.BlockTime(), gaugeInfo.StartTime)
s.Require().Equal(appParams.BaseCoinUnit, gaugeInfo.DistributeTo.Denom)
// s.Require().Equal(appParams.BaseCoinUnit, gaugeInfo.DistributeTo.Denom)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// s.Require().Equal(appParams.BaseCoinUnit, gaugeInfo.DistributeTo.Denom)
s.Require().Equal(appParams.BaseCoinUnit, gaugeInfo.DistributeTo.Denom)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In CreateGauge we pass empty queryCondition so I dont think we should check gaugeInfo.DistributeTo.Denom cause its alway be "" as I think. If im wrong pls fix me

s.Require().Equal(uint64(1), gaugeInfo.NumEpochsPaidOver)
}
})
Expand Down