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

feat!: implement inflating tail emission #6160

Merged
merged 2 commits into from Feb 28, 2024

Conversation

CjS77
Copy link
Collaborator

@CjS77 CjS77 commented Feb 20, 2024

Summary

Adds a feature tari_feature_mainnet_emission to allow for tail emission
inflation. See #6122

This change necessitates the addition of 2 new consensus constants:
inflation_bips -- the annual inflation rate of the total supply.
and tail_emission_epoch_length, which controls the tail emission
inflation. These replace tail_emission.

We update the Protobuf definition for ConsensusConstants to account for
the new fields.

Note: Replaces part of #6131

@CjS77 CjS77 requested a review from a team as a code owner February 20, 2024 19:36
@ghpbot-tari-project ghpbot-tari-project added CR-insufficient_context Your PRs commit messages don't provide enough context to justify accepting the change. P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Feb 20, 2024
@CjS77 CjS77 changed the title Refactor emission schedule feat!: implement inflating tail emission Feb 20, 2024
@CjS77 CjS77 added A-base_node Area - The Tari base node executable and libraries W-consensus_breaking Warn - A change requiring a hard fork to be activated W-breaking A non-backward compatible change and removed CR-insufficient_context Your PRs commit messages don't provide enough context to justify accepting the change. labels Feb 20, 2024
Copy link

github-actions bot commented Feb 20, 2024

Test Results (CI)

1 259 tests   1 259 ✅  11m 53s ⏱️
   39 suites      0 💤
    1 files        0 ❌

Results for commit ec1245e.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Feb 20, 2024

Test Results (Integration tests)

 2 files  + 2  11 suites  +11   14m 43s ⏱️ + 14m 43s
29 tests +29  28 ✅ +28  0 💤 ±0  1 ❌ +1 
30 runs  +30  29 ✅ +29  0 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit ec1245e. ± Comparison against base commit f5860a8.

♻️ This comment has been updated with latest results.

SWvheerden
SWvheerden previously approved these changes Feb 21, 2024
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code reads and follows so much easier without the feature flag in here.

@@ -119,7 +119,7 @@ message ConsensusConstants {
uint64 median_timestamp_count = 9;
uint64 emission_initial = 10;
repeated uint64 emission_decay = 11;
uint64 emission_tail = 12;
uint64 emission_tail = 12 [deprecated=true];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit:
I think we can just delete this, it should default to 0 if you ask for it in a dated proto file.
But I am 99% nothing uses this field

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's nuke it in a separate PR so that it's easy to revert if you're wrong.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Feb 21, 2024
@CjS77 CjS77 force-pushed the Refactor-emission-schedule branch 2 times, most recently from 9de36db to bf98053 Compare February 21, 2024 09:37
@ghpbot-tari-project ghpbot-tari-project added the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Feb 21, 2024
Adds a feature `tari_feature_mainnet_emission` to allow for tail emission
inflation. See #6122

This change necessitates the addition of 2 new consensus constants:
`inflation_bips` -- the annual inflation rate of the total supply in
basis points (100 bips = 1 percentage point).
and `tail_emission_epoch_length`, which controls the tail emission
inflation.

These replace `tail_emission`.

We update the Protobuf definition for `ConsensusConstants` to account for
the new fields.

Updates tests

Adds `as_u128` to `MicroMinotari` since we need to perform a large
multiplication when calculating the inflation issuance.

Note: Replaces part of #6131
@CjS77 CjS77 mentioned this pull request Feb 21, 2024
@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Feb 28, 2024
@SWvheerden SWvheerden merged commit 63b1f68 into development Feb 28, 2024
12 of 14 checks passed
@hansieodendaal hansieodendaal deleted the Refactor-emission-schedule branch March 4, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-base_node Area - The Tari base node executable and libraries P-acks_required Process - Requires more ACKs or utACKs W-breaking A non-backward compatible change W-consensus_breaking Warn - A change requiring a hard fork to be activated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants