feat(osmoutils): custom accumulator position updates

osmoutils/accum/accum.go

@@ -77,10 +77,27 @@ func (accum *AccumulatorObject) UpdateAccumulator(amt sdk.DecCoins) {
 // The position is initialized with empty unclaimed rewards
 // If there is an existing position for the given address, it is overwritten.
 func (accum AccumulatorObject) NewPosition(name string, numShareUnits sdk.Dec, options *Options) error {
+	return accum.newPosition(name, numShareUnits, accum.value, options)
+}
+
+// NewPosition creates a new position for the given name, with the given number of share units.
+// The name can be an owner's address, or any other unique identifier for a position.
+// It sets the position's accumulator to the given value of customAccumulatorValue.
+// All custom accumulator values must be non-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) NewPositionCustomAcc(name string, numShareUnits sdk.Dec, customAccumulatorValue sdk.DecCoins, options *Options) error {
+	if customAccumulatorValue.IsAnyNegative() {
+		return NegativeCustomAccError{customAccumulatorValue}
+	}
+	return accum.newPosition(name, numShareUnits, customAccumulatorValue, options)
+}
+
+func (accum AccumulatorObject) newPosition(name string, numShareUnits sdk.Dec, positionAccumulatorInit sdk.DecCoins, options *Options) error {
 	if err := options.validate(); err != nil {
 		return err
 	}
-	createNewPosition(accum, name, numShareUnits, sdk.NewDecCoins(), options)
+	createNewPosition(accum, positionAccumulatorInit, name, numShareUnits, sdk.NewDecCoins(), options)
 	return nil
 }
 
@@ -99,6 +116,30 @@ func (accum AccumulatorObject) NewPosition(name string, numShareUnits sdk.Dec, o
 // - there is no existing position at the given address
 // - other internal or database error occurs.
 func (accum AccumulatorObject) AddToPosition(name string, newShares sdk.Dec) error {
+	return accum.addToPosition(name, newShares, accum.value)
+}
+
+// AddToPosition adds newShares of shares to an existing position with the given name.
+// This is functionally equivalent to claiming rewards, closing down the position, and
+// opening a fresh one with the new number of shares. We can represent this behavior by
+// claiming rewards. The accumulator of the new position is set to given customAccumulatorValue.
+// All custom accumulator values must be non-negative. They must also be a superset of the
+// old accumulator value associated with the position.
+//
+// An alternative approach is to simply generate an additional position every time an
+// address adds to its position. We do not pursue this path because we want to ensure
+// that withdrawal and claiming functions remain constant time and do not scale with the
+// number of times a user has added to their position.
+//
+// Returns nil on success. Returns error when:
+// - newShares are negative or zero.
+// - there is no existing position at the given address
+// - other internal or database error occurs.
+func (accum AccumulatorObject) AddToPositionCustomAcc(name string, newShares sdk.Dec, customAccumulatorValue sdk.DecCoins) error {
+	return accum.addToPosition(name, newShares, customAccumulatorValue)
+}
+
+func (accum AccumulatorObject) addToPosition(name string, newShares sdk.Dec, accumulatorUpdate sdk.DecCoins) error {
 	if !newShares.IsPositive() {
 		return errors.New("Attempted to add a non-zero and non-negative number of shares to a position")
 	}
@@ -109,6 +150,10 @@ func (accum AccumulatorObject) AddToPosition(name string, newShares sdk.Dec) err
 		return err
 	}
 
+	if err := validateAccumulatorValue(accumulatorUpdate, position.InitAccumValue); err != nil {
+		return err
+	}
+
 	// Save current number of shares and unclaimed rewards
 	unclaimedRewards := getTotalRewards(accum, position)
 	oldNumShares, err := accum.GetPositionSize(name)
@@ -118,7 +163,7 @@ func (accum AccumulatorObject) AddToPosition(name string, newShares sdk.Dec) err
 
 	// Update user's position with new number of shares while moving its unaccrued rewards
 	// into UnclaimedRewards. Starting accumulator value is moved up to accum'scurrent value
-	createNewPosition(accum, name, oldNumShares.Add(newShares), unclaimedRewards, position.Options)
+	createNewPosition(accum, accumulatorUpdate, name, oldNumShares.Add(newShares), unclaimedRewards, position.Options)
 
 	return nil
 }
@@ -128,6 +173,20 @@ func (accum AccumulatorObject) AddToPosition(name string, newShares sdk.Dec) err
 // overwrites the position record with the updated number of shares. Since it accrues rewards, it
 // also moves up the position's accumulator value to the current accum val.
 func (accum AccumulatorObject) RemoveFromPosition(name string, numSharesToRemove sdk.Dec) error {
+	return accum.removeFromPosition(name, numSharesToRemove, accum.value)
+}
+
+// RemovePositionCustomAcc removes the specified number of shares from a position. Specifically, it claims
+// the unclaimed and newly accrued rewards and returns them alongside the redeemed shares. Then, it
+// overwrites the position record with the updated number of shares. Since it accrues rewards, it
+// also resets the position's accumulator value to the given customAccumulatorValue.
+// All custom accumulator values must be non-negative. They must also be a superset of the
+// old accumulator value associated with the position.
+func (accum AccumulatorObject) RemoveFromPositionCustomAcc(name string, numSharesToRemove sdk.Dec, customAccumulatorValue sdk.DecCoins) error {
+	return accum.removeFromPosition(name, numSharesToRemove, customAccumulatorValue)
+}
+
+func (accum AccumulatorObject) removeFromPosition(name string, numSharesToRemove sdk.Dec, accumulatorUpdate sdk.DecCoins) error {
 	// Cannot remove zero or negative shares
 	if numSharesToRemove.LTE(sdk.ZeroDec()) {
 		return fmt.Errorf("Attempted to remove no/negative shares (%s)", numSharesToRemove)
@@ -139,6 +198,10 @@ func (accum AccumulatorObject) RemoveFromPosition(name string, numSharesToRemove
 		return err
 	}
 
+	if err := validateAccumulatorValue(accumulatorUpdate, position.InitAccumValue); err != nil {
+		return err
+	}
+
 	// Ensure not removing more shares than exist
 	if numSharesToRemove.GT(position.NumShares) {
 		return fmt.Errorf("Attempted to remove more shares  (%s) than exist in the position (%s)", numSharesToRemove, position.NumShares)
@@ -151,13 +214,45 @@ func (accum AccumulatorObject) RemoveFromPosition(name string, numSharesToRemove
 		return err
 	}
 
-	createNewPosition(accum, name, oldNumShares.Sub(numSharesToRemove), unclaimedRewards, position.Options)
+	createNewPosition(accum, accumulatorUpdate, name, oldNumShares.Sub(numSharesToRemove), unclaimedRewards, position.Options)
 
 	return nil
 }
 
-// GetPositionSize returns the number of shares the position corresponding to postion's name
-// or an error if no position exists.
+// UpdatePosition updates the position with the given name by adding or removing
+// the given number of shares. If numShares is positive, it is equivalent to calling
+// AddToPosition. If numShares is negative, it is equivalent to calling RemoveFromPosition.
+// Also, it moves up the position's accumulator value to the current accum value.
+// Fails with error if numShares is zero. Returns nil on success.
+func (accum AccumulatorObject) UpdatePosition(name string, numShares sdk.Dec) error {
+	return accum.updatePosition(name, numShares, accum.value)
+}
+
+// UpdatePositionCustomAcc updates the position with the given name by adding or removing
+// the given number of shares. If numShares is positive, it is equivalent to calling
+// AddToPositionCustomAcc. If numShares is negative, it is equivalent to calling RemoveFromPositionCustomAcc.
+// Fails with error if numShares is zero. Returns nil on success.
+// It also resets the position's accumulator value to the given customAccumulatorValue.
+// All custom accumulator values must be non-negative. They must also be a superset of the
+// old accumulator value associated with the position.
+func (accum AccumulatorObject) UpdatePositionCustomAcc(name string, numShares sdk.Dec, customAccumulatorValue sdk.DecCoins) error {
+	return accum.updatePosition(name, numShares, customAccumulatorValue)
+}
+
+func (accum AccumulatorObject) updatePosition(name string, numShares sdk.Dec, positionAccumulatorUpdate sdk.DecCoins) error {
+	if numShares.Equal(sdk.ZeroDec()) {
+		return ZeroSharesError
+	}
+
+	if numShares.IsNegative() {
+		return accum.RemoveFromPositionCustomAcc(name, numShares.Neg(), positionAccumulatorUpdate)
+	}
+
+	return accum.AddToPositionCustomAcc(name, numShares, positionAccumulatorUpdate)
+}
+
+// GetPositionSize returns the number of shares the position corresponding to `addr`
+// in accumulator `accum` has, or an error if no position exists.
 func (accum AccumulatorObject) GetPositionSize(name string) (sdk.Dec, error) {
 	position, err := getPosition(accum, name)
 	if err != nil {
@@ -192,7 +287,7 @@ func (accum AccumulatorObject) ClaimRewards(positionName string) (sdk.Coins, err
 
 	// Create a completely new position, with no rewards
 	// TODO: remove the position from state entirely if numShares = zero
-	createNewPosition(accum, positionName, position.NumShares, sdk.NewDecCoins(), position.Options)
+	createNewPosition(accum, accum.value, positionName, position.NumShares, sdk.NewDecCoins(), position.Options)
 
 	return truncatedRewards, nil
 }

osmoutils/accum/accum_helpers.go

@@ -6,11 +6,15 @@ import (
 	"github.com/osmosis-labs/osmosis/osmoutils"
 )
 
+var (
+	minusOne = sdk.NewDec(-1)
+)
+
 // Creates a new position at accumulator's current value with a specific number of shares and unclaimed rewards
-func createNewPosition(accum AccumulatorObject, index string, numShareUnits sdk.Dec, unclaimedRewards sdk.DecCoins, options *Options) {
+func createNewPosition(accum AccumulatorObject, accumulatorValue sdk.DecCoins, index string, numShareUnits sdk.Dec, unclaimedRewards sdk.DecCoins, options *Options) {
 	position := Record{
 		NumShares:        numShareUnits,
-		InitAccumValue:   accum.value,
+		InitAccumValue:   accumulatorValue,
 		UnclaimedRewards: unclaimedRewards,
 		Options:          options,
 	}
@@ -35,8 +39,24 @@ func getPosition(accum AccumulatorObject, name string) (Record, error) {
 func getTotalRewards(accum AccumulatorObject, position Record) sdk.DecCoins {
 	totalRewards := position.UnclaimedRewards
 
+	// TODO: add a check that accum.value is greater than position.InitAccumValue
 	accumulatorRewards := accum.value.Sub(position.InitAccumValue).MulDec(position.NumShares)
 	totalRewards = totalRewards.Add(accumulatorRewards...)
 
 	return totalRewards
 }
+
+// validateAccumulatorValue validates the provided accumulator.
+// All coins in custom accumulator value must be non-negative.
+// Custom accumulator value must be a suoerset of the old accumulator value.
+// Fails if any coin is negative. On success, returns nil.
+func validateAccumulatorValue(customAccumulatorValue, oldPositionAccumulatorValue sdk.DecCoins) error {
+	if customAccumulatorValue.IsAnyNegative() {
+		return NegativeCustomAccError{customAccumulatorValue}
+	}
+	newValue, IsAnyNegative := customAccumulatorValue.SafeSub(oldPositionAccumulatorValue)
+	if IsAnyNegative {
+		return NegativeAccDifferenceError{newValue.MulDec(minusOne)}
+	}
+	return nil
+}

osmoutils/accum/accum_helpers_test.go

@@ -0,0 +1,50 @@
+package accum_test
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/osmosis-labs/osmosis/osmoutils/accum"
+)
+
+func (suite *AccumTestSuite) TestValidateAccumulatorValue() {
+	tests := map[string]struct {
+		customAccumulatorValue      sdk.DecCoins
+		oldPositionAccumulatorValue sdk.DecCoins
+		expectError                 error
+	}{
+		"negative custom coins - error": {
+			customAccumulatorValue:      initialCoinsDenomOne.MulDec(sdk.NewDec(-1)),
+			oldPositionAccumulatorValue: emptyCoins,
+			expectError:                 accum.NegativeCustomAccError{initialCoinsDenomOne.MulDec(sdk.NewDec(-1))},
+		},
+		"old accumulator coins are greater than new - error": {
+			customAccumulatorValue:      initialCoinsDenomOne,
+			oldPositionAccumulatorValue: initialCoinsDenomOne.Add(sdk.NewDecCoin(initialCoinDenomOne.Denom, sdk.OneInt())),
+			expectError:                 accum.NegativeAccDifferenceError{sdk.NewDecCoins(sdk.NewDecCoin(initialCoinDenomOne.Denom, sdk.OneInt()))},
+		},
+		"old accumulator coins are a superset of new - error": {
+			customAccumulatorValue:      initialCoinsDenomOne,
+			oldPositionAccumulatorValue: initialCoinsDenomOne.Add(initialCoinDenomTwo),
+			expectError:                 accum.NegativeAccDifferenceError{sdk.NewDecCoins(initialCoinDenomTwo)},
+		},
+		"new accumulator coins are a superset of old - success": {
+			customAccumulatorValue:      initialCoinsDenomOne.Add(initialCoinDenomTwo),
+			oldPositionAccumulatorValue: initialCoinsDenomOne,
+		},
+	}
+
+	for name, tc := range tests {
+		suite.Run(name, func() {
+			suite.SetupTest()
+
+			err := accum.ValidateAccumulatorValue(tc.customAccumulatorValue, tc.oldPositionAccumulatorValue)
+
+			if tc.expectError != nil {
+				suite.Require().Error(err)
+				suite.Require().Equal(tc.expectError, err)
+				return
+			}
+			suite.Require().NoError(err)
+		})
+	}
+}