From f37db194c3cdaeeaba74e576351124435d9845b4 Mon Sep 17 00:00:00 2001 From: mconcat Date: Tue, 5 Apr 2022 01:08:21 +0900 Subject: [PATCH 01/14] add governance support for superfluid --- x/superfluid/keeper/stake.go | 53 ++++++++++++ x/superfluid/keeper/stake_test.go | 113 +++++++++++++++++++++++++ x/superfluid/types/expected_keepers.go | 4 + 3 files changed, 170 insertions(+) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 8dea0bfb182..82696234a8f 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -349,3 +349,56 @@ func (k Keeper) forceUndelegateAndBurnOsmoTokens(ctx sdk.Context, // Eugen’s point: Only rewards message needs to be updated. Rest of messages are fine // Queries need to be updated // We can do this at the very end though, since it just relates to queries. + +// IterateBondedValidatorsByPower implements govtypes.StakingKeeper +func (k Keeper) IterateBondedValidatorsByPower(ctx sdk.Context, fn func(int64, stakingtypes.ValidatorI) bool) { + k.sk.IterateBondedValidatorsByPower(ctx, fn) +} + +// TotalBondedTokens implements govtypes.StakingKeeper +func (k Keeper) TotalBondedTokens(ctx sdk.Context) sdk.Int { + return k.sk.TotalBondedTokens(ctx) +} + +// IterateDelegations implements govtypes.StakingKeeper +func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(int64, stakingtypes.DelegationI) bool) { + // we tract the global index + synthlocks := k.lk.GetAllSyntheticLockupsByAddr(ctx, delegator) + for i, lock := range synthlocks { + interm, ok := k.GetIntermediaryAccountFromLockId(ctx, lock.UnderlyingLockId) + if !ok { + panic(1234) // XXX + } + lock, err := k.lk.GetLockByID(ctx, lock.UnderlyingLockId) + if err != nil { + panic(err) + } + coin, err := lock.SingleCoin() + if err != nil { + panic(err) + } + amount := k.GetSuperfluidOSMOTokens(ctx, interm.Denom, coin.Amount) + valAddr, err := sdk.ValAddressFromBech32(interm.ValAddr) + if err != nil { + panic(err) + } + validator, found := k.sk.GetValidator(ctx, valAddr) + if !found { + panic(1234) // XXX: could this happen? + } + shares, err := validator.SharesFromTokens(amount) + if err != nil { + panic(err) // XXX: should we just continue? when err happens? figure out + } + delegation := stakingtypes.Delegation{ + DelegatorAddress: delegator.String(), + ValidatorAddress: interm.ValAddr, + Shares: shares, + } + fn(int64(i), delegation) + } + + k.sk.IterateDelegations(ctx, delegator, func(i int64, delegation stakingtypes.DelegationI) (stop bool) { + return fn(int64(len(synthlocks))+i, delegation) // add len(synthlocks) to index to avoid overlapping + }) +} diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index 4546f623924..30e40a836d2 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -894,3 +895,115 @@ func (suite *KeeperTestSuite) TestRefreshIntermediaryDelegationAmounts() { }) } } + +func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { + testCases := []struct { + name string + validatorStats []stakingtypes.BondStatus + superDelegations [][]superfluidDelegation + }{ + { + "with single validator and single delegation", + []stakingtypes.BondStatus{stakingtypes.Bonded}, + [][]superfluidDelegation{{{0, 0, 0, 1000000}}}, + }, + { + "with single validator and additional delegations", + []stakingtypes.BondStatus{stakingtypes.Bonded}, + [][]superfluidDelegation{{{0, 0, 0, 1000000}, {0, 0, 0, 1000000}}}, + }, + { + "with multiple validator and multiple superfluid delegations", + []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, + [][]superfluidDelegation{{{0, 0, 0, 1000000}}, {{1, 1, 0, 1000000}}}, + }, + { + "with single validator and multiple denom superfluid delegations", + []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, + [][]superfluidDelegation{{{0, 0, 0, 1000000}, {0, 0, 1, 1000000}}}, + }, + { + "with multiple validators and multiple denom superfluid delegations", + []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, + [][]superfluidDelegation{{{0, 0, 0, 1000000}, {0, 1, 1, 1000000}}}, + }, + { + "many delegations", + []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, + [][]superfluidDelegation{ + {{0, 0, 0, 1000000}, {0, 1, 1, 1000000}}, + {{1, 0, 0, 1000000}, {1, 0, 1, 1000000}}, + {{2, 1, 1, 1000000}, {2, 1, 0, 1000000}}, + {{3, 0, 0, 1000000}, {3, 1, 1, 1000000}}, + }, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() + + // Generate delegator addresses + delAddrs := CreateRandomAccounts(len(tc.superDelegations)) + + // setup validators + valAddrs := suite.SetupValidators(tc.validatorStats) + + denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(20), sdk.NewDec(20)}) + + // setup superfluid delegations + for _, sfdel := range tc.superDelegations { + fmt.Println(sfdel) + intermediaryAccs, _ := suite.SetupSuperfluidDelegations(delAddrs, valAddrs, sfdel, denoms) + suite.checkIntermediaryAccountDelegations(intermediaryAccs) + } + + // all expected delegated amounts to a validator from a delegator + delegatedAmount := func(delidx, validx int) sdk.Int { + res := sdk.ZeroInt() + for _, del := range tc.superDelegations[delidx] { + if del.valIndex == int64(validx) { + res = res.AddRaw(del.lpAmount) + } + } + return res + } + /* + // total delegated amount to this validator. used to calculate share. + totalDelegation := func(validx int) sdk.Int { + res := sdk.ZeroInt() + for i := range tc.superDelegations { + res = res.Add(delegatedAmount(i, validx)) + } + return res + } + */ + for delidx := range tc.superDelegations { + // map to store all actual delegations to a validator + sharePerValidatorMap := make(map[string]sdk.Dec) + for validx := range tc.validatorStats { + sharePerValidatorMap[valAddrs[validx].String()] = sdk.ZeroDec() + } + addToSharePerValidatorMap := func(val sdk.ValAddress, share sdk.Dec) { + if existing, ok := sharePerValidatorMap[val.String()]; ok { + share.AddMut(existing) + } + sharePerValidatorMap[val.String()] = share + } + + // iterate delegations + suite.App.SuperfluidKeeper.IterateDelegations(suite.Ctx, delAddrs[delidx], func(_ int64, del stakingtypes.DelegationI) bool { + addToSharePerValidatorMap(del.GetValidatorAddr(), del.GetShares()) + return false + }) + + // check if the expected equals to actual + for validx := range tc.validatorStats { + suite.Equal(delegatedAmount(delidx, validx).Int64(), sharePerValidatorMap[valAddrs[validx].String()].RoundInt().Int64()) + } + } + }) + } +} diff --git a/x/superfluid/types/expected_keepers.go b/x/superfluid/types/expected_keepers.go index 1705a34511f..88b501a7cd7 100644 --- a/x/superfluid/types/expected_keepers.go +++ b/x/superfluid/types/expected_keepers.go @@ -68,6 +68,10 @@ type StakingKeeper interface { GetUnbondingDelegation(ctx sdk.Context, delAddr sdk.AccAddress, valAddr sdk.ValAddress) (ubd stakingtypes.UnbondingDelegation, found bool) UnbondingTime(ctx sdk.Context) time.Duration GetParams(ctx sdk.Context) stakingtypes.Params + + IterateBondedValidatorsByPower(ctx sdk.Context, fn func(int64, stakingtypes.ValidatorI) bool) + TotalBondedTokens(ctx sdk.Context) sdk.Int + IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(int64, stakingtypes.DelegationI) bool) } // DistrKeeper expected distribution keeper. From e5c2ad1ddeee99b7a660704d123aafc9fcabf16a Mon Sep 17 00:00:00 2001 From: mconcat Date: Tue, 12 Apr 2022 15:41:20 +0900 Subject: [PATCH 02/14] in progress --- x/superfluid/keeper/stake_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index 30e40a836d2..5673ae545ea 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -766,7 +766,7 @@ func (suite *KeeperTestSuite) TestRefreshIntermediaryDelegationAmounts() { // setup validators valAddrs := suite.SetupValidators(tc.validatorStats) - denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(20), sdk.NewDec(20)}) + denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(10), sdk.NewDec(10)}) // setup superfluid delegations intermediaryAccs, locks := suite.SetupSuperfluidDelegations(delAddrs, valAddrs, tc.superDelegations, denoms) @@ -945,14 +945,14 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { suite.Run(tc.name, func() { suite.SetupTest() + denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(20), sdk.NewDec(20)}) + // Generate delegator addresses delAddrs := CreateRandomAccounts(len(tc.superDelegations)) // setup validators valAddrs := suite.SetupValidators(tc.validatorStats) - denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(20), sdk.NewDec(20)}) - // setup superfluid delegations for _, sfdel := range tc.superDelegations { fmt.Println(sfdel) @@ -1001,7 +1001,7 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { // check if the expected equals to actual for validx := range tc.validatorStats { - suite.Equal(delegatedAmount(delidx, validx).Int64(), sharePerValidatorMap[valAddrs[validx].String()].RoundInt().Int64()) + suite.Equal(delegatedAmount(delidx, validx).Int64()*20, sharePerValidatorMap[valAddrs[validx].String()].RoundInt().Int64()) } } }) From 57cf66243c2ddf7e51197120c0f535053c5f4df0 Mon Sep 17 00:00:00 2001 From: mconcat Date: Wed, 13 Apr 2022 02:06:25 +0900 Subject: [PATCH 03/14] fix constant --- x/superfluid/keeper/stake.go | 13 +++++++------ x/superfluid/keeper/stake_test.go | 18 ++++-------------- 2 files changed, 11 insertions(+), 20 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 82696234a8f..ed73cfd2452 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -367,28 +367,29 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn for i, lock := range synthlocks { interm, ok := k.GetIntermediaryAccountFromLockId(ctx, lock.UnderlyingLockId) if !ok { - panic(1234) // XXX + panic(fmt.Sprintf("Intermediary account retrieval failed with underlying lock\nLock: %+v\n", lock)) } lock, err := k.lk.GetLockByID(ctx, lock.UnderlyingLockId) if err != nil { - panic(err) + panic(fmt.Sprintf("Lockup retrieval failed with underlying lock\nLock: %+v\nError: %s\n", lock, err)) } coin, err := lock.SingleCoin() if err != nil { - panic(err) + panic(fmt.Sprintf("No single coin in the lock\nLock: %+v\nError: %s\n", lock, err)) } amount := k.GetSuperfluidOSMOTokens(ctx, interm.Denom, coin.Amount) valAddr, err := sdk.ValAddressFromBech32(interm.ValAddr) if err != nil { - panic(err) + panic(fmt.Sprintf("Validator address decoding failed\nLock: %+v\nAddress: %s\nError: %s\n", lock, interm.ValAddr, err)) } validator, found := k.sk.GetValidator(ctx, valAddr) if !found { - panic(1234) // XXX: could this happen? + panic(fmt.Sprintf("Validator not exists\nLock: %+v\nAddress: %s\n", lock, valAddr)) } shares, err := validator.SharesFromTokens(amount) if err != nil { - panic(err) // XXX: should we just continue? when err happens? figure out + // tokens are not valid. continue. + continue } delegation := stakingtypes.Delegation{ DelegatorAddress: delegator.String(), diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index 5673ae545ea..2a045a60a6b 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -970,18 +970,8 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { } return res } - /* - // total delegated amount to this validator. used to calculate share. - totalDelegation := func(validx int) sdk.Int { - res := sdk.ZeroInt() - for i := range tc.superDelegations { - res = res.Add(delegatedAmount(i, validx)) - } - return res - } - */ for delidx := range tc.superDelegations { - // map to store all actual delegations to a validator + // store all actual delegations to a validator sharePerValidatorMap := make(map[string]sdk.Dec) for validx := range tc.validatorStats { sharePerValidatorMap[valAddrs[validx].String()] = sdk.ZeroDec() @@ -993,15 +983,15 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { sharePerValidatorMap[val.String()] = share } - // iterate delegations + // iterate delegations and add eligible shares to the sharePerValidatorMap suite.App.SuperfluidKeeper.IterateDelegations(suite.Ctx, delAddrs[delidx], func(_ int64, del stakingtypes.DelegationI) bool { addToSharePerValidatorMap(del.GetValidatorAddr(), del.GetShares()) return false }) - // check if the expected equals to actual + // check if the expected delegated amount equals to actual for validx := range tc.validatorStats { - suite.Equal(delegatedAmount(delidx, validx).Int64()*20, sharePerValidatorMap[valAddrs[validx].String()].RoundInt().Int64()) + suite.Equal(delegatedAmount(delidx, validx).Int64()*10, sharePerValidatorMap[valAddrs[validx].String()].RoundInt().Int64()) } } }) From 2bbe4ce3ecbddaa28c60c281b65816e3f7b34e9d Mon Sep 17 00:00:00 2001 From: mconcat Date: Fri, 15 Apr 2022 00:08:01 +0900 Subject: [PATCH 04/14] apply comments --- x/superfluid/keeper/stake.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index ed73cfd2452..3ca5ef82131 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -362,43 +362,50 @@ func (k Keeper) TotalBondedTokens(ctx sdk.Context) sdk.Int { // IterateDelegations implements govtypes.StakingKeeper func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(int64, stakingtypes.DelegationI) bool) { - // we tract the global index synthlocks := k.lk.GetAllSyntheticLockupsByAddr(ctx, delegator) for i, lock := range synthlocks { - interm, ok := k.GetIntermediaryAccountFromLockId(ctx, lock.UnderlyingLockId) + // get locked coin from the lock ID + interim, ok := k.GetIntermediaryAccountFromLockId(ctx, lock.UnderlyingLockId) if !ok { - panic(fmt.Sprintf("Intermediary account retrieval failed with underlying lock\nLock: %+v\n", lock)) + panic(fmt.Sprintf("intermediary account retrieval failed with underlying lock(Lock: %+v)", lock)) } lock, err := k.lk.GetLockByID(ctx, lock.UnderlyingLockId) if err != nil { - panic(fmt.Sprintf("Lockup retrieval failed with underlying lock\nLock: %+v\nError: %s\n", lock, err)) + panic(fmt.Sprintf("lockup retrieval failed with underlying lock(Lock: %+v; Error: %s)", lock, err)) } coin, err := lock.SingleCoin() if err != nil { - panic(fmt.Sprintf("No single coin in the lock\nLock: %+v\nError: %s\n", lock, err)) + panic(fmt.Sprintf("no single coin in the lock(Lock: %+v; Error: %s)", lock, err)) } - amount := k.GetSuperfluidOSMOTokens(ctx, interm.Denom, coin.Amount) - valAddr, err := sdk.ValAddressFromBech32(interm.ValAddr) + + // get osmo-equivalent token amount + amount := k.GetSuperfluidOSMOTokens(ctx, interim.Denom, coin.Amount) + + // get validator shares equivalent to the token amount + valAddr, err := sdk.ValAddressFromBech32(interim.ValAddr) if err != nil { - panic(fmt.Sprintf("Validator address decoding failed\nLock: %+v\nAddress: %s\nError: %s\n", lock, interm.ValAddr, err)) + panic(fmt.Sprintf("validator address decoding failed(Lock: %+v; Address: %s; Error: %s)", lock, interim.ValAddr, err)) } validator, found := k.sk.GetValidator(ctx, valAddr) if !found { - panic(fmt.Sprintf("Validator not exists\nLock: %+v\nAddress: %s\n", lock, valAddr)) + panic(fmt.Sprintf("validator not exists(Lock: %+v; Address: %s)", lock, valAddr)) } shares, err := validator.SharesFromTokens(amount) if err != nil { // tokens are not valid. continue. continue } + + // construct delegation and call callback delegation := stakingtypes.Delegation{ DelegatorAddress: delegator.String(), - ValidatorAddress: interm.ValAddr, + ValidatorAddress: interim.ValAddr, Shares: shares, } fn(int64(i), delegation) } + // call the callback with the non-superfluid delegations k.sk.IterateDelegations(ctx, delegator, func(i int64, delegation stakingtypes.DelegationI) (stop bool) { return fn(int64(len(synthlocks))+i, delegation) // add len(synthlocks) to index to avoid overlapping }) From 5d3fbb151a17dc19c53f4bea04ea163e1a43887e Mon Sep 17 00:00:00 2001 From: mconcat Date: Mon, 18 Apr 2022 23:22:54 +0900 Subject: [PATCH 05/14] fix test --- x/superfluid/keeper/stake_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index 24f9fe34290..e2c222f0578 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -4,11 +4,12 @@ import ( "fmt" "time" + abci "github.com/tendermint/tendermint/abci/types" + lockuptypes "github.com/osmosis-labs/osmosis/v7/x/lockup/types" minttypes "github.com/osmosis-labs/osmosis/v7/x/mint/types" "github.com/osmosis-labs/osmosis/v7/x/superfluid/keeper" "github.com/osmosis-labs/osmosis/v7/x/superfluid/types" - abci "github.com/tendermint/tendermint/abci/types" sdk "github.com/cosmos/cosmos-sdk/types" stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" @@ -767,7 +768,7 @@ func (suite *KeeperTestSuite) TestRefreshIntermediaryDelegationAmounts() { // setup validators valAddrs := suite.SetupValidators(tc.validatorStats) - denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(10), sdk.NewDec(10)}) + denoms, _ := suite.SetupGammPoolsAndSuperfluidAssets([]sdk.Dec{sdk.NewDec(20), sdk.NewDec(20)}) // setup superfluid delegations intermediaryAccs, locks := suite.SetupSuperfluidDelegations(delAddrs, valAddrs, tc.superDelegations, denoms) From 7e6c6981e0f78b35560ef6c809414e6947c540a3 Mon Sep 17 00:00:00 2001 From: mconcat Date: Mon, 25 Apr 2022 20:55:28 +0900 Subject: [PATCH 06/14] Apply suggestions from code review Co-authored-by: Aleksandr Bezobchuk --- x/superfluid/keeper/stake.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 037bb60e65c..69aeb8ecbf3 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -370,10 +370,12 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn if !ok { panic(fmt.Sprintf("intermediary account retrieval failed with underlying lock(Lock: %+v)", lock)) } + lock, err := k.lk.GetLockByID(ctx, lock.UnderlyingLockId) if err != nil { panic(fmt.Sprintf("lockup retrieval failed with underlying lock(Lock: %+v; Error: %s)", lock, err)) } + coin, err := lock.SingleCoin() if err != nil { panic(fmt.Sprintf("no single coin in the lock(Lock: %+v; Error: %s)", lock, err)) @@ -385,12 +387,14 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn // get validator shares equivalent to the token amount valAddr, err := sdk.ValAddressFromBech32(interim.ValAddr) if err != nil { - panic(fmt.Sprintf("validator address decoding failed(Lock: %+v; Address: %s; Error: %s)", lock, interim.ValAddr, err)) + panic(fmt.Sprintf("failed to decode validator address %s for lock %d: %s)", interim.ValAddr, lock.UnderlyingLockId, err)) } + validator, found := k.sk.GetValidator(ctx, valAddr) if !found { - panic(fmt.Sprintf("validator not exists(Lock: %+v; Address: %s)", lock, valAddr)) + panic(fmt.Sprintf("validator %s does not exist for lock %d", valAddr, lock.UnderlyingLockId)) } + shares, err := validator.SharesFromTokens(amount) if err != nil { // tokens are not valid. continue. From d166efebafa8cd9b86302993ccdc0ce6e1da2bdc Mon Sep 17 00:00:00 2001 From: mconcat Date: Tue, 26 Apr 2022 00:09:47 +0900 Subject: [PATCH 07/14] continue on nonexisting interim account --- x/superfluid/keeper/stake.go | 16 ++++++++++------ x/superfluid/keeper/stake_test.go | 2 -- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 037bb60e65c..9d9cdee8cab 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -363,12 +363,20 @@ func (k Keeper) TotalBondedTokens(ctx sdk.Context) sdk.Int { // IterateDelegations implements govtypes.StakingKeeper func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(int64, stakingtypes.DelegationI) bool) { + // call the callback with the non-superfluid delegations + var index int64 + k.sk.IterateDelegations(ctx, delegator, func(i int64, delegation stakingtypes.DelegationI) (stop bool) { + index = i + return fn(i, delegation) + }) + synthlocks := k.lk.GetAllSyntheticLockupsByAddr(ctx, delegator) for i, lock := range synthlocks { // get locked coin from the lock ID interim, ok := k.GetIntermediaryAccountFromLockId(ctx, lock.UnderlyingLockId) if !ok { - panic(fmt.Sprintf("intermediary account retrieval failed with underlying lock(Lock: %+v)", lock)) + ctx.Logger().Error("intermediary account retrieval failed with underlying lock", "Lock", lock) + continue } lock, err := k.lk.GetLockByID(ctx, lock.UnderlyingLockId) if err != nil { @@ -403,11 +411,7 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn ValidatorAddress: interim.ValAddr, Shares: shares, } - fn(int64(i), delegation) + fn(index+int64(i), delegation) } - // call the callback with the non-superfluid delegations - k.sk.IterateDelegations(ctx, delegator, func(i int64, delegation stakingtypes.DelegationI) (stop bool) { - return fn(int64(len(synthlocks))+i, delegation) // add len(synthlocks) to index to avoid overlapping - }) } diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index e2c222f0578..cd82ffbffd1 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "fmt" "time" abci "github.com/tendermint/tendermint/abci/types" @@ -957,7 +956,6 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { // setup superfluid delegations for _, sfdel := range tc.superDelegations { - fmt.Println(sfdel) intermediaryAccs, _ := suite.SetupSuperfluidDelegations(delAddrs, valAddrs, sfdel, denoms) suite.checkIntermediaryAccountDelegations(intermediaryAccs) } From 46ee0bd481b001af1c9e40f1df6d65d55105b3c1 Mon Sep 17 00:00:00 2001 From: mconcat Date: Thu, 28 Apr 2022 03:13:11 +0900 Subject: [PATCH 08/14] fix typo --- x/superfluid/keeper/stake.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 9115392f2a8..478f1cd27d5 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -395,12 +395,12 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn // get validator shares equivalent to the token amount valAddr, err := sdk.ValAddressFromBech32(interim.ValAddr) if err != nil { - panic(fmt.Sprintf("failed to decode validator address %s for lock %d: %s)", interim.ValAddr, lock.UnderlyingLockId, err)) + panic(fmt.Sprintf("failed to decode validator address %s for lock %d: %s)", interim.ValAddr, lock.ID, err)) } validator, found := k.sk.GetValidator(ctx, valAddr) if !found { - panic(fmt.Sprintf("validator %s does not exist for lock %d", valAddr, lock.UnderlyingLockId)) + panic(fmt.Sprintf("validator %s does not exist for lock %d", valAddr, lock.ID)) } shares, err := validator.SharesFromTokens(amount) From 3e3433ac81f517252303e0ce99a1e40d24a85116 Mon Sep 17 00:00:00 2001 From: mconcat Date: Thu, 28 Apr 2022 04:02:40 +0900 Subject: [PATCH 09/14] panic -> ctx.Logger.Erroer --- x/superfluid/keeper/stake.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 478f1cd27d5..8cae634ea34 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -381,12 +381,14 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn lock, err := k.lk.GetLockByID(ctx, lock.UnderlyingLockId) if err != nil { - panic(fmt.Sprintf("lockup retrieval failed with underlying lock(Lock: %+v; Error: %s)", lock, err)) + ctx.Logger().Error("lockup retrieval failed with underlying lock", "Lock", lock, "Error", err)) + continue } coin, err := lock.SingleCoin() if err != nil { - panic(fmt.Sprintf("no single coin in the lock(Lock: %+v; Error: %s)", lock, err)) + ctx.Logger().Error("no single coin in the lock", "Lock", lock, "Error", err) + continue } // get osmo-equivalent token amount @@ -395,12 +397,14 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn // get validator shares equivalent to the token amount valAddr, err := sdk.ValAddressFromBech32(interim.ValAddr) if err != nil { - panic(fmt.Sprintf("failed to decode validator address %s for lock %d: %s)", interim.ValAddr, lock.ID, err)) + ctx.Logger().Error("failed to decode validator address", "Intermediary", interim.ValAddr, "LockID", lock.ID, "Error", err)) + continue } validator, found := k.sk.GetValidator(ctx, valAddr) if !found { - panic(fmt.Sprintf("validator %s does not exist for lock %d", valAddr, lock.ID)) + ctx.Logger().Error("validator does not exist for lock", "Validator", valAddr, "LockID", lock.ID) + continue } shares, err := validator.SharesFromTokens(amount) @@ -417,5 +421,4 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn } fn(index+int64(i), delegation) } - } From 1b515acdeba7afa3c3f1632682d5a16058843e05 Mon Sep 17 00:00:00 2001 From: mconcat Date: Thu, 28 Apr 2022 06:11:51 +0900 Subject: [PATCH 10/14] typo --- x/superfluid/keeper/stake.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 8cae634ea34..69a546bee83 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -381,7 +381,7 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn lock, err := k.lk.GetLockByID(ctx, lock.UnderlyingLockId) if err != nil { - ctx.Logger().Error("lockup retrieval failed with underlying lock", "Lock", lock, "Error", err)) + ctx.Logger().Error("lockup retrieval failed with underlying lock", "Lock", lock, "Error", err) continue } @@ -397,7 +397,7 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn // get validator shares equivalent to the token amount valAddr, err := sdk.ValAddressFromBech32(interim.ValAddr) if err != nil { - ctx.Logger().Error("failed to decode validator address", "Intermediary", interim.ValAddr, "LockID", lock.ID, "Error", err)) + ctx.Logger().Error("failed to decode validator address", "Intermediary", interim.ValAddr, "LockID", lock.ID, "Error", err) continue } From 6de2ba02c3b0ab65ff232b6c92f43dc77381f7b6 Mon Sep 17 00:00:00 2001 From: mconcat Date: Fri, 29 Apr 2022 20:40:03 +0900 Subject: [PATCH 11/14] Apply suggestions from code review Co-authored-by: Dev Ojha --- x/superfluid/keeper/stake.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 69a546bee83..8ce2985f87f 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -362,6 +362,7 @@ func (k Keeper) TotalBondedTokens(ctx sdk.Context) sdk.Int { } // IterateDelegations implements govtypes.StakingKeeper +// Iterates through staking keeper's delegations, and then all of the superfluid delegations. func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(int64, stakingtypes.DelegationI) bool) { // call the callback with the non-superfluid delegations var index int64 @@ -387,7 +388,7 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn coin, err := lock.SingleCoin() if err != nil { - ctx.Logger().Error("no single coin in the lock", "Lock", lock, "Error", err) + ctx.Logger().Error("lock fails to meet expected invariant, it contains multiple coins", "Lock", lock, "Error", err) continue } From 2ca3b8df267a9a6647c57c8af914088aecbb22a9 Mon Sep 17 00:00:00 2001 From: mconcat Date: Tue, 3 May 2022 00:49:26 +0900 Subject: [PATCH 12/14] add native coin test --- x/superfluid/keeper/stake_test.go | 49 +++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 3 deletions(-) diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index cd82ffbffd1..0e78fdc0ad9 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "fmt" "time" abci "github.com/tendermint/tendermint/abci/types" @@ -14,6 +15,12 @@ import ( stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types" ) +type normalDelegation struct { + delIndex int64 + valIndex int64 + coinAmount int64 +} + type superfluidDelegation struct { delIndex int64 valIndex int64 @@ -31,6 +38,15 @@ type osmoEquivalentMultiplier struct { price sdk.Dec } +func (suite *KeeperTestSuite) SetupNormalDelegation(delAddrs []sdk.AccAddress, valAddrs []sdk.ValAddress, del normalDelegation) error { + val, found := suite.App.StakingKeeper.GetValidator(suite.Ctx, valAddrs[del.valIndex]) + if !found { + return fmt.Errorf("validator not found") + } + _, err := suite.App.StakingKeeper.Delegate(suite.Ctx, delAddrs[del.delIndex], sdk.NewIntFromUint64(uint64(del.coinAmount)), stakingtypes.Bonded, val, false) + return err +} + func (suite *KeeperTestSuite) SetupSuperfluidDelegations(delAddrs []sdk.AccAddress, valAddrs []sdk.ValAddress, superDelegations []superfluidDelegation, denoms []string) ([]types.SuperfluidIntermediaryAccount, []lockuptypes.PeriodLock) { flagIntermediaryAcc := make(map[string]bool) intermediaryAccs := []types.SuperfluidIntermediaryAccount{} @@ -899,34 +915,40 @@ func (suite *KeeperTestSuite) TestRefreshIntermediaryDelegationAmounts() { func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { testCases := []struct { - name string - validatorStats []stakingtypes.BondStatus - superDelegations [][]superfluidDelegation + name string + validatorStats []stakingtypes.BondStatus + superDelegations [][]superfluidDelegation + normalDelegations []normalDelegation }{ { "with single validator and single delegation", []stakingtypes.BondStatus{stakingtypes.Bonded}, [][]superfluidDelegation{{{0, 0, 0, 1000000}}}, + nil, }, { "with single validator and additional delegations", []stakingtypes.BondStatus{stakingtypes.Bonded}, [][]superfluidDelegation{{{0, 0, 0, 1000000}, {0, 0, 0, 1000000}}}, + nil, }, { "with multiple validator and multiple superfluid delegations", []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, [][]superfluidDelegation{{{0, 0, 0, 1000000}}, {{1, 1, 0, 1000000}}}, + nil, }, { "with single validator and multiple denom superfluid delegations", []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, [][]superfluidDelegation{{{0, 0, 0, 1000000}, {0, 0, 1, 1000000}}}, + nil, }, { "with multiple validators and multiple denom superfluid delegations", []stakingtypes.BondStatus{stakingtypes.Bonded, stakingtypes.Bonded}, [][]superfluidDelegation{{{0, 0, 0, 1000000}, {0, 1, 1, 1000000}}}, + nil, }, { "many delegations", @@ -937,6 +959,17 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { {{2, 1, 1, 1000000}, {2, 1, 0, 1000000}}, {{3, 0, 0, 1000000}, {3, 1, 1, 1000000}}, }, + nil, + }, + { + "with normal delegations", + []stakingtypes.BondStatus{stakingtypes.Bonded}, + [][]superfluidDelegation{ + {{0, 0, 0, 1000000}, {0, 0, 1, 1000000}}, + }, + []normalDelegation{ + {0, 0, 1000000}, + }, }, } @@ -960,6 +993,12 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { suite.checkIntermediaryAccountDelegations(intermediaryAccs) } + // setup normal delegations + for _, del := range tc.normalDelegations { + err := suite.SetupNormalDelegation(delAddrs, valAddrs, del) + suite.NoError(err) + } + // all expected delegated amounts to a validator from a delegator delegatedAmount := func(delidx, validx int) sdk.Int { res := sdk.ZeroInt() @@ -968,6 +1007,10 @@ func (suite *KeeperTestSuite) TestSuperfluidDelegationGovernanceVoting() { res = res.AddRaw(del.lpAmount) } } + if len(tc.normalDelegations) != 0 { + del := tc.normalDelegations[delidx] + res = res.AddRaw(del.coinAmount / 10) // LP price is 10 osmo in this test + } return res } for delidx := range tc.superDelegations { From c9367704757047ff5be61df03ab0d8eda2774f23 Mon Sep 17 00:00:00 2001 From: mconcat Date: Tue, 3 May 2022 00:50:18 +0900 Subject: [PATCH 13/14] Apply suggestions from code review Co-authored-by: Dev Ojha --- x/superfluid/keeper/stake.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 8ce2985f87f..8a134ee7266 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -363,6 +363,7 @@ func (k Keeper) TotalBondedTokens(ctx sdk.Context) sdk.Int { // IterateDelegations implements govtypes.StakingKeeper // Iterates through staking keeper's delegations, and then all of the superfluid delegations. +// Iterates through staking keeper's delegations, and then all of the superfluid delegations. func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(int64, stakingtypes.DelegationI) bool) { // call the callback with the non-superfluid delegations var index int64 From ce499d2688b424b8c916a7f3811319de0ca5f1e9 Mon Sep 17 00:00:00 2001 From: mattverse Date: Tue, 17 May 2022 21:28:10 +0900 Subject: [PATCH 14/14] Address comments from code review --- x/superfluid/keeper/stake.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index 8a134ee7266..a4ec1378f07 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -363,7 +363,6 @@ func (k Keeper) TotalBondedTokens(ctx sdk.Context) sdk.Int { // IterateDelegations implements govtypes.StakingKeeper // Iterates through staking keeper's delegations, and then all of the superfluid delegations. -// Iterates through staking keeper's delegations, and then all of the superfluid delegations. func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn func(int64, stakingtypes.DelegationI) bool) { // call the callback with the non-superfluid delegations var index int64 @@ -377,7 +376,6 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn // get locked coin from the lock ID interim, ok := k.GetIntermediaryAccountFromLockId(ctx, lock.UnderlyingLockId) if !ok { - ctx.Logger().Error("intermediary account retrieval failed with underlying lock", "Lock", lock) continue } @@ -421,6 +419,8 @@ func (k Keeper) IterateDelegations(ctx sdk.Context, delegator sdk.AccAddress, fn ValidatorAddress: interim.ValAddr, Shares: shares, } + + // if valid delegation has been found, increment delegation index fn(index+int64(i), delegation) } }