From c780c738545051db2fa22c25a410f416ed143068 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Tue, 21 May 2024 14:54:09 -0600 Subject: [PATCH] fix errors for GetDelegation --- x/superfluid/keeper/grpc_query.go | 2 +- x/superfluid/keeper/migrate_test.go | 14 +++++++------- x/superfluid/keeper/stake.go | 5 ++--- x/superfluid/keeper/stake_test.go | 8 ++++---- x/superfluid/keeper/unpool_test.go | 2 +- x/valset-pref/simulation/sim_msgs.go | 4 ++-- x/valset-pref/validator_set.go | 6 +++--- 7 files changed, 20 insertions(+), 21 deletions(-) diff --git a/x/superfluid/keeper/grpc_query.go b/x/superfluid/keeper/grpc_query.go index 701986a46f2..aa8cefd4143 100644 --- a/x/superfluid/keeper/grpc_query.go +++ b/x/superfluid/keeper/grpc_query.go @@ -490,7 +490,7 @@ func (q Querier) EstimateSuperfluidDelegatedAmountByValidatorDenom(goCtx context delegation, err := q.Keeper.sk.GetDelegation(ctx, intermediaryAcc.GetAccAddress(), valAddr) if err != nil { - return nil, stakingtypes.ErrNoDelegation + return nil, err } syntheticOsmoAmt := delegation.Shares.Quo(val.DelegatorShares).MulInt(val.Tokens) diff --git a/x/superfluid/keeper/migrate_test.go b/x/superfluid/keeper/migrate_test.go index faadb33266d..c0ae7fcca02 100644 --- a/x/superfluid/keeper/migrate_test.go +++ b/x/superfluid/keeper/migrate_test.go @@ -195,7 +195,7 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() { // The delegation from the balancer intermediary account holder should not exist. delegation, error := stakingKeeper.GetDelegation(s.Ctx, balancerIntermediaryAcc.GetAccAddress(), valAddr) - s.Require().Error(error, "expected no delegation, found delegation w/ %d shares", delegation.Shares) + s.Require().Error(error, "expected error, found delegation w/ %d shares", delegation.Shares) // Check that the original gamm lockup is deleted. _, err := s.App.LockupKeeper.GetLockByID(s.Ctx, originalGammLockId) @@ -212,7 +212,7 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() { // The delegation from the balancer intermediary account holder should still exist. delegation, err := stakingKeeper.GetDelegation(s.Ctx, balancerIntermediaryAcc.GetAccAddress(), valAddr) - s.Require().NoError(err, "expected delegation, found delegation no delegation") + s.Require().NoError(err, "expected delegation, got error instead") s.Require().Equal(balancerDelegationPre.Shares.Sub(balancerDelegationPre.Shares.Mul(tc.percentOfSharesToMigrate)).RoundInt().String(), delegation.Shares.RoundInt().String(), "expected %d shares, found %d shares", balancerDelegationPre.Shares.Mul(tc.percentOfSharesToMigrate).RoundInt().String(), delegation.Shares.String()) // Check what is remaining in the original gamm lock. @@ -223,7 +223,7 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() { // Check the new superfluid staked amount. clIntermediaryAcc := superfluidKeeper.GetLockIdIntermediaryAccountConnection(s.Ctx, concentratedLockId) delegation, err := stakingKeeper.GetDelegation(s.Ctx, clIntermediaryAcc, valAddr) - s.Require().NoError(err, "expected delegation, found delegation no delegation") + s.Require().NoError(err, "expected delegation, got error instead") s.Require().Equal(balancerDelegationPre.Shares.Mul(tc.percentOfSharesToMigrate).RoundInt().Sub(osmomath.OneInt()).String(), delegation.Shares.RoundInt().String(), "expected %d shares, found %d shares", balancerDelegationPre.Shares.Mul(tc.percentOfSharesToMigrate).RoundInt().String(), delegation.Shares.String()) } @@ -241,7 +241,7 @@ func (s *KeeperTestSuite) TestRouteLockedBalancerToConcentratedMigration() { // The delegation from the intermediary account holder does not exist. delegation, err := stakingKeeper.GetDelegation(s.Ctx, balancerIntermediaryAcc.GetAccAddress(), valAddr) - s.Require().Error(err, "expected no delegation, found delegation w/ %d shares", delegation.Shares) + s.Require().Error(err, "expected error, found delegation w/ %d shares", delegation.Shares) } // Run slashing logic if the test case involves locks and check if the new and old locks are slashed. @@ -356,7 +356,7 @@ func (s *KeeperTestSuite) TestMigrateSuperfluidBondedBalancerToConcentrated() { // The delegation from the intermediary account holder should not exist. delegation, err := stakingKeeper.GetDelegation(s.Ctx, balancerIntermediaryAcc.GetAccAddress(), valAddr) - s.Require().Error(err, "expected no delegation, found delegation w/ %d shares", delegation.Shares) + s.Require().Error(err, "expected error, found delegation w/ %d shares", delegation.Shares) // Check that the original gamm lockup is deleted. _, err = s.App.LockupKeeper.GetLockByID(s.Ctx, originalGammLockId) @@ -375,7 +375,7 @@ func (s *KeeperTestSuite) TestMigrateSuperfluidBondedBalancerToConcentrated() { // The delegation from the intermediary account holder should still exist. _, err = stakingKeeper.GetDelegation(s.Ctx, balancerIntermediaryAcc.GetAccAddress(), valAddr) - s.Require().NoError(err, "expected delegation, found delegation no delegation") + s.Require().NoError(err, "expected delegation, got error instead") // Check what is remaining in the original gamm lock. lock, err := s.App.LockupKeeper.GetLockByID(s.Ctx, originalGammLockId) @@ -385,7 +385,7 @@ func (s *KeeperTestSuite) TestMigrateSuperfluidBondedBalancerToConcentrated() { // Check the new superfluid staked amount. clIntermediaryAcc := superfluidKeeper.GetLockIdIntermediaryAccountConnection(s.Ctx, concentratedLockId) delegation, err := stakingKeeper.GetDelegation(s.Ctx, clIntermediaryAcc, valAddr) - s.Require().NoError(err, "expected delegation, found delegation no delegation") + s.Require().NoError(err, "expected delegation, got error instead") s.Require().Equal(balancerDelegationPre.Shares.Mul(tc.percentOfSharesToMigrate).RoundInt().Sub(osmomath.OneInt()).String(), delegation.Shares.RoundInt().String(), "expected %d shares, found %d shares", balancerDelegationPre.Shares.Mul(tc.percentOfSharesToMigrate).RoundInt().String(), delegation.Shares.String()) // Check if the new intermediary account connection was created. diff --git a/x/superfluid/keeper/stake.go b/x/superfluid/keeper/stake.go index ee683d865f4..db604696c19 100644 --- a/x/superfluid/keeper/stake.go +++ b/x/superfluid/keeper/stake.go @@ -71,11 +71,10 @@ func (k Keeper) RefreshIntermediaryDelegationAmounts(context context.Context, ac currentAmount := osmomath.NewInt(0) delegation, err := k.sk.GetDelegation(ctx, mAddr, valAddress) if err != nil { - // continue if current delegation is 0, in case its really a dust delegation + // continue if current delegation return an error, in case its really a dust delegation // that becomes worth something after refresh. // TODO: We have a correct explanation for this in some github issue, lets amend this correctly. - k.Logger(ctx).Debug(fmt.Sprintf("Existing delegation not found for %s with %s during superfluid refresh."+ - " It may have been previously bonded, but now unbonded.", mAddr.String(), acc.ValAddr)) + k.Logger(ctx).Debug(err.Error()) } else { currentAmount = validator.TokensFromShares(delegation.Shares).RoundInt() } diff --git a/x/superfluid/keeper/stake_test.go b/x/superfluid/keeper/stake_test.go index e135b6c1fa8..cd3fe9fe8d7 100644 --- a/x/superfluid/keeper/stake_test.go +++ b/x/superfluid/keeper/stake_test.go @@ -447,7 +447,7 @@ func (s *KeeperTestSuite) TestSuperfluidUndelegate() { s.Require().NoError(err) delegation, err := s.App.StakingKeeper.GetDelegation(s.Ctx, acc.GetAccAddress(), valAddr) if expDelegation.IsZero() { - s.Require().Error(err, "expected no delegation, found delegation w/ %d shares", delegation.Shares) + s.Require().Error(err, "expected error, found delegation w/ %d shares", delegation.Shares) } else { s.Require().NoError(err) s.Require().Equal(expDelegation, delegation.Shares) @@ -611,7 +611,7 @@ func (s *KeeperTestSuite) TestSuperfluidUndelegateToConcentratedPosition() { s.Require().NoError(err) delegation, err := s.App.StakingKeeper.GetDelegation(s.Ctx, acc.GetAccAddress(), valAddr) if expDelegation.IsZero() { - s.Require().Error(err, "expected no delegation, found delegation w/ %d shares", delegation.Shares) + s.Require().Error(err, "expected error, found delegation w/ %d shares", delegation.Shares) } else { s.Require().NoError(err) s.Require().Equal(expDelegation, delegation.Shares) @@ -1931,8 +1931,8 @@ func (s *KeeperTestSuite) getExpectedBondDenomPoolAmtAfterConvert(sender sdk.Acc // s.Require().Equal(intAcc.String(), expAcc.GetAccAddress().String()) // // check delegation from intermediary account to validator -// _, found := s.App.StakingKeeper.GetDelegation(s.Ctx, expAcc.GetAccAddress(), valAddrs[srd.newValIndex]) -// s.Require().True(found) +// _, err := s.App.StakingKeeper.GetDelegation(s.Ctx, expAcc.GetAccAddress(), valAddrs[srd.newValIndex]) +// s.Require().NoError(err) // } // // try redelegating twice diff --git a/x/superfluid/keeper/unpool_test.go b/x/superfluid/keeper/unpool_test.go index 47c698bfc73..b3b5966867a 100644 --- a/x/superfluid/keeper/unpool_test.go +++ b/x/superfluid/keeper/unpool_test.go @@ -247,7 +247,7 @@ func (s *KeeperTestSuite) TestUnpool() { // check if delegation has reduced from intermediary account delegation, err := stakingKeeper.GetDelegation(ctx, intermediaryAcc.GetAccAddress(), valAddr) - s.Require().Error(err, "expected no delegation, found delegation w/ %d shares", delegation.Shares) + s.Require().Error(err, "expected err, instead found delegation w/ %d shares", delegation.Shares) } }) } diff --git a/x/valset-pref/simulation/sim_msgs.go b/x/valset-pref/simulation/sim_msgs.go index a43e29bccb7..f91b0b771c9 100644 --- a/x/valset-pref/simulation/sim_msgs.go +++ b/x/valset-pref/simulation/sim_msgs.go @@ -77,7 +77,7 @@ func RandomMsgUnDelegateFromValSet(k valsetkeeper.Keeper, sim *osmosimtypes.SimC // check if the user has delegated tokens to the valset del, err := sim.SDKStakingKeeper().GetDelegation(ctx, delAddr, val) if err != nil { - return nil, fmt.Errorf("user hasn't delegated tokens to the validator, %s", val.String()) + return nil, err } totalBond := validator.TokensFromShares(del.GetShares()).TruncateInt() @@ -129,7 +129,7 @@ func RandomMsgReDelegateToValSet(k valsetkeeper.Keeper, sim *osmosimtypes.SimCtx // check if the user has delegated tokens to the valset _, err = sim.SDKStakingKeeper().GetDelegation(ctx, delAddr, val) if err != nil { - return nil, fmt.Errorf("user hasn't delegated tokens to the validator, %s", val.String()) + return nil, err } } diff --git a/x/valset-pref/validator_set.go b/x/valset-pref/validator_set.go index 818063f6865..7e4d814da1e 100644 --- a/x/valset-pref/validator_set.go +++ b/x/valset-pref/validator_set.go @@ -293,7 +293,7 @@ func (k Keeper) UndelegateFromRebalancedValidatorSet(ctx sdk.Context, delegatorA // in the event some rounding issue increases our calculated undelegation amount. delegation, err := k.stakingKeeper.GetDelegation(ctx, delegator, val.ValAddr) if err != nil { - return fmt.Errorf("No delegation found for delegator %s to validator %s\n", delegator, val.ValAddr) + return err } delegationToVal := delegation.Shares.TruncateInt() calculatedUndelegationAmt := undelegation.Amount.Sub(totalUnDelAmt).ToLegacyDec().TruncateInt() @@ -335,7 +335,7 @@ func (k Keeper) getValsetRatios(ctx sdk.Context, delegator sdk.AccAddress, delegation, err := k.stakingKeeper.GetDelegation(ctx, delegator, valAddr) if err != nil { - return nil, map[string]stakingtypes.Validator{}, osmomath.ZeroDec(), fmt.Errorf("No delegation found for delegator %s to validator %s\n", delegator, valAddr) + return nil, map[string]stakingtypes.Validator{}, osmomath.ZeroDec(), err } undelegateSharesAmt, err := validator.SharesFromTokens(amountToUnDelegate) @@ -388,7 +388,7 @@ func (k Keeper) PreformRedelegation(ctx sdk.Context, delegator sdk.AccAddress, e // check if the user has delegated tokens to the valset delegation, err := k.stakingKeeper.GetDelegation(ctx, delegator, valAddr) if err != nil { - return fmt.Errorf("No delegation found") + return err } tokenFromShares := validator.TokensFromShares(delegation.Shares)