fix/test/docs: negative interval accumulation with incentive rewards

CHANGELOG.md

@@ -45,6 +45,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### State Breaking
 
 * [#5532](https://github.com/osmosis-labs/osmosis/pull/5532) fix: Fix x/tokenfactory genesis import denoms reset x/bank existing denom metadata
+* [#5869](https://github.com/osmosis-labs/osmosis/pull/5869) fix negative interval accumulation with spread rewards
+* [#5872](https://github.com/osmosis-labs/osmosis/pull/5872) fix negative interval accumulation with incentive rewards
 
 ### BugFix
 

app/apptesting/test_suite.go

@@ -130,6 +130,18 @@ func (s *KeeperTestHelper) SetupTestForInitGenesis() {
 	s.hasUsedAbci = true
 }
 
+// RunTestCaseWithoutStateUpdates runs the testcase as a callback with the given name.
+// Does not persist any state changes. This is useful when test suite uses common state setup
+// but desures each test case to be run in isolation.
+func (s *KeeperTestHelper) RunTestCaseWithoutStateUpdates(name string, cb func(t *testing.T)) {
+	originalCtx := s.Ctx
+	s.Ctx, _ = s.Ctx.CacheContext()
+
+	s.T().Run(name, cb)
+
+	s.Ctx = originalCtx
+}
+
 func (s *KeeperTestHelper) SetEpochStartTime() {
 	epochsKeeper := s.App.EpochsKeeper
 

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/ory/dockertest/v3 v3.10.0
 	github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3
 	github.com/osmosis-labs/osmosis/osmomath v0.0.3-dev.0.20230629191111-f375469de8b6
-	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230715164027-b45d8bd42434
+	github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230718155055-1f3b64aa3a69
 	github.com/osmosis-labs/osmosis/x/epochs v0.0.0-20230328024000-175ec88e4304
 	github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230602130523-f9a94d8bbd10
 	github.com/pkg/errors v0.9.1

go.sum

@@ -950,10 +950,8 @@ github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3 h1:Ylmch
 github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3/go.mod h1:lV6KnqXYD/ayTe7310MHtM3I2q8Z6bBfMAi+bhwPYtI=
 github.com/osmosis-labs/osmosis/osmomath v0.0.3-dev.0.20230629191111-f375469de8b6 h1:Kmkx5Rh72+LB8AL6dc6fZA+IVR0INu0YIiMF2ScDhaQ=
 github.com/osmosis-labs/osmosis/osmomath v0.0.3-dev.0.20230629191111-f375469de8b6/go.mod h1:JTym95/bqrSnG5MPcXr1YDhv43JdCeo3p+iDbazoX68=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230709024311-81c831b050de h1:W2lMduMgpNA5zheEIIialw08n1pWJ44Y4t2F924tpDU=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230709024311-81c831b050de/go.mod h1:Pl8Nzx6O6ow/+aqfMoMSz4hX+zz6RrnDYsooptECGxM=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230715164027-b45d8bd42434 h1:MXPrA3sDtqOHYUa9zl4HMGMW+IJwGMqUf6+Hl9nhrCA=
-github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230715164027-b45d8bd42434/go.mod h1:Pl8Nzx6O6ow/+aqfMoMSz4hX+zz6RrnDYsooptECGxM=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230718155055-1f3b64aa3a69 h1:Eci7fRVgCFw8F7jNxGhkoG/dT3Lnue627KTjXxetiEQ=
+github.com/osmosis-labs/osmosis/osmoutils v0.0.0-20230718155055-1f3b64aa3a69/go.mod h1:Pl8Nzx6O6ow/+aqfMoMSz4hX+zz6RrnDYsooptECGxM=
 github.com/osmosis-labs/osmosis/x/epochs v0.0.0-20230328024000-175ec88e4304 h1:RIrWLzIiZN5Xd2JOfSOtGZaf6V3qEQYg6EaDTAkMnCo=
 github.com/osmosis-labs/osmosis/x/epochs v0.0.0-20230328024000-175ec88e4304/go.mod h1:yPWoJTj5RKrXKUChAicp+G/4Ni/uVEpp27mi/FF/L9c=
 github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230602130523-f9a94d8bbd10 h1:XrES5AHZMZ/Y78boW35PTignkhN9h8VvJ1sP8EJDIu8=

osmoutils/accum/accum.go

@@ -134,14 +134,10 @@ func (accum *AccumulatorObject) NewPosition(name string, numShareUnits sdk.Dec,
 // It sets the position's accumulator to the given value of intervalAccumulationPerShare.
 // This is useful for when the accumulation happens at a sub-range of the full accumulator
 // rewards range. For example, a concentrated liquidity narrow range position.
-// All intervalAccumulationPerShare DecCoin values must be non-negative.
+// intervalAccumulationPerShare DecCoin values are allowed to be negative.
 // The position is initialized with empty unclaimed rewards
 // If there is an existing position for the given address, it is overwritten.
 func (accum *AccumulatorObject) NewPositionIntervalAccumulation(name string, numShareUnits sdk.Dec, intervalAccumulationPerShare sdk.DecCoins, options *Options) error {
-	if intervalAccumulationPerShare.IsAnyNegative() {
-		return NegativeIntervalAccumulationPerShareError{intervalAccumulationPerShare}
-	}
-
 	if err := options.validate(); err != nil {
 		return err
 	}

osmoutils/accum/accum_test.go

@@ -349,12 +349,16 @@ func (suite *AccumTestSuite) TestNewPositionIntervalAccumulation() {
 			},
 			options: &emptyPositionOptions,
 		},
-		"negative acc value - error": {
+		"negative acc value - no error": {
 			accObject:                    defaultAccObject,
 			name:                         testAddressOne,
 			numShareUnits:                positionOne.NumShares,
 			intervalAccumulationPerShare: defaultAccObject.GetValue().MulDec(sdk.NewDec(-1)),
-			expectedError:                accumPackage.NegativeIntervalAccumulationPerShareError{defaultAccObject.GetValue().MulDec(sdk.NewDec(-1))},
+			expectedPosition: accumPackage.Record{
+				NumShares:             positionOne.NumShares,
+				AccumValuePerShare:    defaultAccObject.GetValue().MulDec(sdk.NewDec(-1)),
+				UnclaimedRewardsTotal: emptyCoins,
+			},
 		},
 	}
 

osmoutils/coin_helper.go

@@ -45,6 +45,23 @@ func SubDecCoinArrays(decCoinsArrayA []sdk.DecCoins, decCoinsArrayB []sdk.DecCoi
 	return finalDecCoinArray, nil
 }
 
+// SafeSubDecCoinArrays subtracts the contents of the second param from the first (decCoinsArrayA - decCoinsArrayB)
+// Note that this takes in two _arrays_ of DecCoins, meaning that each term itself is of type DecCoins (i.e. an array of DecCoin).
+// Contrary to SubDecCoinArrays, this subtractions allows for negative result values.
+func SafeSubDecCoinArrays(decCoinsArrayA []sdk.DecCoins, decCoinsArrayB []sdk.DecCoins) ([]sdk.DecCoins, error) {
+	if len(decCoinsArrayA) != len(decCoinsArrayB) {
+		return []sdk.DecCoins{}, fmt.Errorf("DecCoin arrays must be of equal length to be subtracted")
+	}
+
+	finalDecCoinArray := []sdk.DecCoins{}
+	for i := range decCoinsArrayA {
+		subResult, _ := decCoinsArrayA[i].SafeSub(decCoinsArrayB[i])
+		finalDecCoinArray = append(finalDecCoinArray, subResult)
+	}
+
+	return finalDecCoinArray, nil
+}
+
 // AddDecCoinArrays adds the contents of the second param from the first (decCoinsArrayA + decCoinsArrayB)
 // Note that this takes in two _arrays_ of DecCoins, meaning that each term itself is of type DecCoins (i.e. an array of DecCoin).
 func AddDecCoinArrays(decCoinsArrayA []sdk.DecCoins, decCoinsArrayB []sdk.DecCoins) ([]sdk.DecCoins, error) {

osmoutils/coin_helper_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/osmosis-labs/osmosis/osmoutils"
+	"github.com/osmosis-labs/osmosis/osmoutils/osmoassert"
 )
 
 var (
@@ -32,6 +33,8 @@ func TestSubDecCoins(t *testing.T) {
 
 		expectedOutput []sdk.DecCoins
 		expectError    bool
+		// whether unsafe subtraction should panic
+		expectPanicUnsafe bool
 	}{
 		"[[100foo, 100bar], [100foo, 100bar]] - [[50foo, 50bar], [50foo, 100bar]]": {
 			firstInput:  []sdk.DecCoins{hundredEach, hundredEach},
@@ -69,19 +72,42 @@ func TestSubDecCoins(t *testing.T) {
 			expectedOutput: []sdk.DecCoins{},
 			expectError:    true,
 		},
+
+		"negative result": {
+			firstInput:  []sdk.DecCoins{fiftyEach},
+			secondInput: []sdk.DecCoins{hundredEach},
+
+			expectedOutput:    []sdk.DecCoins{{sdk.DecCoin{Denom: "bar", Amount: sdk.NewDec(-50)}, sdk.DecCoin{Denom: "foo", Amount: sdk.NewDec(-50)}}},
+			expectPanicUnsafe: true,
+		},
 	}
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			actualOutput, err := osmoutils.SubDecCoinArrays(tc.firstInput, tc.secondInput)
+
+			var (
+				actualOutput []sdk.DecCoins
+				err1         error
+			)
+			osmoassert.ConditionalPanic(t, tc.expectPanicUnsafe, func() {
+				actualOutput, err1 = osmoutils.SubDecCoinArrays(tc.firstInput, tc.secondInput)
+			})
+
+			actualOutputSafe, err2 := osmoutils.SafeSubDecCoinArrays(tc.firstInput, tc.secondInput)
 
 			if tc.expectError {
-				require.Error(t, err)
+				require.Error(t, err1)
+				require.Error(t, err2)
 				require.Equal(t, tc.expectedOutput, actualOutput)
+				require.Equal(t, tc.expectedOutput, actualOutputSafe)
 				return
 			}
 
-			require.NoError(t, err)
-			require.Equal(t, tc.expectedOutput, actualOutput)
+			require.NoError(t, err1)
+			require.NoError(t, err2)
+			if !tc.expectPanicUnsafe {
+				require.Equal(t, tc.expectedOutput, actualOutput)
+			}
+			require.Equal(t, tc.expectedOutput, actualOutputSafe)
 		})
 	}
 }