-
Notifications
You must be signed in to change notification settings - Fork 47
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
CIP: reduce inflation #249
Conversation
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.
[blocking] the reference implementation code looks incorrect b/c it contains escape characters.
[optional] IMO the current CIP doesn't have a convincing motivation section. I think it would help to articulate clear quantifiable goals and how the proposed inflation schedule meets those goals. The proposed schedule seems a bit arbitrary.
cips/cip-29.md
Outdated
Reasons for this CIP are: | ||
|
||
1. Current issuance is too high, especially in dollar-terms. We want to avoid accelerated dilution of non-stakers. | ||
2. To empower applications to compete effectively with staking yields. While it is important to maintain a high bond ratio to secure the network, we also envision a vibrant ecosystem of diverse applications emerging on Celestia-secured rollups. Using TIA as collateral onchain competes with staking yield and reducing inflation makes onchain use more attractive. |
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.
While it is important to maintain a high bond ratio to secure the network
Can this quantify a target bond ratio?
cips/cip-29.md
Outdated
|
||
* The chain upgrade process includes the new inflation parameters without disrupting block production. | ||
|
||
* The new schedule is included in the next voting proposal to reflect the updated inflation rates on-chain. |
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.
in the next voting proposal
What is the next voting proposal? Is this referring to a governance proposal?
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.
This probably meant to say
* The new schedule is included in the next voting proposal to reflect the updated inflation rates on-chain. | |
* The new schedule is included in the next major version release to reflect the updated inflation rates on-chain in the next app version (v4). |
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Rootul P <[email protected]>
The escape characters in the reference implementation. The second point about quantifiable goals is an optional suggestion. |
cips/cip-29.md
Outdated
|
||
* The chain upgrade process includes the new inflation parameters without disrupting block production. | ||
|
||
* The new schedule is included in the next voting proposal to reflect the updated inflation rates on-chain. |
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.
This probably meant to say
* The new schedule is included in the next voting proposal to reflect the updated inflation rates on-chain. | |
* The new schedule is included in the next major version release to reflect the updated inflation rates on-chain in the next app version (v4). |
cips/cip-29.md
Outdated
| **0** | 8.00 | 8.00 | Genesis year, no change. | | ||
| **1** | 7.20 | 7.20 | First disinflation applied (10%). | | ||
| **1.5** | 7.20 | 4.82 | 33% drop in inflation applied, disinflation rate reduced to 6.7%. (This will be implemented in the next upgrade of Celestia) | | ||
| **2** | 6.48 | 4.50 | Regular annual disinflation applied (6.7%). | |
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.
I'm calculating different numbers for the adjusted inflation. Here are the existing constants:
initialInflation = .08
disinflation = .1
The "New Adjusted Inflation" column is decreasing both of those by 33% so
initialInflation = .0536
disinflation = .067
The formula to calculate inflation rate in a given year is:
InitialInflationRate * ((1 - DisinflationRate) ^ YearsSinceGenesis)
let's skip year 0 and 1 because we can hard-code them to use the previous constants. For year 1.5
.0536 * (1 - .067) ^ 1 = 0.0500088
For year 2
.0536 * (1 - .067) ^ 2 = 0.04665821
which diverge from the values in the table.
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.
I think it's the formula that might not be correct. I just double checked the table in a spreadsheet.
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.
Yea my formula is incorrect.
InitialInflationRate * ((1 - DisinflationRate) ^ YearsSinceGenesis)
no longer works because this proposal modifies the inflation rate and disinflation rate for years > 1.5
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.
When I use the reference implementation in this CIP, I get different numbers from the ones in the table. See celestiaorg/celestia-app#4299
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.
Note: if we adopt current reference implementation then the observed inflation in year 1 will be a time-weighted average of the previous inflation (7.2%) and the adjusted inflation (4.82%).
For more context: this proposal will only impact block provisions for blocks produced on celestia-app v4. The v3 blocks in year 1 are currently emitting block provisions using the previous inflation (7.2%).
We should get clarity on the desired year 1 inflation:
- Original inflation (7.2%)
- Adjusted inflation (4.82%)
- Time-weighted average of the two. Assuming 6 months on v3 and 6 months on v4 then 6.01%
Options 1 and 2 imply more changes that aren't in the reference implementation.
hey @tac0turtle can you please provide clarity on @rootulp's comment above? |
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.
LGTM, ready for review?
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.
I spot checked the calculation inflation numbers in the updated schedule and they seem correct. See https://github.com/celestiaorg/celestia-app/pull/4299/files#diff-59d4062609e837d218c1ca5b58fc7cf22419c204c245e606f77502a6aff36381R90-R110
Note the image may need to be updated.
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Ismail Khoffi <[email protected]>
LGTM after image is updated |
Overview
This CIP is focused around dropping this years inflation by 33% and reducing the disinflation by 33%. We propose this change be applied in the next upgrade (v4)