diff --git a/osmomath/math_test.go b/osmomath/math_test.go index 07d21abece5..742112e5035 100644 --- a/osmomath/math_test.go +++ b/osmomath/math_test.go @@ -90,7 +90,7 @@ func TestPowApprox(t *testing.T) { for i, tc := range testCases { var actualResult sdk.Dec - ConditionalPanic(t, tc.base.Equal(sdk.ZeroDec()), func() { + ConditionalPanic(t, tc.base.IsZero(), func() { fmt.Println(tc.base) actualResult = PowApprox(tc.base, tc.exp, tc.powPrecision) require.True( @@ -154,7 +154,7 @@ func TestPow(t *testing.T) { for i, tc := range testCases { var actualResult sdk.Dec - ConditionalPanic(t, tc.base.Equal(sdk.ZeroDec()), func() { + ConditionalPanic(t, tc.base.IsZero(), func() { actualResult = Pow(tc.base, tc.exp) require.True( t, diff --git a/osmoutils/accum/accum.go b/osmoutils/accum/accum.go index c16008f548a..7ef5762ca11 100644 --- a/osmoutils/accum/accum.go +++ b/osmoutils/accum/accum.go @@ -294,7 +294,7 @@ func (accum *AccumulatorObject) UpdatePosition(name string, numShares sdk.Dec) e // All intervalAccumulationPerShare DecCoin value must be non-negative. They must also be a superset of the // old accumulator value associated with the position. func (accum *AccumulatorObject) UpdatePositionIntervalAccumulation(name string, numShares sdk.Dec, intervalAccumulationPerShare sdk.DecCoins) error { - if numShares.Equal(sdk.ZeroDec()) { + if numShares.IsZero() { return ZeroSharesError } @@ -422,7 +422,7 @@ func (accum AccumulatorObject) ClaimRewards(positionName string) (sdk.Coins, sdk // This is acceptable because we round in favor of the protocol. truncatedRewardsTotal, dust := totalRewards.TruncateDecimal() - if position.NumShares.Equal(sdk.ZeroDec()) { + if position.NumShares.IsZero() { // remove the position from state entirely if numShares = zero accum.deletePosition(positionName) } else { diff --git a/osmoutils/accum/accum_test.go b/osmoutils/accum/accum_test.go index fc14c2978a7..ee90a26ad1b 100644 --- a/osmoutils/accum/accum_test.go +++ b/osmoutils/accum/accum_test.go @@ -972,7 +972,7 @@ func (suite *AccumTestSuite) TestRemoveFromPosition() { suite.Require().Equal(tc.startingUnclaimedRewards.Add(tc.expAccumDelta.MulDec(tc.startingNumShares)...), newPosition.UnclaimedRewardsTotal) // Ensure address's position properly reflects new number of shares - if (tc.startingNumShares.Sub(tc.removedShares)).Equal(sdk.ZeroDec()) { + if (tc.startingNumShares.Sub(tc.removedShares)).IsZero() { suite.Require().Equal(emptyDec, newPosition.NumShares) } else { suite.Require().Equal(tc.startingNumShares.Sub(tc.removedShares), newPosition.NumShares) @@ -1163,7 +1163,7 @@ func (suite *AccumTestSuite) TestGetPositionSize() { // Get position size from valid address (or from nonexistant if address does not exist) positionSize, err := curAccum.GetPositionSize(positionName) - if tc.changedShares.GT(sdk.ZeroDec()) { + if tc.changedShares.IsPositive() { accumPackage.InitOrUpdatePosition(curAccum, curAccum.GetValue(), positionName, tc.numShares.Add(tc.changedShares), sdk.NewDecCoins(), nil) } @@ -1613,14 +1613,14 @@ func (suite *AccumTestSuite) TestGetTotalShares() { // Half the time, we add to the new position addAmt := baseAmt.Mul(addShares) - if addAmt.GT(sdk.ZeroDec()) { + if addAmt.IsPositive() { err = curAccum.AddToPosition(curAddr, addAmt) suite.Require().NoError(err) } // Half the time, we remove one share from the position amtToRemove := sdk.OneDec().Mul(removeShares) - if amtToRemove.GT(sdk.ZeroDec()) { + if amtToRemove.IsPositive() { err = curAccum.RemoveFromPosition(curAddr, amtToRemove) suite.Require().NoError(err) } diff --git a/x/concentrated-liquidity/export_test.go b/x/concentrated-liquidity/export_test.go index dec638850cf..613ca6ba69d 100644 --- a/x/concentrated-liquidity/export_test.go +++ b/x/concentrated-liquidity/export_test.go @@ -118,10 +118,6 @@ func (k Keeper) CollectSpreadRewards(ctx sdk.Context, owner sdk.AccAddress, posi return k.collectSpreadRewards(ctx, owner, positionId) } -func (k Keeper) EnsurePositionOwner(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, positionId uint64) error { - return k.ensurePositionOwner(ctx, sender, poolId, positionId) -} - func (k Keeper) PrepareClaimableSpreadRewards(ctx sdk.Context, positionId uint64) (sdk.Coins, error) { return k.prepareClaimableSpreadRewards(ctx, positionId) } diff --git a/x/concentrated-liquidity/incentives.go b/x/concentrated-liquidity/incentives.go index 11c726a4b64..ca97c7d27db 100644 --- a/x/concentrated-liquidity/incentives.go +++ b/x/concentrated-liquidity/incentives.go @@ -403,27 +403,24 @@ func (k Keeper) updateGivenPoolUptimeAccumulatorsToNow(ctx sdk.Context, pool typ // We optimistically assume that all liquidity on the active tick qualifies and handle // uptime-related checks in forfeiting logic. + + // If there is no share to be incentivized for the current uptime accumulator, we leave it unchanged qualifyingLiquidity := pool.GetLiquidity().Add(qualifyingBalancerShares) + if !qualifyingLiquidity.LT(sdk.OneDec()) { + for uptimeIndex := range uptimeAccums { + // Get relevant uptime-level values + curUptimeDuration := types.SupportedUptimes[uptimeIndex] + incentivesToAddToCurAccum, updatedPoolRecords, err := calcAccruedIncentivesForAccum(ctx, curUptimeDuration, qualifyingLiquidity, timeElapsedSec, poolIncentiveRecords) + if err != nil { + return err + } - for uptimeIndex := range uptimeAccums { - // Get relevant uptime-level values - curUptimeDuration := types.SupportedUptimes[uptimeIndex] + // Emit incentives to current uptime accumulator + uptimeAccums[uptimeIndex].AddToAccumulator(incentivesToAddToCurAccum) - // If there is no share to be incentivized for the current uptime accumulator, we leave it unchanged - if qualifyingLiquidity.LT(sdk.OneDec()) { - continue + // Update pool records (stored in state after loop) + poolIncentiveRecords = updatedPoolRecords } - - incentivesToAddToCurAccum, updatedPoolRecords, err := calcAccruedIncentivesForAccum(ctx, curUptimeDuration, qualifyingLiquidity, timeElapsedSec, poolIncentiveRecords) - if err != nil { - return err - } - - // Emit incentives to current uptime accumulator - uptimeAccums[uptimeIndex].AddToAccumulator(incentivesToAddToCurAccum) - - // Update pool records (stored in state after loop) - poolIncentiveRecords = updatedPoolRecords } // Update pool incentive records and LastLiquidityUpdate time in state to reflect emitted incentives @@ -538,7 +535,7 @@ func (k Keeper) setIncentiveRecord(ctx sdk.Context, incentiveRecord types.Incent // In all other cases, we update the record in state if store.Has(key) && incentiveRecordBody.RemainingCoin.IsZero() { store.Delete(key) - } else if incentiveRecordBody.RemainingCoin.Amount.GT(sdk.ZeroDec()) { + } else if incentiveRecordBody.RemainingCoin.Amount.IsPositive() { osmoutils.MustSet(store, key, &incentiveRecordBody) } @@ -972,9 +969,11 @@ func (k Keeper) collectIncentives(ctx sdk.Context, sender sdk.AccAddress, positi return sdk.Coins{}, sdk.Coins{}, err } - err = k.ensurePositionOwner(ctx, sender, position.PoolId, positionId) - if err != nil { - return sdk.Coins{}, sdk.Coins{}, err + if sender.String() != position.Address { + return sdk.Coins{}, sdk.Coins{}, types.NotPositionOwnerError{ + PositionId: positionId, + Address: sender.String(), + } } // Claim all incentives for the position. diff --git a/x/concentrated-liquidity/lp_test.go b/x/concentrated-liquidity/lp_test.go index c1e7cdaca75..dae13b0ad16 100644 --- a/x/concentrated-liquidity/lp_test.go +++ b/x/concentrated-liquidity/lp_test.go @@ -663,9 +663,6 @@ func (s *KeeperTestSuite) TestWithdrawPosition() { s.Require().ErrorAs(err, &types.PositionIdNotFoundError{PositionId: config.positionId}) s.Require().Equal(clmodel.Position{}, position) - err = concentratedLiquidityKeeper.EnsurePositionOwner(s.Ctx, owner, config.poolId, config.positionId) - s.Require().Error(err, types.NotPositionOwnerError{PositionId: config.positionId, Address: owner.String()}) - // Since the positionLiquidity is deleted, retrieving it should return an error. positionLiquidity, err := s.App.ConcentratedLiquidityKeeper.GetPositionLiquidity(s.Ctx, config.positionId) s.Require().Error(err) diff --git a/x/concentrated-liquidity/math/math.go b/x/concentrated-liquidity/math/math.go index fabd75e5116..f52a5b95f1f 100644 --- a/x/concentrated-liquidity/math/math.go +++ b/x/concentrated-liquidity/math/math.go @@ -26,11 +26,11 @@ func Liquidity0(amount sdk.Int, sqrtPriceA, sqrtPriceB sdk.Dec) sdk.Dec { product := sqrtPriceABigDec.Mul(sqrtPriceBBigDec) diff := sqrtPriceBBigDec.Sub(sqrtPriceABigDec) - if diff.Equal(osmomath.ZeroDec()) { + if diff.IsZero() { panic(fmt.Sprintf("liquidity0 diff is zero: sqrtPriceA %s sqrtPriceB %s", sqrtPriceA, sqrtPriceB)) } - return amountBigDec.Mul(product).Quo(diff).SDKDec() + return amountBigDec.MulMut(product).QuoMut(diff).SDKDec() } // Liquidity1 takes an amount of asset1 in the pool as well as the sqrtpCur and the nextPrice @@ -49,11 +49,11 @@ func Liquidity1(amount sdk.Int, sqrtPriceA, sqrtPriceB sdk.Dec) sdk.Dec { sqrtPriceBBigDec := osmomath.BigDecFromSDKDec(sqrtPriceB) diff := sqrtPriceBBigDec.Sub(sqrtPriceABigDec) - if diff.Equal(osmomath.ZeroDec()) { + if diff.IsZero() { panic(fmt.Sprintf("liquidity1 diff is zero: sqrtPriceA %s sqrtPriceB %s", sqrtPriceA, sqrtPriceB)) } - return amountBigDec.Quo(diff).SDKDec() + return amountBigDec.QuoMut(diff).SDKDec() } // CalcAmount0 takes the asset with the smaller liquidity in the pool as well as the sqrtpCur and the nextPrice and calculates the amount of asset 0 @@ -70,7 +70,7 @@ func CalcAmount0Delta(liq, sqrtPriceA, sqrtPriceB sdk.Dec, roundUp bool) sdk.Dec // this is to prevent removing more from the pool than expected due to rounding // example: we calculate 1000000.9999999 uusdc (~$1) amountIn and 2000000.999999 uosmo amountOut // we would want the user to put in 1000001 uusdc rather than 1000000 uusdc to ensure we are charging enough for the amount they are removing - // additionally, without rounding, there exists cases where the swapState.amountSpecifiedRemaining.GT(sdk.ZeroDec()) for loop within + // additionally, without rounding, there exists cases where the swapState.amountSpecifiedRemaining.IsPositive() for loop within // the CalcOut/In functions never actually reach zero due to dust that would have never gotten counted towards the amount (numbers after the 10^6 place) if roundUp { // Note that we do MulTruncate so that the denominator is smaller as this is @@ -105,7 +105,7 @@ func CalcAmount1Delta(liq, sqrtPriceA, sqrtPriceB sdk.Dec, roundUp bool) sdk.Dec // this is to prevent removing more from the pool than expected due to rounding // example: we calculate 1000000.9999999 uusdc (~$1) amountIn and 2000000.999999 uosmo amountOut // we would want the used to put in 1000001 uusdc rather than 1000000 uusdc to ensure we are charging enough for the amount they are removing - // additionally, without rounding, there exists cases where the swapState.amountSpecifiedRemaining.GT(sdk.ZeroDec()) for loop within + // additionally, without rounding, there exists cases where the swapState.amountSpecifiedRemaining.IsPositive() for loop within // the CalcOut/In functions never actually reach zero due to dust that would have never gotten counted towards the amount (numbers after the 10^6 place) if roundUp { // Note that we do MulRoundUp so that the end result is larger as this is @@ -128,12 +128,14 @@ func CalcAmount1Delta(liq, sqrtPriceA, sqrtPriceB sdk.Dec, roundUp bool) sdk.Dec // to avoid overpaying the amount out of the pool. Therefore, we round up. // sqrt_next = liq * sqrt_cur / (liq + token_in * sqrt_cur) func GetNextSqrtPriceFromAmount0InRoundingUp(sqrtPriceCurrent, liquidity, amountZeroRemainingIn sdk.Dec) (sqrtPriceNext sdk.Dec) { - if amountZeroRemainingIn.Equal(sdk.ZeroDec()) { + if amountZeroRemainingIn.IsZero() { return sqrtPriceCurrent } product := amountZeroRemainingIn.Mul(sqrtPriceCurrent) - denominator := product.AddMut(liquidity) + // denominator = product + liquidity + denominator := product + denominator.AddMut(liquidity) return liquidity.Mul(sqrtPriceCurrent).QuoRoundupMut(denominator) } @@ -143,7 +145,7 @@ func GetNextSqrtPriceFromAmount0InRoundingUp(sqrtPriceCurrent, liquidity, amount // so that we get the desired output amount out. Therefore, we round up. // sqrt_next = liq * sqrt_cur / (liq - token_out * sqrt_cur) func GetNextSqrtPriceFromAmount0OutRoundingUp(sqrtPriceCurrent, liquidity, amountZeroRemainingOut sdk.Dec) (sqrtPriceNext sdk.Dec) { - if amountZeroRemainingOut.Equal(sdk.ZeroDec()) { + if amountZeroRemainingOut.IsZero() { return sqrtPriceCurrent } diff --git a/x/concentrated-liquidity/model/pool_test.go b/x/concentrated-liquidity/model/pool_test.go index a464bead0a4..97031b13244 100644 --- a/x/concentrated-liquidity/model/pool_test.go +++ b/x/concentrated-liquidity/model/pool_test.go @@ -695,8 +695,8 @@ func (suite *ConcentratedPoolTestSuite) TestCalcActualAmounts() { amt1Diff := actualAmount1.Sub(actualAmount1Neg.Neg()) // Difference is between 0 and 1 due to positive liquidity rounding up and negative liquidity performing math normally. - suite.Require().True(amt0Diff.GT(sdk.ZeroDec()) && amt0Diff.LT(sdk.OneDec())) - suite.Require().True(amt1Diff.GT(sdk.ZeroDec()) && amt1Diff.LT(sdk.OneDec())) + suite.Require().True(amt0Diff.IsPositive() && amt0Diff.LT(sdk.OneDec())) + suite.Require().True(amt1Diff.IsPositive() && amt1Diff.LT(sdk.OneDec())) } }) } diff --git a/x/concentrated-liquidity/position.go b/x/concentrated-liquidity/position.go index 21089344e0a..e2e96b565e2 100644 --- a/x/concentrated-liquidity/position.go +++ b/x/concentrated-liquidity/position.go @@ -99,22 +99,6 @@ func (k Keeper) HasAnyPositionForPool(ctx sdk.Context, poolId uint64) (bool, err return osmoutils.HasAnyAtPrefix(store, poolPositionKey, parse) } -func (k Keeper) ensurePositionOwner(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, positionId uint64) error { - isOwner := k.isPositionOwner(ctx, sender, poolId, positionId) - if !isOwner { - return types.NotPositionOwnerError{ - PositionId: positionId, - Address: sender.String(), - } - } - return nil -} - -// isPositionOwner returns true if the given positionId is owned by the given sender inside the given pool. -func (k Keeper) isPositionOwner(ctx sdk.Context, sender sdk.AccAddress, poolId uint64, positionId uint64) bool { - return ctx.KVStore(k.storeKey).Has(types.KeyAddressPoolIdPositionId(sender, poolId, positionId)) -} - // GetAllPositionsForPoolId gets all the position for a specific poolId and store prefix. func (k Keeper) GetAllPositionIdsForPoolId(ctx sdk.Context, prefix []byte, poolId uint64) ([]uint64, error) { store := ctx.KVStore(k.storeKey) diff --git a/x/concentrated-liquidity/position_test.go b/x/concentrated-liquidity/position_test.go index cb18de096ec..ebacc9aa82e 100644 --- a/x/concentrated-liquidity/position_test.go +++ b/x/concentrated-liquidity/position_test.go @@ -313,7 +313,7 @@ func (s *KeeperTestSuite) TestInitOrUpdatePosition() { if test.positionExists { // Track how much the current uptime accum has grown by actualUptimeAccumDelta[uptimeIndex] = newUptimeAccumValues[uptimeIndex].Sub(initUptimeAccumValues[uptimeIndex]) - if timeElapsedSec.GT(sdk.ZeroDec()) { + if timeElapsedSec.IsPositive() { expectedGrowthCurAccum, _, err := cl.CalcAccruedIncentivesForAccum(s.Ctx, uptime, test.param.liquidityDelta, timeElapsedSec, expectedIncentiveRecords) s.Require().NoError(err) expectedUptimeAccumValueGrowth[uptimeIndex] = expectedGrowthCurAccum @@ -410,103 +410,6 @@ type positionOwnershipTest struct { poolId uint64 } -func (s *KeeperTestSuite) runIsPositionOwnerTest(test positionOwnershipTest) { - s.SetupTest() - p := s.PrepareConcentratedPool() - for i, owner := range test.setupPositions { - err := s.App.ConcentratedLiquidityKeeper.InitOrUpdatePosition(s.Ctx, p.GetId(), owner, DefaultLowerTick, DefaultUpperTick, DefaultLiquidityAmt, DefaultJoinTime, uint64(i)) - s.Require().NoError(err) - } - err := s.App.ConcentratedLiquidityKeeper.EnsurePositionOwner(s.Ctx, test.queryPositionOwner, test.poolId, test.queryPositionId) - if test.expPass { - s.Require().NoError(err) - } else { - s.Require().Error(err) - } - -} - -func (s *KeeperTestSuite) TestIsPositionOwnerMultiPosition() { - tenAddrOneAddr := []sdk.AccAddress{} - for i := 0; i < 10; i++ { - tenAddrOneAddr = append(tenAddrOneAddr, s.TestAccs[0]) - } - tenAddrOneAddr = append(tenAddrOneAddr, s.TestAccs[1]) - tests := map[string]positionOwnershipTest{ - "prefix malleability (prior bug)": { - queryPositionOwner: s.TestAccs[1], - queryPositionId: 1, expPass: false, - setupPositions: tenAddrOneAddr, - }, - "things work": { - queryPositionOwner: s.TestAccs[1], - queryPositionId: 10, expPass: true, - setupPositions: tenAddrOneAddr, - }, - } - for name, test := range tests { - s.Run(name, func() { - test.poolId = 1 - s.runIsPositionOwnerTest(test) - }) - } -} - -func (s *KeeperTestSuite) TestIsPositionOwner() { - actualOwner := s.TestAccs[0] - nonOwner := s.TestAccs[1] - - tests := []struct { - name string - ownerToQuery sdk.AccAddress - poolId uint64 - positionId uint64 - isOwner bool - }{ - { - name: "Happy path", - ownerToQuery: actualOwner, - poolId: 1, - positionId: DefaultPositionId, - isOwner: true, - }, - { - name: "query non owner", - ownerToQuery: nonOwner, - poolId: 1, - positionId: DefaultPositionId, - isOwner: false, - }, - { - name: "different pool ID, not the owner", - ownerToQuery: actualOwner, - poolId: 2, - positionId: DefaultPositionId, - isOwner: false, - }, - { - name: "different position ID, not the owner", - ownerToQuery: actualOwner, - poolId: 1, - positionId: DefaultPositionId + 1, - isOwner: false, - }, - } - - for _, test := range tests { - s.Run(test.name, func() { - s.runIsPositionOwnerTest(positionOwnershipTest{ - queryPositionOwner: test.ownerToQuery, - queryPositionId: test.positionId, - expPass: test.isOwner, - // positions 0 and 1 are owned by actualOwner - setupPositions: []sdk.AccAddress{actualOwner, actualOwner}, - poolId: test.poolId, - }) - }) - } -} - func (s *KeeperTestSuite) TestGetUserPositions() { s.Setup() defaultAddress := s.TestAccs[0] diff --git a/x/concentrated-liquidity/simulation/sim_msgs.go b/x/concentrated-liquidity/simulation/sim_msgs.go index 7d304e83798..859ab561952 100644 --- a/x/concentrated-liquidity/simulation/sim_msgs.go +++ b/x/concentrated-liquidity/simulation/sim_msgs.go @@ -114,7 +114,7 @@ func RandMsgWithdrawPosition(k clkeeper.Keeper, sim *osmosimtypes.SimCtx, ctx sd } withdrawAmount := sim.RandomDecAmount(position.Liquidity) - if withdrawAmount.TruncateDec().LT(sdk.ZeroDec()) { + if withdrawAmount.TruncateDec().IsNegative() { return nil, fmt.Errorf("Invalid withdraw amount") } diff --git a/x/concentrated-liquidity/spread_rewards.go b/x/concentrated-liquidity/spread_rewards.go index 9a1bb045763..be3e2c314dc 100644 --- a/x/concentrated-liquidity/spread_rewards.go +++ b/x/concentrated-liquidity/spread_rewards.go @@ -171,9 +171,11 @@ func (k Keeper) collectSpreadRewards(ctx sdk.Context, sender sdk.AccAddress, pos } // Spread reward collector must be the owner of the position. - err = k.ensurePositionOwner(ctx, sender, position.PoolId, positionId) - if err != nil { - return sdk.Coins{}, err + if sender.String() != position.Address { + return sdk.Coins{}, types.NotPositionOwnerError{ + PositionId: positionId, + Address: sender.String(), + } } // Get the amount of spread rewards that the position is eligible to claim. diff --git a/x/concentrated-liquidity/swaps.go b/x/concentrated-liquidity/swaps.go index 2a574755975..f8d8acf118f 100644 --- a/x/concentrated-liquidity/swaps.go +++ b/x/concentrated-liquidity/swaps.go @@ -503,8 +503,8 @@ func (k Keeper) computeInAmtGivenOut( // Update the swapState with the new sqrtPrice from the above swap swapState.sqrtPrice = computedSqrtPrice - swapState.amountSpecifiedRemaining = swapState.amountSpecifiedRemaining.SubMut(amountOut) - swapState.amountCalculated = swapState.amountCalculated.AddMut(amountIn.Add(spreadRewardChargeTotal)) + swapState.amountSpecifiedRemaining.SubMut(amountOut) + swapState.amountCalculated.AddMut(amountIn.Add(spreadRewardChargeTotal)) // If the ComputeSwapWithinBucketInGivenOut(...) calculated a computedSqrtPrice that is equal to the nextInitializedTickSqrtPrice, this means all liquidity in the current // bucket has been consumed and we must move on to the next bucket by crossing a tick to complete the swap @@ -578,8 +578,7 @@ func (k Keeper) swapCrossTickLogic(ctx sdk.Context, liquidityNet = swapState.swapStrategy.SetLiquidityDeltaSign(liquidityNet) // Update the swapState's liquidity with the new tick's liquidity - newLiquidity := swapState.liquidity.AddMut(liquidityNet) - swapState.liquidity = newLiquidity + swapState.liquidity.AddMut(liquidityNet) return swapState, nil } diff --git a/x/concentrated-liquidity/tick.go b/x/concentrated-liquidity/tick.go index 29f8ea85216..cbb186469f3 100644 --- a/x/concentrated-liquidity/tick.go +++ b/x/concentrated-liquidity/tick.go @@ -61,9 +61,9 @@ func (k Keeper) initOrUpdateTick(ctx sdk.Context, poolId uint64, currentTick int // calculate liquidityNet, which we take into account and track depending on whether liquidityIn is positive or negative if upper { - tickInfo.LiquidityNet = tickInfo.LiquidityNet.Sub(liquidityDelta) + tickInfo.LiquidityNet.SubMut(liquidityDelta) } else { - tickInfo.LiquidityNet = tickInfo.LiquidityNet.Add(liquidityDelta) + tickInfo.LiquidityNet.AddMut(liquidityDelta) } k.SetTickInfo(ctx, poolId, tickIndex, &tickInfo) diff --git a/x/concentrated-liquidity/types/genesis/genesis.go b/x/concentrated-liquidity/types/genesis/genesis.go index 8f54115d332..69e6c9891fe 100644 --- a/x/concentrated-liquidity/types/genesis/genesis.go +++ b/x/concentrated-liquidity/types/genesis/genesis.go @@ -25,6 +25,5 @@ func (gs GenesisState) Validate() error { if gs.NextIncentiveRecordId == 0 { return types.InvalidNextIncentiveRecordIdError{NextIncentiveRecordId: gs.NextIncentiveRecordId} } - return nil } diff --git a/x/concentrated-liquidity/types/params.go b/x/concentrated-liquidity/types/params.go index 4efe7eddc94..c058e1f4ab9 100644 --- a/x/concentrated-liquidity/types/params.go +++ b/x/concentrated-liquidity/types/params.go @@ -187,7 +187,7 @@ func validateBalancerSharesDiscount(i interface{}) error { } // Ensure that the passed in discount rate is between 0 and 1. - if balancerSharesRewardDiscount.LT(sdk.ZeroDec()) || balancerSharesRewardDiscount.GT(sdk.OneDec()) { + if balancerSharesRewardDiscount.IsNegative() || balancerSharesRewardDiscount.GT(sdk.OneDec()) { return InvalidDiscountRateError{DiscountRate: balancerSharesRewardDiscount} } diff --git a/x/gamm/keeper/pool_service.go b/x/gamm/keeper/pool_service.go index 462e767d44a..274ea39f60e 100644 --- a/x/gamm/keeper/pool_service.go +++ b/x/gamm/keeper/pool_service.go @@ -70,7 +70,7 @@ func (k Keeper) InitializePool(ctx sdk.Context, pool poolmanagertypes.PoolI, sen } exitFee := cfmmPool.GetExitFee(ctx) - if !exitFee.Equal(sdk.ZeroDec()) { + if !exitFee.IsZero() { return fmt.Errorf("can not create pool with non zero exit fee, got %d", exitFee) } diff --git a/x/gamm/pool-models/internal/cfmm_common/lp.go b/x/gamm/pool-models/internal/cfmm_common/lp.go index 5e2db2289f1..ae9102c2335 100644 --- a/x/gamm/pool-models/internal/cfmm_common/lp.go +++ b/x/gamm/pool-models/internal/cfmm_common/lp.go @@ -25,7 +25,7 @@ func CalcExitPool(ctx sdk.Context, pool types.CFMMPoolI, exitingShares sdk.Int, var refundedShares sdk.Dec if !exitFee.IsZero() { // exitingShares * (1 - exit fee) - oneSubExitFee := sdk.OneDec().SubMut(exitFee) + oneSubExitFee := sdk.OneDec().Sub(exitFee) refundedShares = oneSubExitFee.MulIntMut(exitingShares) } else { refundedShares = exitingShares.ToDec() diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index 832f23240c8..1eb682083cf 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -278,7 +278,7 @@ func (k Keeper) distributeSyntheticInternal( // - perpetual // - non-perpetual // -// CONTRACT: gauge must be active +// 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, ) (sdk.Coins, error) { @@ -292,6 +292,12 @@ func (k Keeper) distributeInternal( remainEpochs = gauge.NumEpochsPaidOver - gauge.FilledEpochs } + // defense in depth + // this should never happen in practice since gauge passed in should always be an active gauge. + if remainEpochs == uint64(0) { + return nil, fmt.Errorf("gauge with id of %d is not active", gauge.Id) + } + // This is a no lock distribution flow that assumes that we have a pool associated with the gauge. // Currently, this flow is only used for CL pools. Fails if the pool is not found. // Fails if the pool found is not a CL pool. diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index 093afc1bd55..60c2e5dc6fd 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -405,6 +405,9 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { withNumEpochs := func(tc test, numEpochs uint64) test { tc.numEpochsPaidOver = numEpochs + if numEpochs == uint64(0) { + return tc + } // Do deep copies tempDistributions := make(sdk.Coins, len(tc.expectedDistributions)) @@ -433,6 +436,10 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { tc.distrTo.Duration = time.Hour } tc.poolId = poolId + return tc + } + + withError := func(tc test) test { tc.expectErr = true return tc } @@ -444,7 +451,8 @@ func (s *KeeperTestSuite) TestDistribute_ExternalIncentives_NoLock() { "perpetual, 2 coins, paid over 1 epoch": withIsPerpetual(withGaugeCoins(defaultTest, defaultBothCoins), true), "non-perpetual, 1 coin, paid over 2 epochs": withNumEpochs(defaultTest, 2), "non-perpetual, 2 coins, paid over 3 epochs": withNumEpochs(withGaugeCoins(defaultTest, defaultBothCoins), 3), - "error: balancer pool id": withPoolId(defaultTest, defaultBalancerPool), + "error: balancer pool id": withError(withPoolId(defaultTest, defaultBalancerPool)), + "error: inactive gauge": withError(withNumEpochs(defaultTest, 0)), } for name, tc := range tests { diff --git a/x/mint/types/params.go b/x/mint/types/params.go index 2fe6648c737..f3a1cb192cb 100644 --- a/x/mint/types/params.go +++ b/x/mint/types/params.go @@ -154,7 +154,7 @@ func validateGenesisEpochProvisions(i interface{}) error { return fmt.Errorf("invalid parameter type: %T", i) } - if v.LT(sdk.ZeroDec()) { + if v.IsNegative() { return fmt.Errorf("genesis epoch provision must be non-negative") } diff --git a/x/superfluid/types/params.go b/x/superfluid/types/params.go index 5fd26f855a2..a7969803de7 100644 --- a/x/superfluid/types/params.go +++ b/x/superfluid/types/params.go @@ -50,7 +50,7 @@ func ValidateMinimumRiskFactor(i interface{}) error { return fmt.Errorf("invalid parameter type: %T", i) } - if v.LT(sdk.ZeroDec()) || v.GT(sdk.NewDec(100)) { + if v.IsNegative() || v.GT(sdk.NewDec(100)) { return fmt.Errorf("minimum risk factor should be between 0 - 100: %s", v.String()) } diff --git a/x/twap/logic.go b/x/twap/logic.go index d8bde3358b7..2f9b801fd2a 100644 --- a/x/twap/logic.go +++ b/x/twap/logic.go @@ -164,8 +164,8 @@ func (k Keeper) updateRecord(ctx sdk.Context, record types.TwapRecord) (types.Tw // Incase record height should equal to ctx height // But ArithmeticTwapAccumulators should be zero if (record.Height == ctx.BlockHeight() || record.Time.Equal(ctx.BlockTime())) && - !record.P1ArithmeticTwapAccumulator.Equal(sdk.ZeroDec()) && - !record.P0ArithmeticTwapAccumulator.Equal(sdk.ZeroDec()) { + !record.P1ArithmeticTwapAccumulator.IsZero() && + !record.P0ArithmeticTwapAccumulator.IsZero() { return types.TwapRecord{}, types.InvalidUpdateRecordError{} } else if record.Height > ctx.BlockHeight() || record.Time.After(ctx.BlockTime()) { // Normal case, ctx should be after record height & time diff --git a/x/valset-pref/simulation/sim_msgs.go b/x/valset-pref/simulation/sim_msgs.go index f5b259cd5a6..c9c1a6d17fd 100644 --- a/x/valset-pref/simulation/sim_msgs.go +++ b/x/valset-pref/simulation/sim_msgs.go @@ -168,7 +168,7 @@ func GetRandomValAndWeights(ctx sdk.Context, k valsetkeeper.Keeper, sim *osmosim var preferences []types.ValidatorPreference // Generate random validators with random weights that sums to 1 - for remainingWeight.GT(sdk.ZeroDec()) { + for remainingWeight.IsPositive() { randValidator := RandomValidator(ctx, sim) if randValidator == nil { return nil, fmt.Errorf("No validator") @@ -177,7 +177,7 @@ func GetRandomValAndWeights(ctx sdk.Context, k valsetkeeper.Keeper, sim *osmosim randValue := sim.RandomDecAmount(remainingWeight) remainingWeight = remainingWeight.Sub(randValue) - if !randValue.Equal(sdk.ZeroDec()) { + if !randValue.IsZero() { preferences = append(preferences, types.ValidatorPreference{ ValOperAddress: randValidator.OperatorAddress, Weight: randValue, diff --git a/x/valset-pref/validator_set.go b/x/valset-pref/validator_set.go index 4870b580380..791d31e35d8 100644 --- a/x/valset-pref/validator_set.go +++ b/x/valset-pref/validator_set.go @@ -252,16 +252,16 @@ func (k Keeper) PreformRedelegation(ctx sdk.Context, delegator sdk.AccAddress, e // Algorithm starts here, verbose explanation in README.md for _, diffVal := range diffValSets { - if diffVal.Amount.TruncateDec().GT(sdk.ZeroDec()) { + if diffVal.Amount.TruncateDec().IsPositive() { for idx, targetDiffVal := range diffValSets { - if targetDiffVal.Amount.TruncateDec().LT(sdk.ZeroDec()) && diffVal.ValAddr != targetDiffVal.ValAddr { + if targetDiffVal.Amount.TruncateDec().IsNegative() && diffVal.ValAddr != targetDiffVal.ValAddr { valSource, valTarget, err := k.getValTargetAndSource(ctx, diffVal.ValAddr, targetDiffVal.ValAddr) if err != nil { return err } transferAmount := sdk.MinDec(diffVal.Amount, targetDiffVal.Amount.Abs()).TruncateDec() - if transferAmount.Equal(sdk.ZeroDec()) { + if transferAmount.IsZero() { break } @@ -273,7 +273,7 @@ func (k Keeper) PreformRedelegation(ctx sdk.Context, delegator sdk.AccAddress, e diffVal.Amount = diffVal.Amount.Sub(transferAmount) diffValSets[idx].Amount = targetDiffVal.Amount.Add(transferAmount) - if diffVal.Amount.Equal(sdk.ZeroDec()) { + if diffVal.Amount.IsZero() { break } }