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

CIP: reduce inflation #249

Merged
merged 12 commits into from
Feb 20, 2025
Merged

CIP: reduce inflation #249

merged 12 commits into from
Feb 20, 2025

Conversation

tac0turtle
Copy link
Contributor

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)

Copy link
Collaborator

@rootulp rootulp left a 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.
Copy link
Collaborator

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.
Copy link
Collaborator

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?

Copy link
Collaborator

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

Suggested change
* 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).

tac0turtle and others added 3 commits February 5, 2025 15:37
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
@liamsi

This comment was marked as resolved.

Co-authored-by: Rootul P <[email protected]>
@rootulp
Copy link
Collaborator

rootulp commented Feb 5, 2025

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.
Copy link
Collaborator

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

Suggested change
* 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
Comment on lines 56 to 59
| **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%). |
Copy link
Collaborator

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.

Copy link
Member

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.

Copy link
Collaborator

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

Copy link
Collaborator

@rootulp rootulp Feb 10, 2025

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

Copy link
Collaborator

@rootulp rootulp left a 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.

@jcstein
Copy link
Member

jcstein commented Feb 12, 2025

hey @tac0turtle can you please provide clarity on @rootulp's comment above?

Copy link
Member

@jcstein jcstein left a 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?

@tac0turtle tac0turtle marked this pull request as ready for review February 18, 2025 16:06
Copy link
Collaborator

@rootulp rootulp left a 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.

tac0turtle and others added 2 commits February 19, 2025 09:40
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Rootul P <[email protected]>
tac0turtle and others added 2 commits February 19, 2025 09:41
Co-authored-by: Rootul P <[email protected]>
Co-authored-by: Ismail Khoffi <[email protected]>
@jcstein
Copy link
Member

jcstein commented Feb 19, 2025

LGTM after image is updated

@jcstein jcstein merged commit eb359b6 into celestiaorg:main Feb 20, 2025
1 check passed
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.

5 participants