Skip to content

Commit

Permalink
refactor(staking): use validator & address codecs in staking (backport
Browse files Browse the repository at this point in the history
…#16958) (#17066)

Co-authored-by: Marko <marbar3778@yahoo.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>
  • Loading branch information
3 people committed Jul 19, 2023
1 parent 609a4e8 commit 16ab635
Show file tree
Hide file tree
Showing 42 changed files with 302 additions and 167 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,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())
if err != nil {
return sdk.DecCoins{}, err
}

valAddr, err := k.stakingKeeper.ValidatorAddressCodec().StringToBytes(del.GetValidatorAddr())
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

0 comments on commit 16ab635

Please sign in to comment.