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(staking): use validator & address codecs in staking (backport #16958) #17066

Merged
merged 6 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (staking) [#16959](https://github.com/cosmos/cosmos-sdk/pull/16959) Add validator and consensus address codec as staking keeper arguments.
* (x/staking) [#16958](https://github.com/cosmos/cosmos-sdk/pull/16958) DelegationI interface `GetDelegatorAddr` & `GetValidatorAddr` have been migrated to return string instead of sdk.AccAddress and sdk.ValAddress respectively. stakingtypes.NewDelegation takes a string instead of sdk.AccAddress and sdk.ValAddress.
* (x/staking) [#16959](https://github.com/cosmos/cosmos-sdk/pull/16959) Add validator and consensus address codec as staking keeper arguments.
* (types) [#16272](https://github.com/cosmos/cosmos-sdk/pull/16272) `FeeGranter` in the `FeeTx` interface returns `[]byte` instead of `string`.
* (testutil) [#16899](https://github.com/cosmos/cosmos-sdk/pull/16899) The *cli testutil* `QueryBalancesExec` has been removed. Use the gRPC or REST query instead.
* (x/auth) [#16650](https://github.com/cosmos/cosmos-sdk/pull/16650) The *cli testutil* `QueryAccountExec` has been removed. Use the gRPC or REST query instead.
Expand Down
8 changes: 4 additions & 4 deletions api/cosmos/staking/v1beta1/staking.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions proto/cosmos/staking/v1beta1/staking.proto
Original file line number Diff line number Diff line change
Expand Up @@ -196,9 +196,9 @@ message Delegation {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

// delegator_address is the bech32-encoded address of the delegator.
// delegator_address is the encoded address of the delegator.
string delegator_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// validator_address is the bech32-encoded address of the validator.
// validator_address is the encoded address of the validator.
string validator_address = 2 [(cosmos_proto.scalar) = "cosmos.ValidatorAddressString"];
// shares define the delegation shares received.
string shares = 3 [
Expand All @@ -214,13 +214,13 @@ message UnbondingDelegation {
option (gogoproto.equal) = false;
option (gogoproto.goproto_getters) = false;

// delegator_address is the bech32-encoded address of the delegator.
// delegator_address is the encoded address of the delegator.
string delegator_address = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
// validator_address is the bech32-encoded address of the validator.
// validator_address is the encoded address of the validator.
string validator_address = 2 [(cosmos_proto.scalar) = "cosmos.ValidatorAddressString"];
// entries are the unbonding delegation entries.
repeated UnbondingDelegationEntry entries = 3
[(gogoproto.nullable) = false, (amino.dont_omitempty) = true]; // unbonding delegation entries
[(gogoproto.nullable) = false, (amino.dont_omitempty) = true]; // unbonding delegation entries
}

// UnbondingDelegationEntry defines an unbonding object with relevant metadata.
Expand Down Expand Up @@ -293,7 +293,7 @@ message Redelegation {
string validator_dst_address = 3 [(cosmos_proto.scalar) = "cosmos.ValidatorAddressString"];
// entries are the redelegation entries.
repeated RedelegationEntry entries = 4
[(gogoproto.nullable) = false, (amino.dont_omitempty) = true]; // redelegation entries
[(gogoproto.nullable) = false, (amino.dont_omitempty) = true]; // redelegation entries
}

// Params defines the parameters for the x/staking module.
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/distribution/grpc_query_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (s *GRPCQueryTestSuite) TestQueryDelegatorRewardsGRPC() {
&types.QueryDelegationTotalRewardsResponse{},
&types.QueryDelegationTotalRewardsResponse{
Rewards: []types.DelegationDelegatorReward{
types.NewDelegationDelegatorReward(val.ValAddress, rewards),
types.NewDelegationDelegatorReward(val.ValAddress.String(), rewards),
},
Total: rewards,
},
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/distribution/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,7 @@ func TestGRPCDelegationRewards(t *testing.T) {
// setup delegation
delTokens := sdk.TokensFromConsensusPower(2, sdk.DefaultPowerReduction)
validator, issuedShares := val.AddTokensFromDel(delTokens)
delegation := stakingtypes.NewDelegation(delAddr, f.valAddr, issuedShares)
delegation := stakingtypes.NewDelegation(delAddr.String(), f.valAddr.String(), issuedShares)
assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, delegation))
assert.NilError(t, f.distrKeeper.SetDelegatorStartingInfo(f.sdkCtx, validator.GetOperator(), delAddr, types.NewDelegatorStartingInfo(2, math.LegacyNewDec(initialStake), 20)))

Expand Down
8 changes: 4 additions & 4 deletions tests/integration/distribution/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,10 @@ func TestMsgWithdrawDelegatorReward(t *testing.T) {
// setup delegation
delTokens := sdk.TokensFromConsensusPower(2, sdk.DefaultPowerReduction)
validator, issuedShares := validator.AddTokensFromDel(delTokens)
delegation := stakingtypes.NewDelegation(delAddr, validator.GetOperator(), issuedShares)
assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, delegation))
err = f.distrKeeper.SetDelegatorStartingInfo(f.sdkCtx, validator.GetOperator(), delAddr, distrtypes.NewDelegatorStartingInfo(2, math.LegacyOneDec(), 20))
require.NoError(t, err)

delegation := stakingtypes.NewDelegation(delAddr.String(), validator.GetOperator().String(), issuedShares)
require.NoError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, delegation))
require.NoError(t, f.distrKeeper.SetDelegatorStartingInfo(f.sdkCtx, validator.GetOperator(), delAddr, distrtypes.NewDelegatorStartingInfo(2, math.LegacyOneDec(), 20)))
// setup validator rewards
decCoins := sdk.DecCoins{sdk.NewDecCoinFromDec(sdk.DefaultBondDenom, math.LegacyOneDec())}
historicalRewards := distrtypes.NewValidatorHistoricalRewards(decCoins, 2)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/staking/keeper/delegation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestUnbondingDelegationsMaxEntries(t *testing.T) {
assert.Assert(math.IntEq(t, startTokens, validator.BondedTokens()))
assert.Assert(t, validator.IsBonded())

delegation := types.NewDelegation(addrDel, addrVal, issuedShares)
delegation := types.NewDelegation(addrDel.String(), addrVal.String(), issuedShares)
assert.NilError(t, f.stakingKeeper.SetDelegation(ctx, delegation))

maxEntries, err := f.stakingKeeper.MaxEntries(ctx)
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/staking/keeper/slash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestSlashRedelegation(t *testing.T) {
assert.NilError(t, f.stakingKeeper.SetRedelegation(f.sdkCtx, rd))

// set the associated delegation
del := types.NewDelegation(addrDels[0], addrVals[1], math.LegacyNewDec(10))
del := types.NewDelegation(addrDels[0].String(), addrVals[1].String(), math.LegacyNewDec(10))
assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, del))

// started redelegating prior to the current height, stake didn't contribute to infraction
Expand Down Expand Up @@ -392,7 +392,7 @@ func TestSlashWithRedelegation(t *testing.T) {
assert.NilError(t, f.stakingKeeper.SetRedelegation(f.sdkCtx, rd))

// set the associated delegation
del := types.NewDelegation(addrDels[0], addrVals[1], math.LegacyNewDecFromInt(rdTokens))
del := types.NewDelegation(addrDels[0].String(), addrVals[1].String(), math.LegacyNewDecFromInt(rdTokens))
assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, del))

// update bonded tokens
Expand Down Expand Up @@ -550,7 +550,7 @@ func TestSlashBoth(t *testing.T) {
assert.NilError(t, f.stakingKeeper.SetRedelegation(f.sdkCtx, rdA))

// set the associated delegation
delA := types.NewDelegation(addrDels[0], addrVals[1], math.LegacyNewDecFromInt(rdATokens))
delA := types.NewDelegation(addrDels[0].String(), addrVals[1].String(), math.LegacyNewDecFromInt(rdATokens))
assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, delA))

// set an unbonding delegation with expiration timestamp (beyond which the
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/staking/keeper/unbonding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func SetupUnbondingTests(t *testing.T, f *fixture, hookCalled *bool, ubdeID *uin
assert.Assert(t, validator1.IsBonded())

// Create a delegator
delegation := types.NewDelegation(addrDels[0], addrVals[0], issuedShares1)
delegation := types.NewDelegation(addrDels[0].String(), addrVals[0].String(), issuedShares1)
assert.NilError(t, f.stakingKeeper.SetDelegation(f.sdkCtx, delegation))

// Create a validator to redelegate to
Expand Down
28 changes: 20 additions & 8 deletions tests/integration/staking/keeper/validator_bench_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package keeper_test

import (
"bytes"
"fmt"
"testing"

Expand Down Expand Up @@ -59,8 +60,11 @@ func BenchmarkGetValidatorDelegations(b *testing.B) {
delegator := sdk.AccAddress(fmt.Sprintf("address%d", i))
banktestutil.FundAccount(f.sdkCtx, f.bankKeeper, delegator,
sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(int64(i)))))
NewDel := types.NewDelegation(delegator, val, math.LegacyNewDec(int64(i)))
f.stakingKeeper.SetDelegation(f.sdkCtx, NewDel)
NewDel := types.NewDelegation(delegator.String(), val.String(), math.LegacyNewDec(int64(i)))

if err := f.stakingKeeper.SetDelegation(f.sdkCtx, NewDel); err != nil {
panic(err)
}
}
}

Expand Down Expand Up @@ -90,10 +94,11 @@ func BenchmarkGetValidatorDelegationsLegacy(b *testing.B) {
for _, val := range valAddrs {
for i := 0; i < delegationsNum; i++ {
delegator := sdk.AccAddress(fmt.Sprintf("address%d", i))
banktestutil.FundAccount(f.sdkCtx, f.bankKeeper, delegator,
sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(int64(i)))))
NewDel := types.NewDelegation(delegator, val, math.LegacyNewDec(int64(i)))
f.stakingKeeper.SetDelegation(f.sdkCtx, NewDel)
banktestutil.FundAccount(f.sdkCtx, f.bankKeeper, delegator, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, math.NewInt(int64(i)))))
NewDel := types.NewDelegation(delegator.String(), val.String(), math.LegacyNewDec(int64(i)))
if err := f.stakingKeeper.SetDelegation(f.sdkCtx, NewDel); err != nil {
panic(err)
}
}
}

Expand All @@ -114,8 +119,15 @@ func updateValidatorDelegationsLegacy(f *fixture, existingValAddr, newValAddr sd

for ; iterator.Valid(); iterator.Next() {
delegation := types.MustUnmarshalDelegation(cdc, iterator.Value())
if delegation.GetValidatorAddr().Equals(existingValAddr) {
k.RemoveDelegation(f.sdkCtx, delegation)
valAddr, err := k.ValidatorAddressCodec().StringToBytes(delegation.GetValidatorAddr())
if err != nil {
panic(err)
}

if bytes.EqualFold(valAddr, existingValAddr) {
if err := k.RemoveDelegation(f.sdkCtx, delegation); err != nil {
panic(err)
}
delegation.ValidatorAddress = newValAddr.String()
k.SetDelegation(f.sdkCtx, delegation)
}
Expand Down
2 changes: 1 addition & 1 deletion testutil/sims/app_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ func GenesisStateWithValSet(
MinSelfDelegation: sdkmath.ZeroInt(),
}
validators = append(validators, validator)
delegations = append(delegations, stakingtypes.NewDelegation(genAccs[0].GetAddress(), val.Address.Bytes(), sdkmath.LegacyOneDec()))
delegations = append(delegations, stakingtypes.NewDelegation(genAccs[0].GetAddress().String(), sdk.ValAddress(val.Address).String(), sdkmath.LegacyOneDec()))

}

Expand Down
44 changes: 33 additions & 11 deletions x/distribution/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,19 @@ func (k Keeper) calculateDelegationRewardsBetween(ctx context.Context, val staki

// calculate the total rewards accrued by a delegation
func (k Keeper) CalculateDelegationRewards(ctx context.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI, endingPeriod uint64) (rewards sdk.DecCoins, err error) {
addrCodec := k.authKeeper.AddressCodec()
delAddr, err := addrCodec.StringToBytes(del.GetDelegatorAddr())
Fixed Show fixed Hide fixed
if err != nil {
return sdk.DecCoins{}, err
}

valAddr, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(del.GetValidatorAddr())
Fixed Show fixed Hide fixed
if err != nil {
return sdk.DecCoins{}, err
}

// fetch starting info for delegation
startingInfo, err := k.GetDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
startingInfo, err := k.GetDelegatorStartingInfo(ctx, sdk.ValAddress(valAddr), sdk.AccAddress(delAddr))
if err != nil {
return
}
Expand All @@ -107,7 +118,7 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val stakingtypes
// for them for the stake sanity check below.
endingHeight := uint64(sdkCtx.BlockHeight())
if endingHeight > startingHeight {
k.IterateValidatorSlashEventsBetween(ctx, del.GetValidatorAddr(), startingHeight, endingHeight,
k.IterateValidatorSlashEventsBetween(ctx, valAddr, startingHeight, endingHeight,
func(height uint64, event types.ValidatorSlashEvent) (stop bool) {
endingPeriod := event.ValidatorPeriod
if endingPeriod > startingPeriod {
Expand Down Expand Up @@ -176,8 +187,19 @@ func (k Keeper) CalculateDelegationRewards(ctx context.Context, val stakingtypes
}

func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.ValidatorI, del stakingtypes.DelegationI) (sdk.Coins, error) {
addrCodec := k.authKeeper.AddressCodec()
delAddr, err := addrCodec.StringToBytes(del.GetDelegatorAddr())
if err != nil {
return nil, err
}

valAddr, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(del.GetValidatorAddr())
if err != nil {
return nil, err
}

// check existence of delegator starting info
hasInfo, err := k.HasDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
hasInfo, err := k.HasDelegatorStartingInfo(ctx, sdk.ValAddress(valAddr), sdk.AccAddress(delAddr))
if err != nil {
return nil, err
}
Expand All @@ -197,7 +219,7 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.
return nil, err
}

outstanding, err := k.GetValidatorOutstandingRewardsCoins(ctx, del.GetValidatorAddr())
outstanding, err := k.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(valAddr))
if err != nil {
return nil, err
}
Expand All @@ -209,7 +231,7 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.
logger := k.Logger(ctx)
logger.Info(
"rounding error withdrawing rewards from validator",
"delegator", del.GetDelegatorAddr().String(),
"delegator", del.GetDelegatorAddr(),
"validator", val.GetOperator().String(),
"got", rewards.String(),
"expected", rewardsRaw.String(),
Expand All @@ -221,7 +243,7 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.

// add coins to user account
if !finalRewards.IsZero() {
withdrawAddr, err := k.GetDelegatorWithdrawAddr(ctx, del.GetDelegatorAddr())
withdrawAddr, err := k.GetDelegatorWithdrawAddr(ctx, delAddr)
if err != nil {
return nil, err
}
Expand All @@ -234,7 +256,7 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.

// update the outstanding rewards and the community pool only if the
// transaction was successful
err = k.SetValidatorOutstandingRewards(ctx, del.GetValidatorAddr(), types.ValidatorOutstandingRewards{Rewards: outstanding.Sub(rewards)})
err = k.SetValidatorOutstandingRewards(ctx, sdk.ValAddress(valAddr), types.ValidatorOutstandingRewards{Rewards: outstanding.Sub(rewards)})
if err != nil {
return nil, err
}
Expand All @@ -251,19 +273,19 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.
}

// decrement reference count of starting period
startingInfo, err := k.GetDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
startingInfo, err := k.GetDelegatorStartingInfo(ctx, sdk.ValAddress(valAddr), sdk.AccAddress(delAddr))
if err != nil {
return nil, err
}

startingPeriod := startingInfo.PreviousPeriod
err = k.decrementReferenceCount(ctx, del.GetValidatorAddr(), startingPeriod)
err = k.decrementReferenceCount(ctx, sdk.ValAddress(valAddr), startingPeriod)
if err != nil {
return nil, err
}

// remove delegator starting info
err = k.DeleteDelegatorStartingInfo(ctx, del.GetValidatorAddr(), del.GetDelegatorAddr())
err = k.DeleteDelegatorStartingInfo(ctx, sdk.ValAddress(valAddr), sdk.AccAddress(delAddr))
if err != nil {
return nil, err
}
Expand All @@ -285,7 +307,7 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.
types.EventTypeWithdrawRewards,
sdk.NewAttribute(sdk.AttributeKeyAmount, finalRewards.String()),
sdk.NewAttribute(types.AttributeKeyValidator, val.GetOperator().String()),
sdk.NewAttribute(types.AttributeKeyDelegator, del.GetDelegatorAddr().String()),
sdk.NewAttribute(types.AttributeKeyDelegator, del.GetDelegatorAddr()),
),
)

Expand Down
Loading