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

test(cip-29): reduce inflation #4299

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Feb 5, 2025

@rootulp rootulp self-assigned this Feb 5, 2025
@rootulp
Copy link
Collaborator Author

rootulp commented Feb 5, 2025

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:

InitialInflationRate * ((1 - DisinflationRate) ^ YearsSinceGenesis)

or rather the formula needs a conditional where it selectively switches based on the app version.

@rootulp
Copy link
Collaborator Author

rootulp commented Feb 10, 2025

@tac0turtle when I use the reference implementation in CIP-29, the inflation numbers diverge from what is presented in the table. I'm observing

		testCases := []testCase{
			{0, 0.0536}, // note this value won't be used in production because celestia-app was on app version 3 in year 0.
			{1, 0.04824},
			{2, 0.040507128},
			{3, 0.0364564152},
		}

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 {
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants