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.

Sign up for GitHub

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 #16958

Merged
merged 20 commits into from
Jul 19, 2023
Merged

refactor(staking): use validator & address codecs in staking #16958

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3cf7e9b
sdk.Accaddress -> String
tac0turtle Jul 12, 2023
0aaf01d
minor cleanup
tac0turtle Jul 13, 2023
3ffb3fe
Merge branch 'main' into marko/delegatorAddress_string
tac0turtle Jul 13, 2023
82d4e61
fix sims
tac0turtle Jul 13, 2023
69bdeaf
Merge branch 'main' into marko/delegatorAddress_string
tac0turtle Jul 13, 2023
e0640a8
migrate GetVallAddress to string
tac0turtle Jul 14, 2023
92096fd
alt type
tac0turtle Jul 14, 2023
6b2c930
valaddress codec in dist
tac0turtle Jul 14, 2023
6214875
pull in vall addresscodec
tac0turtle Jul 14, 2023
0f66d1b
fix build
tac0turtle Jul 15, 2023
656e6ab
fix linting
tac0turtle Jul 16, 2023
d472036
fix distribution tests
tac0turtle Jul 16, 2023
989d6fe
do not call .String in newdelegation
tac0turtle Jul 16, 2023
10460de
fix tests
tac0turtle Jul 17, 2023
e4ae428
fix err hoisting
tac0turtle Jul 18, 2023
90915e7
cleanup form review
tac0turtle Jul 18, 2023
99aac9a
changelog entry
tac0turtle Jul 18, 2023
cf7224f
Merge branch 'main' into marko/delegatorAddress_string
tac0turtle Jul 18, 2023
c645284
update comments in proto
tac0turtle Jul 18, 2023
b9d96c3
linting fix
tac0turtle Jul 19, 2023
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
25 changes: 18 additions & 7 deletions x/distribution/keeper/delegation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,14 @@ 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
}

// fetch starting info for delegation
startingInfo, err := k.DelegatorStartingInfo.Get(ctx, collections.Join(del.GetValidatorAddr(), del.GetDelegatorAddr()))
startingInfo, err := k.DelegatorStartingInfo.Get(ctx, collections.Join(del.GetValidatorAddr(), sdk.AccAddress(delAddr)))
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return sdk.DecCoins{}, err
}
Expand Down Expand Up @@ -178,8 +184,13 @@ 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
}
// check existence of delegator starting info
hasInfo, err := k.DelegatorStartingInfo.Has(ctx, collections.Join(del.GetValidatorAddr(), del.GetDelegatorAddr()))
hasInfo, err := k.DelegatorStartingInfo.Has(ctx, collections.Join(del.GetValidatorAddr(), sdk.AccAddress(delAddr)))
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -211,7 +222,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 @@ -223,7 +234,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 Down Expand Up @@ -253,7 +264,7 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.
}

// decrement reference count of starting period
startingInfo, err := k.DelegatorStartingInfo.Get(ctx, collections.Join(del.GetValidatorAddr(), del.GetDelegatorAddr()))
startingInfo, err := k.DelegatorStartingInfo.Get(ctx, collections.Join(del.GetValidatorAddr(), sdk.AccAddress(delAddr)))
if err != nil && !errors.Is(err, collections.ErrNotFound) {
return nil, err
}
Expand All @@ -265,7 +276,7 @@ func (k Keeper) withdrawDelegationRewards(ctx context.Context, val stakingtypes.
}

// remove delegator starting info
err = k.DelegatorStartingInfo.Remove(ctx, collections.Join(del.GetValidatorAddr(), del.GetDelegatorAddr()))
err = k.DelegatorStartingInfo.Remove(ctx, collections.Join(del.GetValidatorAddr(), sdk.AccAddress(delAddr)))
if err != nil {
return nil, err
}
Expand All @@ -287,7 +298,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
8 changes: 6 additions & 2 deletions x/distribution/keeper/invariants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,19 @@ func CanWithdrawInvariant(k Keeper) sdk.Invariant {

var remaining sdk.DecCoins

valDelegationAddrs := make(map[string][]sdk.AccAddress)
valDelegationAddrs := make(map[string][][]byte)
allDelegations, err := k.stakingKeeper.GetAllSDKDelegations(ctx)
if err != nil {
panic(err)
}

for _, del := range allDelegations {
delAddr, err := k.authKeeper.AddressCodec().StringToBytes(del.GetDelegatorAddr())
if err != nil {
panic(err)
}
valAddr := del.GetValidatorAddr().String()
valDelegationAddrs[valAddr] = append(valDelegationAddrs[valAddr], del.GetDelegatorAddr())
valDelegationAddrs[valAddr] = append(valDelegationAddrs[valAddr], delAddr)
}

// iterate over all validators
Expand Down
28 changes: 19 additions & 9 deletions x/staking/simulation/operations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,12 @@ func SimulateMsgUndelegate(
delegation := delegations[r.Intn(len(delegations))]
delAddr := delegation.GetDelegatorAddr()

hasMaxUD, err := k.HasMaxUnbondingDelegationEntries(ctx, delAddr, valAddr)
delAddrBz, err := ak.AddressCodec().StringToBytes(delAddr)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, msgType, "error getting delegator address bytes"), nil, err
}

hasMaxUD, err := k.HasMaxUnbondingDelegationEntries(ctx, delAddrBz, valAddr)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, msgType, "error getting max unbonding delegation entries"), nil, err
}
Expand Down Expand Up @@ -408,14 +413,14 @@ func SimulateMsgUndelegate(
}

msg := types.NewMsgUndelegate(
delAddr, valAddr, sdk.NewCoin(bondDenom, unbondAmt),
delAddrBz, valAddr, sdk.NewCoin(bondDenom, unbondAmt),
)

// need to retrieve the simulation account associated with delegation to retrieve PrivKey
var simAccount simtypes.Account

for _, simAcc := range accs {
if simAcc.Address.Equals(delAddr) {
if simAcc.Address.Equals(sdk.AccAddress(delAddrBz)) {
simAccount = simAcc
break
}
Expand All @@ -425,7 +430,7 @@ func SimulateMsgUndelegate(
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "account private key is nil"), nil, nil
}

account := ak.GetAccount(ctx, delAddr)
account := ak.GetAccount(ctx, delAddrBz)
spendable := bk.SpendableCoins(ctx, account.GetAddress())

txCtx := simulation.OperationInput{
Expand Down Expand Up @@ -583,7 +588,12 @@ func SimulateMsgBeginRedelegate(
delegation := delegations[r.Intn(len(delegations))]
delAddr := delegation.GetDelegatorAddr()

hasRecRedel, err := k.HasReceivingRedelegation(ctx, delAddr, srcAddr)
delAddrBz, err := ak.AddressCodec().StringToBytes(delAddr)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, msgType, "error getting delegator address bytes"), nil, err
}

hasRecRedel, err := k.HasReceivingRedelegation(ctx, delAddrBz, srcAddr)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, msgType, "error getting receiving redelegation"), nil, err
}
Expand All @@ -599,7 +609,7 @@ func SimulateMsgBeginRedelegate(
}

destAddr := destVal.GetOperator()
hasMaxRedel, err := k.HasMaxRedelegationEntries(ctx, delAddr, srcAddr, destAddr)
hasMaxRedel, err := k.HasMaxRedelegationEntries(ctx, delAddrBz, srcAddr, destAddr)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, msgType, "error getting max redelegation entries"), nil, err
}
Expand Down Expand Up @@ -636,7 +646,7 @@ func SimulateMsgBeginRedelegate(
var simAccount simtypes.Account

for _, simAcc := range accs {
if simAcc.Address.Equals(delAddr) {
if simAcc.Address.Equals(sdk.AccAddress(delAddrBz)) {
simAccount = simAcc
break
}
Expand All @@ -647,7 +657,7 @@ func SimulateMsgBeginRedelegate(
return simtypes.NoOpMsg(types.ModuleName, msgType, "account private key is nil"), nil, nil
}

account := ak.GetAccount(ctx, delAddr)
account := ak.GetAccount(ctx, delAddrBz)
spendable := bk.SpendableCoins(ctx, account.GetAddress())

bondDenom, err := k.BondDenom(ctx)
Expand All @@ -656,7 +666,7 @@ func SimulateMsgBeginRedelegate(
}

msg := types.NewMsgBeginRedelegate(
delAddr, srcAddr, destAddr,
delAddrBz, srcAddr, destAddr,
sdk.NewCoin(bondDenom, redAmt),
)

Expand Down
6 changes: 2 additions & 4 deletions x/staking/types/delegation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@ func UnmarshalDelegation(cdc codec.BinaryCodec, value []byte) (delegation Delega
return delegation, err
}

func (d Delegation) GetDelegatorAddr() sdk.AccAddress {
delAddr := sdk.MustAccAddressFromBech32(d.DelegatorAddress)

return delAddr
func (d Delegation) GetDelegatorAddr() string {
return d.DelegatorAddress
}

func (d Delegation) GetValidatorAddr() sdk.ValAddress {
Expand Down
2 changes: 1 addition & 1 deletion x/staking/types/exported.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

// DelegationI delegation bond for a delegated proof of stake system
type DelegationI interface {
GetDelegatorAddr() sdk.AccAddress // delegator sdk.AccAddress for the bond
GetDelegatorAddr() string // delegator string for the bond
GetValidatorAddr() sdk.ValAddress // validator operator address
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i can migrate this one once we merge #16959, that way this interface is consistent

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tac0turtle why not use bytes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would need to do a state migration, this bled into state, we will get rid of it in the next release or the upcoming rewrite of staking

GetShares() math.LegacyDec // amount of validator's shares held in this delegation
}
Expand Down