Skip to content

Commit

Permalink
Make accumulator objects more consistently pointers throughout (#5848)
Browse files Browse the repository at this point in the history 
* Make accumulator objects more consistently pointers throughout

* Update go mod

(cherry picked from commit 04596ca)

# Conflicts:
#	go.mod
#	go.sum
  • Loading branch information
@ValarDragon @mergify
ValarDragon authored and mergify[bot] committed Jul 18, 2023
1 parent 7285dd2 commit 1837192
Show file tree
Hide file tree
Showing 16 changed files with 95 additions and 74 deletions.
7 changes: 7 additions & 0 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,17 @@ require (
github.com/mattn/go-sqlite3 v1.14.17
github.com/ory/dockertest/v3 v3.10.0
github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3
<<<<<<< HEAD
github.com/osmosis-labs/osmosis/osmomath v0.0.5
github.com/osmosis-labs/osmosis/osmoutils v0.0.6-0.20230709040235-cbf530ed88cc
github.com/osmosis-labs/osmosis/x/epochs v0.0.1
github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.7
=======
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/x/epochs v0.0.0-20230328024000-175ec88e4304
github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230602130523-f9a94d8bbd10
>>>>>>> 04596cab (Make accumulator objects more consistently pointers throughout (#5848))
github.com/pkg/errors v0.9.1
github.com/rakyll/statik v0.1.7
github.com/spf13/cast v1.5.1
Expand Down
13 changes: 13 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,7 @@ github.com/osmosis-labs/cosmos-sdk v0.45.0-rc1.0.20230703010110-ed4eb883f2a6 h1:
github.com/osmosis-labs/cosmos-sdk v0.45.0-rc1.0.20230703010110-ed4eb883f2a6/go.mod h1:9KGhMg+7ZWgZ50Wa/x8w/jN19O0TSqYLlqUj+2wwxLU=
github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3 h1:YlmchqTmlwdWSmrRmXKR+PcU96ntOd8u10vTaTZdcNY=
github.com/osmosis-labs/go-mutesting v0.0.0-20221208041716-b43bcd97b3b3/go.mod h1:lV6KnqXYD/ayTe7310MHtM3I2q8Z6bBfMAi+bhwPYtI=
<<<<<<< HEAD
github.com/osmosis-labs/osmosis/osmomath v0.0.5 h1:veOu4VvnqMbJyjx2MEEFW53I9h3m8Kd3WN4hVvUyeIU=
github.com/osmosis-labs/osmosis/osmomath v0.0.5/go.mod h1:UlftwozB+QObT3o0YfkuuyL9fsVdgoWt0dm6J7MLYnU=
github.com/osmosis-labs/osmosis/osmoutils v0.0.6-0.20230709040235-cbf530ed88cc h1:NhjYKK0ADJCq5nmDfLMGRIAq8/QIhrStlt78+5xOZP4=
Expand All @@ -956,6 +957,18 @@ github.com/osmosis-labs/osmosis/x/epochs v0.0.1 h1:RCL3+W2s712dgtaKGgUHVt/6iJKfu
github.com/osmosis-labs/osmosis/x/epochs v0.0.1/go.mod h1:8dvJFHTpu6SIxmOaSaEw0tHnQ/Z9blf5qsF/ZJnMVHo=
github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.7 h1:rd5guXn/SF6i66PO5rlGaDK0AT81kCpiLixyQ5EJ6Yg=
github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.7/go.mod h1:sR0lpev9mcm9/9RY50T1og3UC3WpZAsINh/OmgrmFlg=
=======
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/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=
github.com/osmosis-labs/osmosis/x/ibc-hooks v0.0.0-20230602130523-f9a94d8bbd10/go.mod h1:Ln6CKcXg/CJLSBE6Fd96/MIKPyA4iHuQTKSbl9q7vYo=
>>>>>>> 04596cab (Make accumulator objects more consistently pointers throughout (#5848))
github.com/osmosis-labs/wasmd v0.31.0-osmo-v16 h1:X747cZYdnqc/+RV48iPVeGprpVb/fUWSaKGsZUWrdbg=
github.com/osmosis-labs/wasmd v0.31.0-osmo-v16/go.mod h1:Rf8zW/GgBQyFRRB4s62VQHWA6sTlMFSjoDQQpoq64iI=
github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw=
Expand Down
37 changes: 19 additions & 18 deletions osmoutils/accum/accum.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,19 @@ func MakeAccumulatorWithValueAndShare(accumStore store.KVStore, accumName string
}

// Gets the current value of the accumulator corresponding to accumName in accumStore
func GetAccumulator(accumStore store.KVStore, accumName string) (AccumulatorObject, error) {
func GetAccumulator(accumStore store.KVStore, accumName string) (*AccumulatorObject, error) {
accumContent := AccumulatorContent{}
found, err := osmoutils.Get(accumStore, formatAccumPrefixKey(accumName), &accumContent)
if err != nil {
return AccumulatorObject{}, err
return &AccumulatorObject{}, err
}
if !found {
return AccumulatorObject{}, AccumDoesNotExistError{AccumName: accumName}
return &AccumulatorObject{}, AccumDoesNotExistError{AccumName: accumName}
}

accum := AccumulatorObject{accumStore, accumName, accumContent.AccumValue, accumContent.TotalShares}

return accum, nil
return &accum, nil
}

// MustGetPosition returns the position associated with the given address. No errors in position retrieval are allowed.
Expand Down Expand Up @@ -146,7 +146,7 @@ func (accum *AccumulatorObject) NewPositionIntervalAccumulation(name string, num
return err
}

initOrUpdatePosition(*accum, intervalAccumulationPerShare, name, numShareUnits, sdk.NewDecCoins(), options)
initOrUpdatePosition(accum, intervalAccumulationPerShare, name, numShareUnits, sdk.NewDecCoins(), options)

// Update total shares in accum (re-fetch accum from state to ensure it's up to date)
updatedAccum, err := GetAccumulator(accum.store, accum.name)
Expand Down Expand Up @@ -199,21 +199,21 @@ func (accum *AccumulatorObject) AddToPositionIntervalAccumulation(name string, n
}

// Get addr's current position
position, err := GetPosition(*accum, name)
position, err := GetPosition(accum, name)
if err != nil {
return err
}

// Save current number of shares and unclaimed rewards
unclaimedRewards := GetTotalRewards(*accum, position)
unclaimedRewards := GetTotalRewards(accum, position)
oldNumShares, err := accum.GetPositionSize(name)
if err != nil {
return 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
initOrUpdatePosition(*accum, intervalAccumulationPerShare, name, oldNumShares.Add(newShares), unclaimedRewards, position.Options)
initOrUpdatePosition(accum, intervalAccumulationPerShare, name, oldNumShares.Add(newShares), unclaimedRewards, position.Options)

// Update total shares in accum (re-fetch accum from state to ensure it's up to date)
updatedAccum, err := GetAccumulator(accum.store, accum.name)
Expand Down Expand Up @@ -247,7 +247,7 @@ func (accum *AccumulatorObject) RemoveFromPositionIntervalAccumulation(name stri
}

// Get addr's current position
position, err := GetPosition(*accum, name)
position, err := GetPosition(accum, name)
if err != nil {
return err
}
Expand All @@ -258,14 +258,14 @@ func (accum *AccumulatorObject) RemoveFromPositionIntervalAccumulation(name stri
}

// Save current number of shares and unclaimed rewards
unclaimedRewards := GetTotalRewards(*accum, position)
unclaimedRewards := GetTotalRewards(accum, position)
oldNumShares, err := accum.GetPositionSize(name)
if err != nil {
return err
}

// Update user's position with new number of shares
initOrUpdatePosition(*accum, intervalAccumulationPerShare, name, oldNumShares.Sub(numSharesToRemove), unclaimedRewards, position.Options)
initOrUpdatePosition(accum, intervalAccumulationPerShare, name, oldNumShares.Sub(numSharesToRemove), unclaimedRewards, position.Options)

updatedAccum, err := GetAccumulator(accum.store, accum.name)
if err != nil {
Expand Down Expand Up @@ -313,14 +313,14 @@ func (accum *AccumulatorObject) UpdatePositionIntervalAccumulation(name string,
// Returns nil on success, error otherwise.
func (accum *AccumulatorObject) SetPositionIntervalAccumulation(name string, intervalAccumulationPerShare sdk.DecCoins) error {
// Get addr's current position
position, err := GetPosition(*accum, name)
position, err := GetPosition(accum, name)
if err != nil {
return err
}

// Update the user's position with the new accumulator value. The unclaimed rewards, options, and
// the number of shares stays the same as in the original position.
initOrUpdatePosition(*accum, intervalAccumulationPerShare, name, position.NumShares, position.UnclaimedRewardsTotal, position.Options)
initOrUpdatePosition(accum, intervalAccumulationPerShare, name, position.NumShares, position.UnclaimedRewardsTotal, position.Options)

return nil
}
Expand Down Expand Up @@ -366,7 +366,7 @@ func (accum AccumulatorObject) deletePosition(positionName string) {
// GetPositionSize returns the number of shares the position with the given
// name has in the accumulator. Returns error if position does not exist
// or if fails to retrieve position from state.
func (accum AccumulatorObject) GetPositionSize(name string) (sdk.Dec, error) {
func (accum *AccumulatorObject) GetPositionSize(name string) (sdk.Dec, error) {
position, err := GetPosition(accum, name)
if err != nil {
return sdk.Dec{}, err
Expand Down Expand Up @@ -400,7 +400,7 @@ func (accum AccumulatorObject) GetValue() sdk.DecCoins {
// Returns error if
// - no position exists for the given address
// - any database errors occur.
func (accum AccumulatorObject) ClaimRewards(positionName string) (sdk.Coins, sdk.DecCoins, error) {
func (accum *AccumulatorObject) ClaimRewards(positionName string) (sdk.Coins, sdk.DecCoins, error) {
position, err := GetPosition(accum, positionName)
if err != nil {
return sdk.Coins{}, sdk.DecCoins{}, NoPositionError{positionName}
Expand All @@ -426,14 +426,15 @@ func (accum AccumulatorObject) ClaimRewards(positionName string) (sdk.Coins, sdk

// GetTotalShares returns the total number of shares in the accumulator
func (accum AccumulatorObject) GetTotalShares() (sdk.Dec, error) {
accum, err := GetAccumulator(accum.store, accum.name)
return accum.totalShares, err
// TODO: Make this not do an extra get.
accumPtr, err := GetAccumulator(accum.store, accum.name)
return accumPtr.totalShares, err
}

// AddToUnclaimedRewards adds the given amount of rewards to the unclaimed rewards
// for the given position. Returns error if no position exists for the given position name.
// Returns error if any database errors occur or if neggative rewards are provided.
func (accum AccumulatorObject) AddToUnclaimedRewards(positionName string, rewardsToAddTotal sdk.DecCoins) error {
func (accum *AccumulatorObject) AddToUnclaimedRewards(positionName string, rewardsToAddTotal sdk.DecCoins) error {
position, err := GetPosition(accum, positionName)
if err != nil {
return err
Expand Down
6 changes: 3 additions & 3 deletions osmoutils/accum/accum_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

// initOrUpdatePosition creates a new position or override an existing position
// at accumulator's current value with a specific number of shares and unclaimed rewards
func initOrUpdatePosition(accum AccumulatorObject, accumulatorValuePerShare sdk.DecCoins, index string, numShareUnits sdk.Dec, unclaimedRewardsTotal sdk.DecCoins, options *Options) {
func initOrUpdatePosition(accum *AccumulatorObject, accumulatorValuePerShare sdk.DecCoins, index string, numShareUnits sdk.Dec, unclaimedRewardsTotal sdk.DecCoins, options *Options) {
position := Record{
NumShares: numShareUnits,
AccumValuePerShare: accumulatorValuePerShare,
Expand All @@ -19,7 +19,7 @@ func initOrUpdatePosition(accum AccumulatorObject, accumulatorValuePerShare sdk.
}

// Gets addr's current position from store
func GetPosition(accum AccumulatorObject, name string) (Record, error) {
func GetPosition(accum *AccumulatorObject, name string) (Record, error) {
position := Record{}
found, err := osmoutils.Get(accum.store, FormatPositionPrefixKey(accum.name, name), &position)
if err != nil {
Expand All @@ -33,7 +33,7 @@ func GetPosition(accum AccumulatorObject, name string) (Record, error) {
}

// Gets total unclaimed rewards, including existing and newly accrued unclaimed rewards
func GetTotalRewards(accum AccumulatorObject, position Record) sdk.DecCoins {
func GetTotalRewards(accum *AccumulatorObject, position Record) sdk.DecCoins {
totalRewards := position.UnclaimedRewardsTotal

// TODO: add a check that accum.value is greater than position.InitAccumValue
Expand Down
38 changes: 19 additions & 19 deletions osmoutils/accum/accum_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,21 @@ type AccumTestSuite struct {
store store.KVStore
}

func (suite *AccumTestSuite) GetAccumulator(name string) accumPackage.AccumulatorObject {
func (suite *AccumTestSuite) GetAccumulator(name string) *accumPackage.AccumulatorObject {
accum, err := accumPackage.GetAccumulator(suite.store, name)
suite.Require().NoError(err)
return accum
}

func (suite *AccumTestSuite) MakeAndGetAccumulator(name string) accumPackage.AccumulatorObject {
func (suite *AccumTestSuite) MakeAndGetAccumulator(name string) *accumPackage.AccumulatorObject {
err := accumPackage.MakeAccumulator(suite.store, name)
suite.Require().NoError(err)
accum, err := accumPackage.GetAccumulator(suite.store, name)
suite.Require().NoError(err)
return accum
}

func (suite *AccumTestSuite) TotalSharesCheck(accum accumPackage.AccumulatorObject, expected sdk.Dec) {
func (suite *AccumTestSuite) TotalSharesCheck(accum *accumPackage.AccumulatorObject, expected sdk.Dec) {
shareCount, err := accum.GetTotalShares()
suite.Require().NoError(err)
suite.Require().Equal(expected.String(), shareCount.String())
Expand Down Expand Up @@ -245,32 +245,32 @@ func (suite *AccumTestSuite) TestNewPosition() {
expectedPosition accumPackage.Record
}{
"test address one - position created": {
accObject: &defaultAccObject,
accObject: defaultAccObject,
name: testAddressOne,
numShareUnits: positionOne.NumShares,
expectedPosition: positionOne,
},
"test address two (non-nil options) - position created": {
accObject: &defaultAccObject,
accObject: defaultAccObject,
name: testAddressTwo,
numShareUnits: positionTwo.NumShares,
expectedPosition: positionTwo,
options: &emptyPositionOptions,
},
"test address one - position overwritten": {
accObject: &defaultAccObject,
accObject: defaultAccObject,
name: testAddressOne,
numShareUnits: positionOneV2.NumShares,
expectedPosition: positionOneV2,
},
"test address three - added": {
accObject: &defaultAccObject,
accObject: defaultAccObject,
name: testAddressThree,
numShareUnits: positionThree.NumShares,
expectedPosition: positionThree,
},
"test address one with non-empty accumulator - position created": {
accObject: &nonEmptyAccObject,
accObject: nonEmptyAccObject,
name: testAddressOne,
numShareUnits: positionOne.NumShares,
expectedPosition: withInitialAccumValue(positionOne, initialCoinsDenomOne),
Expand All @@ -297,7 +297,7 @@ func (suite *AccumTestSuite) TestNewPosition() {
// ensure receiver was mutated
suite.Require().Equal(expectedAccValue, tc.accObject.GetTotalShareField())
// ensure state was mutated
suite.TotalSharesCheck(*tc.accObject, expectedAccValue)
suite.TotalSharesCheck(tc.accObject, expectedAccValue)

if tc.options == nil {
suite.Require().Nil(position.Options)
Expand Down Expand Up @@ -327,7 +327,7 @@ func (suite *AccumTestSuite) TestNewPositionIntervalAccumulation() {
expectedError error
}{
"interval acc value equals to acc": {
accObject: &defaultAccObject,
accObject: defaultAccObject,
name: testAddressOne,
numShareUnits: positionOne.NumShares,
intervalAccumulationPerShare: defaultAccObject.GetValue(),
Expand All @@ -338,7 +338,7 @@ func (suite *AccumTestSuite) TestNewPositionIntervalAccumulation() {
},
},
"interval acc value does not equal to acc": {
accObject: &defaultAccObject,
accObject: defaultAccObject,
name: testAddressTwo,
numShareUnits: positionTwo.NumShares,
intervalAccumulationPerShare: defaultAccObject.GetValue().MulDec(sdk.NewDec(2)),
Expand All @@ -350,7 +350,7 @@ func (suite *AccumTestSuite) TestNewPositionIntervalAccumulation() {
options: &emptyPositionOptions,
},
"negative acc value - error": {
accObject: &defaultAccObject,
accObject: defaultAccObject,
name: testAddressOne,
numShareUnits: positionOne.NumShares,
intervalAccumulationPerShare: defaultAccObject.GetValue().MulDec(sdk.NewDec(-1)),
Expand Down Expand Up @@ -384,7 +384,7 @@ func (suite *AccumTestSuite) TestNewPositionIntervalAccumulation() {
// ensure receiver was mutated
suite.Require().Equal(expectedAccValue, tc.accObject.GetTotalShareField())
// ensure state was mutated
suite.TotalSharesCheck(*tc.accObject, expectedAccValue)
suite.TotalSharesCheck(tc.accObject, expectedAccValue)
if tc.options == nil {
suite.Require().Nil(position.Options)
return
Expand Down Expand Up @@ -447,7 +447,7 @@ func (suite *AccumTestSuite) TestClaimRewards() {

tests := []struct {
testName string
accObject accumPackage.AccumulatorObject
accObject *accumPackage.AccumulatorObject
accName string
expectedResult sdk.Coins
updateNumSharesToZero bool
Expand Down Expand Up @@ -760,7 +760,7 @@ func (suite *AccumTestSuite) TestAddToPositionIntervalAccumulation() {
expectedError error
}{
"interval acc value equals to acc": {
accObject: &accObject,
accObject: accObject,
name: testAddressOne,
numShareUnits: positionOne.NumShares,
intervalAccumulationPerShare: accObject.GetValue(),
Expand All @@ -771,7 +771,7 @@ func (suite *AccumTestSuite) TestAddToPositionIntervalAccumulation() {
},
},
"interval acc value does not equal to acc": {
accObject: &accObject,
accObject: accObject,
name: testAddressTwo,
numShareUnits: positionTwo.NumShares,
intervalAccumulationPerShare: accObject.GetValue().MulDec(sdk.NewDec(2)),
Expand Down Expand Up @@ -814,7 +814,7 @@ func (suite *AccumTestSuite) TestAddToPositionIntervalAccumulation() {
// ensure receiver was mutated
suite.Require().Equal(expectedAccValue, tc.accObject.GetTotalShareField())
// ensure state was mutated
suite.TotalSharesCheck(*tc.accObject, expectedAccValue)
suite.TotalSharesCheck(tc.accObject, expectedAccValue)
})
}
}
Expand Down Expand Up @@ -1030,7 +1030,7 @@ func (suite *AccumTestSuite) TestRemoveFromPositionIntervalAccumulation() {
accObject := accumPackage.MakeTestAccumulator(suite.store, testNameOne, baseAccumValue, emptyDec)

tests := map[string]struct {
accObject accumPackage.AccumulatorObject
accObject *accumPackage.AccumulatorObject
name string
numShareUnits sdk.Dec
intervalAccumulationPerShare sdk.DecCoins
Expand Down Expand Up @@ -1559,7 +1559,7 @@ func (suite *AccumTestSuite) TestGetTotalShares() {

// Run a number of NewPosition, AddToPosition, and RemoveFromPosition operations on each accum
testAddresses := []string{testAddressOne, testAddressTwo, testAddressThree}
accums := []accumPackage.AccumulatorObject{accumOne, accumTwo}
accums := []*accumPackage.AccumulatorObject{accumOne, accumTwo}
expectedShares := []sdk.Dec{sdk.OneDec(), sdk.ZeroDec()}

for i := 1; i <= 10; i++ {
Expand Down
Loading

0 comments on commit 1837192

Please sign in to comment.