-
Notifications
You must be signed in to change notification settings - Fork 386
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
test(cip-29): reduce inflation #4299
base: main
Are you sure you want to change the base?
Conversation
Discussed w/ @tac0turtle offline and this PR currently assumes that the constants for initial inflation rate and disinflation rate decreased by 33% but the CIP is actually suggesting retaining the constants as is and then decreasing them by 33% when calculating inflation. It means we can't use this formula any more:
or rather the formula needs a conditional where it selectively switches based on the app version. |
@tac0turtle when I use the reference implementation in CIP-29, the inflation numbers diverge from what is presented in the table. I'm observing
when the table says https://github.com/celestiaorg/CIPs/pull/249/files#diff-5bc579390f911ecfc45fde43055ce471aea57812b146e1669a507682deb3ef15R57-R59 |
@@ -51,6 +59,35 @@ func (m Minter) CalculateInflationRate(ctx sdk.Context, genesis time.Time) sdk.D | |||
return inflationRate | |||
} | |||
|
|||
func calculateInflationRatePostCip29(ctx sdk.Context, genesis time.Time) sdk.Dec { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here the math is slightly off same with the code in the CIP.
Here we are not doing a reduction of the previous year inflation we are taking the original inflation rate for that year reducing it by 33% and then reducing it by 6.7%
in order to calculate the correct inflation we would need to do something like so
func calculateInflationRatePostCip29(ctx sdk.Context, genesis time.Time) sdk.Dec {
if ctx.ConsensusParams().Version.AppVersion <= 3 {
panic("calculateInflationRatePostCip29 should not be called with AppVersion <= 3")
}
years := yearsSinceGenesis(genesis, ctx.BlockTime())
initialInflationRate := InitialInflationRateAsDec().Mul(sdk.OneDec().Sub(DisinflationRateAsDec()).Power(uint64(1)))
updatedInflationRate := sdk.Dec{}
// For AppVersion > 3, adjust the inflation rate:
if ctx.ConsensusParams().Version.AppVersion > 3 {
// First, reduce the current inflation rate by 33%
updatedInflationRate = initialInflationRate.Mul(sdk.NewDecWithPrec(67, 2)) // 0.67 \= 67%
// Then, if we are in year two or later, apply a one-time disinflation of 6.7%
if years >= 2 {
for i := 1; i < int(years); i++ {
updatedInflationRate = updatedInflationRate.Mul(sdk.OneDec().Sub(sdk.NewDecWithPrec(67, 3))) // 1 \- 0.067 \= 0.933
}
}
}
// Ensure the inflation rate does not fall below the target inflation rate.
if updatedInflationRate.LT(TargetInflationRateAsDec()) {
return TargetInflationRateAsDec()
}
return updatedInflationRate
}
the code isnt the nicest, would be happy to simplify and change the numbers as they are not that far off.
cc: @tac0turtle