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

Validator.UnbondingHeight meaning is inconsistent, misused and not useful. #12486

Open
4 tasks
danwt opened this issue Jul 7, 2022 · 0 comments
Open
4 tasks
Labels
T:Docs Changes and features related to documentation.

Comments

@danwt
Copy link
Contributor

danwt commented Jul 7, 2022

Summary of Bug

The field Validator.UnbondingHeight in the staking module

// unbonding_height defines, if unbonding, the height at which this validator has begun unbonding.
UnbondingHeight int64 `protobuf:"varint,8,opt,name=unbonding_height,json=unbondingHeight,proto3" json:"unbonding_height,omitempty"`

is not useful or correctly used and is just confusing.

Version

v0.45.6
https://github.com/cosmos/cosmos-sdk/tree/v0.45.6

Explanation

Unbonding validator validator.UnbondingHeight set =ctx.BlockHeader().Height

// set the unbonding completion time and completion height appropriately
validator.UnbondingTime = ctx.BlockHeader().Time.Add(params.UnbondingTime)
validator.UnbondingHeight = ctx.BlockHeader().Height
// save the now unbonded validator record and power index
k.SetValidator(ctx, validator)
k.SetValidatorByPowerIndex(ctx, validator)
// Adds to unbonding validator queue
k.InsertUnbondingValidatorQueue(ctx, validator)

and correctly documented here

// unbonding_height defines, if unbonding, the height at which this validator has begun unbonding.
UnbondingHeight int64 `protobuf:"varint,8,opt,name=unbonding_height,json=unbondingHeight,proto3" json:"unbonding_height,omitempty"`

but misused here (comment is wrong)

// getBeginInfo returns the completion time and height of a redelegation, along
// with a boolean signaling if the redelegation is complete based on the source
// validator.
func (k Keeper) getBeginInfo(
ctx sdk.Context, valSrcAddr sdk.ValAddress,
) (completionTime time.Time, height int64, completeNow bool) {
validator, found := k.GetValidator(ctx, valSrcAddr)
// TODO: When would the validator not be found?
switch {
case !found || validator.IsBonded():
// the longest wait - just unbonding period from now
completionTime = ctx.BlockHeader().Time.Add(k.UnbondingTime(ctx))
height = ctx.BlockHeight()
return completionTime, height, false
case validator.IsUnbonded():
return completionTime, height, true
case validator.IsUnbonding():
return validator.UnbondingTime, validator.UnbondingHeight, false

and again here by checking keyHeight <= blockHeight (validator.UnbondingHeight <= now) which is trivially true

key := unbondingValIterator.Key()
keyTime, keyHeight, err := types.ParseValidatorQueueKey(key)
if err != nil {
panic(fmt.Errorf("failed to parse unbonding key: %w", err))
}
// All addresses for the given key have the same unbonding height and time.
// We only unbond if the height and time are less than the current height
// and time.
if keyHeight <= blockHeight && (keyTime.Before(blockTime) || keyTime.Equal(blockTime)) {


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:Docs Changes and features related to documentation.
Projects
None yet
Development

No branches or pull requests

2 participants