Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: migrate away from using valBech32 globals (2/2) #17157

Merged
merged 22 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions simapp/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []

// withdraw all validator commission
err := app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator())
valBz, err := app.StakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
if err != nil {
panic(err)
}
_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, valBz)
return false
})
if err != nil {
Expand Down Expand Up @@ -122,8 +126,12 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []

// reinitialize all validators
err = app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
valBz, err := app.StakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
if err != nil {
panic(err)
}
// donate any unwithdrawn outstanding reward fraction tokens to the community pool
scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, val.GetOperator())
scraps, err := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, valBz)
if err != nil {
panic(err)
}
Expand All @@ -136,7 +144,7 @@ func (app *SimApp) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []
panic(err)
}

if err := app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, val.GetOperator()); err != nil {
if err := app.DistrKeeper.Hooks().AfterValidatorCreated(ctx, valBz); err != nil {
panic(err)
}
return false
Expand Down
1 change: 1 addition & 0 deletions types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,7 @@ func (va ValAddress) Format(s fmt.State, verb rune) {
type ConsAddress []byte

// ConsAddressFromHex creates a ConsAddress from a hex string.
// Deprecated: use ConsensusAddressCodec from Staking keeper
func ConsAddressFromHex(address string) (addr ConsAddress, err error) {
bz, err := addressBytesFromHexString(address)
return ConsAddress(bz), err
Expand Down
18 changes: 9 additions & 9 deletions x/distribution/keeper/allocation.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (k Keeper) AllocateTokensToValidator(ctx context.Context, val stakingtypes.
commission := tokens.MulDec(val.GetCommission())
shared := tokens.Sub(commission)

valStr, err := k.stakingKeeper.ValidatorAddressCodec().BytesToString(val.GetOperator())
valBz, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
if err != nil {
return err
}
Expand All @@ -100,29 +100,29 @@ func (k Keeper) AllocateTokensToValidator(ctx context.Context, val stakingtypes.
sdk.NewEvent(
types.EventTypeCommission,
sdk.NewAttribute(sdk.AttributeKeyAmount, commission.String()),
sdk.NewAttribute(types.AttributeKeyValidator, valStr),
sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator()),
),
)
currentCommission, err := k.ValidatorsAccumulatedCommission.Get(ctx, val.GetOperator())
currentCommission, err := k.ValidatorsAccumulatedCommission.Get(ctx, valBz)
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return err
}

currentCommission.Commission = currentCommission.Commission.Add(commission...)
err = k.ValidatorsAccumulatedCommission.Set(ctx, val.GetOperator(), currentCommission)
err = k.ValidatorsAccumulatedCommission.Set(ctx, valBz, currentCommission)
if err != nil {
return err
}

// update current rewards
currentRewards, err := k.ValidatorCurrentRewards.Get(ctx, val.GetOperator())
currentRewards, err := k.ValidatorCurrentRewards.Get(ctx, valBz)
// if the rewards do not exist it's fine, we will just add to zero.
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return err
}

currentRewards.Rewards = currentRewards.Rewards.Add(shared...)
err = k.ValidatorCurrentRewards.Set(ctx, val.GetOperator(), currentRewards)
err = k.ValidatorCurrentRewards.Set(ctx, valBz, currentRewards)
if err != nil {
return err
}
Expand All @@ -132,15 +132,15 @@ func (k Keeper) AllocateTokensToValidator(ctx context.Context, val stakingtypes.
sdk.NewEvent(
types.EventTypeRewards,
sdk.NewAttribute(sdk.AttributeKeyAmount, tokens.String()),
sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator().String()),
sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator()),
),
)

outstanding, err := k.ValidatorOutstandingRewards.Get(ctx, val.GetOperator())
outstanding, err := k.ValidatorOutstandingRewards.Get(ctx, valBz)
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return err
}

outstanding.Rewards = outstanding.Rewards.Add(tokens...)
return k.ValidatorOutstandingRewards.Set(ctx, val.GetOperator(), outstanding)
return k.ValidatorOutstandingRewards.Set(ctx, valBz, outstanding)
}
11 changes: 8 additions & 3 deletions x/distribution/keeper/allocation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ func TestAllocateTokensToValidatorWithCommission(t *testing.T) {
stakingKeeper := distrtestutil.NewMockStakingKeeper(ctrl)
accountKeeper := distrtestutil.NewMockAccountKeeper(ctrl)

valCodec := address.NewBech32Codec("cosmosvaloper")

accountKeeper.EXPECT().GetModuleAddress("distribution").Return(distrAcc.GetAddress())
stakingKeeper.EXPECT().ValidatorAddressCodec().Return(address.NewBech32Codec("cosmosvaloper")).AnyTimes()
stakingKeeper.EXPECT().ValidatorAddressCodec().Return(valCodec).AnyTimes()

distrKeeper := keeper.NewKeeper(
encCfg.Codec,
Expand Down Expand Up @@ -68,12 +70,15 @@ func TestAllocateTokensToValidatorWithCommission(t *testing.T) {
{Denom: sdk.DefaultBondDenom, Amount: math.LegacyNewDec(5)},
}

valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, val.GetOperator())
valBz, err := valCodec.StringToBytes(val.GetOperator())
require.NoError(t, err)

valCommission, err := distrKeeper.ValidatorsAccumulatedCommission.Get(ctx, valBz)
require.NoError(t, err)
require.Equal(t, expected, valCommission.Commission)

// check current rewards
currentRewards, err := distrKeeper.ValidatorCurrentRewards.Get(ctx, val.GetOperator())
currentRewards, err := distrKeeper.ValidatorCurrentRewards.Get(ctx, valBz)
require.NoError(t, err)
require.Equal(t, expected, currentRewards.Rewards)
}
Expand Down
13 changes: 9 additions & 4 deletions x/distribution/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,18 @@
panic("stake should not be negative")
}

valBz, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
if err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}

// return staking * (ending - starting)
starting, err := k.ValidatorHistoricalRewards.Get(ctx, collections.Join(val.GetOperator(), startingPeriod))
starting, err := k.ValidatorHistoricalRewards.Get(ctx, collections.Join(sdk.ValAddress(valBz), startingPeriod))
if err != nil {
return sdk.DecCoins{}, err
}

ending, err := k.ValidatorHistoricalRewards.Get(ctx, collections.Join(val.GetOperator(), endingPeriod))
ending, err := k.ValidatorHistoricalRewards.Get(ctx, collections.Join(sdk.ValAddress(valBz), endingPeriod))
if err != nil {
return sdk.DecCoins{}, err
}
Expand Down Expand Up @@ -237,7 +242,7 @@
logger.Info(
"rounding error withdrawing rewards from validator",
"delegator", del.GetDelegatorAddr(),
"validator", val.GetOperator().String(),
"validator", val.GetOperator(),
"got", rewards.String(),
"expected", rewardsRaw.String(),
)
Expand Down Expand Up @@ -311,7 +316,7 @@
sdk.NewEvent(
types.EventTypeWithdrawRewards,
sdk.NewAttribute(sdk.AttributeKeyAmount, finalRewards.String()),
sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator().String()),
sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator()),
sdk.NewAttribute(types.AttributeKeyDelegator, del.GetDelegatorAddr()),
),
)
Expand Down
11 changes: 10 additions & 1 deletion x/distribution/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ func TestCalculateRewardsAfterSlash(t *testing.T) {
math.LegacyNewDecWithPrec(5, 1),
&val,
&distrKeeper,
stakingKeeper,
)
require.True(t, slashedTokens.IsPositive(), "expected positive slashed tokens, got: %s", slashedTokens)

Expand Down Expand Up @@ -301,6 +302,7 @@ func TestCalculateRewardsAfterManySlashes(t *testing.T) {
math.LegacyNewDecWithPrec(5, 1),
&val,
&distrKeeper,
stakingKeeper,
)
require.True(t, slashedTokens.IsPositive(), "expected positive slashed tokens, got: %s", slashedTokens)

Expand All @@ -324,6 +326,7 @@ func TestCalculateRewardsAfterManySlashes(t *testing.T) {
math.LegacyNewDecWithPrec(2, 1),
&val,
&distrKeeper,
stakingKeeper,
)
require.True(t, slashedTokens.IsPositive(), "expected positive slashed tokens, got: %s", slashedTokens)

Expand Down Expand Up @@ -408,7 +411,7 @@ func TestCalculateRewardsMultiDelegator(t *testing.T) {

// second delegation
addr1 := sdk.AccAddress(valConsAddr1)
_, del1, err := distrtestutil.Delegate(ctx, distrKeeper, addr1, &val, math.NewInt(100), nil)
_, del1, err := distrtestutil.Delegate(ctx, distrKeeper, addr1, &val, math.NewInt(100), nil, stakingKeeper)
require.NoError(t, err)

stakingKeeper.EXPECT().Delegation(gomock.Any(), addr1, valAddr).Return(del1, nil)
Expand Down Expand Up @@ -600,6 +603,7 @@ func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) {
math.LegacyNewDecWithPrec(5, 1),
&val,
&distrKeeper,
stakingKeeper,
)

// slash the validator by 50% again
Expand All @@ -612,6 +616,7 @@ func TestCalculateRewardsAfterManySlashesInSameBlock(t *testing.T) {
math.LegacyNewDecWithPrec(5, 1),
&val,
&distrKeeper,
stakingKeeper,
)

// increase block height
Expand Down Expand Up @@ -703,6 +708,7 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) {
math.LegacyNewDecWithPrec(5, 1),
&val,
&distrKeeper,
stakingKeeper,
)
ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 3)

Expand All @@ -717,6 +723,7 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) {
&val,
sdk.TokensFromConsensusPower(100, sdk.DefaultPowerReduction),
nil,
stakingKeeper,
)
require.NoError(t, err)

Expand Down Expand Up @@ -744,6 +751,7 @@ func TestCalculateRewardsMultiDelegatorMultiSlash(t *testing.T) {
math.LegacyNewDecWithPrec(5, 1),
&val,
&distrKeeper,
stakingKeeper,
)
ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 3)

Expand Down Expand Up @@ -836,6 +844,7 @@ func TestCalculateRewardsMultiDelegatorMultWithdraw(t *testing.T) {
&val,
math.NewInt(100),
nil,
stakingKeeper,
)
require.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion x/distribution/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data types.GenesisState) {
var previousProposer sdk.ConsAddress
if data.PreviousProposer != "" {
var err error
previousProposer, err = sdk.ConsAddressFromBech32(data.PreviousProposer)
previousProposer, err = k.stakingKeeper.ConsensusAddressCodec().StringToBytes(data.PreviousProposer)
if err != nil {
panic(err)
}
Expand Down
12 changes: 8 additions & 4 deletions x/distribution/keeper/invariants.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,19 +92,23 @@ func CanWithdrawInvariant(k Keeper) sdk.Invariant {

// iterate over all validators
err = k.stakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
_, _ = k.WithdrawValidatorCommission(ctx, val.GetOperator())
valBz, err1 := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
if err != nil {
panic(err1)
}
_, _ = k.WithdrawValidatorCommission(ctx, valBz)

delegationAddrs, ok := valDelegationAddrs[val.GetOperator().String()]
delegationAddrs, ok := valDelegationAddrs[val.GetOperator()]
if ok {
for _, delAddr := range delegationAddrs {
if _, err := k.WithdrawDelegationRewards(ctx, delAddr, val.GetOperator()); err != nil {
if _, err := k.WithdrawDelegationRewards(ctx, delAddr, valBz); err != nil {
panic(err)
}
}
}

var err error
remaining, err = k.GetValidatorOutstandingRewardsCoins(ctx, val.GetOperator())
remaining, err = k.GetValidatorOutstandingRewardsCoins(ctx, valBz)
if err != nil {
panic(err)
}
Expand Down
31 changes: 20 additions & 11 deletions x/distribution/keeper/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,33 +16,42 @@ import (

// initialize rewards for a new validator
func (k Keeper) initializeValidator(ctx context.Context, val stakingtypes.ValidatorI) error {
valBz, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
if err != nil {
return err
}
// set initial historical rewards (period 0) with reference count of 1
err := k.ValidatorHistoricalRewards.Set(ctx, collections.Join(val.GetOperator(), uint64(0)), types.NewValidatorHistoricalRewards(sdk.DecCoins{}, 1))
err = k.ValidatorHistoricalRewards.Set(ctx, collections.Join(sdk.ValAddress(valBz), uint64(0)), types.NewValidatorHistoricalRewards(sdk.DecCoins{}, 1))
if err != nil {
return err
}

// set current rewards (starting at period 1)
err = k.ValidatorCurrentRewards.Set(ctx, val.GetOperator(), types.NewValidatorCurrentRewards(sdk.DecCoins{}, 1))
err = k.ValidatorCurrentRewards.Set(ctx, valBz, types.NewValidatorCurrentRewards(sdk.DecCoins{}, 1))
if err != nil {
return err
}

// set accumulated commission
err = k.ValidatorsAccumulatedCommission.Set(ctx, val.GetOperator(), types.InitialValidatorAccumulatedCommission())
err = k.ValidatorsAccumulatedCommission.Set(ctx, valBz, types.InitialValidatorAccumulatedCommission())
if err != nil {
return err
}

// set outstanding rewards
err = k.ValidatorOutstandingRewards.Set(ctx, val.GetOperator(), types.ValidatorOutstandingRewards{Rewards: sdk.DecCoins{}})
err = k.ValidatorOutstandingRewards.Set(ctx, valBz, types.ValidatorOutstandingRewards{Rewards: sdk.DecCoins{}})
return err
}

// increment validator period, returning the period just ended
func (k Keeper) IncrementValidatorPeriod(ctx context.Context, val stakingtypes.ValidatorI) (uint64, error) {
valBz, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(val.GetOperator())
if err != nil {
return 0, err
}

// fetch current rewards
rewards, err := k.ValidatorCurrentRewards.Get(ctx, val.GetOperator())
rewards, err := k.ValidatorCurrentRewards.Get(ctx, valBz)
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return 0, err
}
Expand All @@ -58,7 +67,7 @@ func (k Keeper) IncrementValidatorPeriod(ctx context.Context, val stakingtypes.V
return 0, err
}

outstanding, err := k.ValidatorOutstandingRewards.Get(ctx, val.GetOperator())
outstanding, err := k.ValidatorOutstandingRewards.Get(ctx, valBz)
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return 0, err
}
Expand All @@ -70,7 +79,7 @@ func (k Keeper) IncrementValidatorPeriod(ctx context.Context, val stakingtypes.V
return 0, err
}

err = k.ValidatorOutstandingRewards.Set(ctx, val.GetOperator(), outstanding)
err = k.ValidatorOutstandingRewards.Set(ctx, valBz, outstanding)
if err != nil {
return 0, err
}
Expand All @@ -82,27 +91,27 @@ func (k Keeper) IncrementValidatorPeriod(ctx context.Context, val stakingtypes.V
}

// fetch historical rewards for last period
historical, err := k.ValidatorHistoricalRewards.Get(ctx, collections.Join(val.GetOperator(), rewards.Period-1))
historical, err := k.ValidatorHistoricalRewards.Get(ctx, collections.Join(sdk.ValAddress(valBz), rewards.Period-1))
if err != nil {
return 0, err
}

cumRewardRatio := historical.CumulativeRewardRatio

// decrement reference count
err = k.decrementReferenceCount(ctx, val.GetOperator(), rewards.Period-1)
err = k.decrementReferenceCount(ctx, valBz, rewards.Period-1)
if err != nil {
return 0, err
}

// set new historical rewards with reference count of 1
err = k.ValidatorHistoricalRewards.Set(ctx, collections.Join(val.GetOperator(), rewards.Period), types.NewValidatorHistoricalRewards(cumRewardRatio.Add(current...), 1))
err = k.ValidatorHistoricalRewards.Set(ctx, collections.Join(sdk.ValAddress(valBz), rewards.Period), types.NewValidatorHistoricalRewards(cumRewardRatio.Add(current...), 1))
if err != nil {
return 0, err
}

// set current rewards, incrementing period by 1
err = k.ValidatorCurrentRewards.Set(ctx, val.GetOperator(), types.NewValidatorCurrentRewards(sdk.DecCoins{}, rewards.Period+1))
err = k.ValidatorCurrentRewards.Set(ctx, valBz, types.NewValidatorCurrentRewards(sdk.DecCoins{}, rewards.Period+1))
if err != nil {
return 0, err
}
Expand Down
Loading
Loading