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

misleading documentation about ValidatorSigningInfo.StartHeight #10312

Closed
0Tech opened this issue Oct 6, 2021 · 0 comments · Fixed by #11973
Closed

misleading documentation about ValidatorSigningInfo.StartHeight #10312

0Tech opened this issue Oct 6, 2021 · 0 comments · Fixed by #11973
Assignees

Comments

@0Tech
Copy link
Contributor

0Tech commented Oct 6, 2021

A comment in the proto file says start_height would be set to the height at which the validator was unjailed:

// Height at which validator was first a candidate OR was unjailed
int64 start_height = 2;

But this is not true. You can see the counter example in the test code:

// validator rejoins and starts signing again
app.StakingKeeper.Unjail(ctx, consAddr)
app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, true)
height++
// validator should not be kicked since we reset counter/array when it was jailed
staking.EndBlocker(ctx, app.StakingKeeper)
tstaking.CheckValidator(valAddr, stakingtypes.Bonded, false)
// validator misses 501 blocks
latest = height
for ; height < latest+501; height++ {
ctx = ctx.WithBlockHeight(height)
app.SlashingKeeper.HandleValidatorSignature(ctx, val.Address(), newPower, false)
}
// validator should now be jailed & kicked
staking.EndBlocker(ctx, app.StakingKeeper)
tstaking.CheckValidator(valAddr, stakingtypes.Unbonding, true)

Which means the test expects that the validator would be punished even if another 1000 blocks had not passed since the unjail.

I think at least one of the three parts should be fixed - the comment, the test code or the behavior.

Please correct me if there is anything I have missed.

@julienrbrt julienrbrt self-assigned this Apr 22, 2022
@julienrbrt julienrbrt moved this to In Progress in Cosmos SDK Maintenance Apr 25, 2022
@julienrbrt julienrbrt moved this from In Progress to Ready in Cosmos SDK Maintenance Apr 25, 2022
@julienrbrt julienrbrt moved this to Todo in Cosmos-SDK May 6, 2022
@julienrbrt julienrbrt moved this from 📝 Todo to 💪 In Progress in Cosmos-SDK May 10, 2022
@julienrbrt julienrbrt moved this from 💪 In Progress to 👀 Needs Review in Cosmos-SDK May 16, 2022
Repository owner moved this from 👀 Needs Review to 👏 Done in Cosmos-SDK May 18, 2022
Repository owner moved this from Ready to Done in Cosmos SDK Maintenance May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
2 participants