feat: allow scaling of trusting period for client upgrades #8185
Conversation
damiannolan
requested review from
AdityaSripal,
gjermundgaraba and
srdtrk
as code owners
| { | ||
| name: "successful upgrade scales trusting period with unbonding period decrease", | ||
| setup: func() { | ||
| newUnbondingPeriod := time.Hour * 24 * 7 * 2 |
There was a problem hiding this comment.
Lemme know if you want a dedicated test func to perform assertions on new trusting period.
This test case covers the code but doesn't do strict assertions.
There was a problem hiding this comment.
I think it would be good to assert the new trusting period is what we expect
There was a problem hiding this comment.
Agree, I'll add a separate test func for this. Will try to get around to it tomorrow or so
| decreaseRatio := origUnbondingDec.Sub(newUnbondingDec).Quo(origUnbondingDec) | ||
|
|
||
| // compute new trusting period: trustingPeriod * (1 - decreaseRatio) | ||
| newTrustingPeriodDec := trustingPeriodDec.Mul(sdkmath.LegacyOneDec().Sub(decreaseRatio)) |
There was a problem hiding this comment.
Isn't it much simpler to just do:
newTrustingPeriodDec := trustingPeeriodDec.Mul(newUnbondingDec).Quo(origUnbondingDec)
This is equivalent to what you have if I'm not mistaken
There was a problem hiding this comment.
lol, yes, I think they are equivalent. Happy to use this if you find it more readable. I can update!
| { | ||
| name: "successful upgrade scales trusting period with unbonding period decrease", | ||
| setup: func() { | ||
| newUnbondingPeriod := time.Hour * 24 * 7 * 2 |
There was a problem hiding this comment.
I think it would be good to assert the new trusting period is what we expect
Description
Drafting this PR. I can add a finer grained test but stepping through when reducing unbonding period from 21 days to 14 where trusting period is 14, results in a new unbonding period of 14 days and trusting period of 9.33 days
closes: #8184
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.