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

Hybrid Inflation Model #1077

Merged
merged 15 commits into from
Nov 17, 2023
Merged

Hybrid Inflation Model #1077

merged 15 commits into from
Nov 17, 2023

Conversation

PierreOssun
Copy link
Member

@PierreOssun PierreOssun commented Nov 2, 2023

Pull Request Summary

closes #1049

This PR adds a new pallet block_rewards_hybrid that is forked from pallet-block-reward.
Changes:

  • It first calculates the rewards then increase total issuance and then distribute the reward. It's closer to solution 2. expressed in [Tokenomics][Meta] Tokenomics v2 #1023 . I chose this over the first solution because it avoids to issue as well as burn total issuance in on_timestamp_set hook and it looks not idiomatic.
  • the reward of the treasury is now fix
  • stakers receive the dynamic part in exactly same way (untouched).
  • Added tests to ensure it works under different TVL scenario & that total issuance is only increased by rewarded amount

Before
A fixed amount of BLOCK_REWARD was distributed (to stakers, dapps, collators, and treasury) so inflation was fixed. The TVL (fluctuating) was distributing the reward_config.adjustable_percent rewards between stakers and treasury.

Now
The TVL (fluctuating) is not distributing the reward_config.adjustable_percent rewards between stakers and treasury anymore. instead the proportion is only distributed to the stakers (and is calculated the same way). The remaining proportion that was previously reward to treasury is now just omitted (not minted), that's why the rewards per block/inflation is now dynamic.

To help review
The main update and code logic updated from previous pallet is in on_timestamp_set and the two new functions calculate_rewards and distribute_rewards (HERE in master).

Check list

  • added or updated unit tests
  • updated Astar official documentation
  • added OnRuntimeUpgrade
  • updated spec version
  • updated semver

@PierreOssun PierreOssun added the runtime This PR/Issue is related to the topic “runtime”. label Nov 2, 2023
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

Nice, great work!

bin/collator/src/local/chain_spec.rs Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/src/benchmarking.rs Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/src/lib.rs Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/src/tests.rs Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/src/tests.rs Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/src/tests.rs Outdated Show resolved Hide resolved
runtime/shibuya/src/lib.rs Show resolved Hide resolved
@PierreOssun PierreOssun marked this pull request as draft November 6, 2023 07:26
@PierreOssun PierreOssun marked this pull request as ready for review November 10, 2023 15:16
bin/collator/Cargo.toml Outdated Show resolved Hide resolved
runtime/shibuya/src/lib.rs Show resolved Hide resolved
runtime/local/src/lib.rs Show resolved Hide resolved
pallets/block-rewards-hybrid/src/lib.rs Show resolved Hide resolved
pallets/block-rewards-hybrid/src/lib.rs Show resolved Hide resolved
pallets/block-rewards-hybrid/src/tests.rs Outdated Show resolved Hide resolved
Dinonard
Dinonard previously approved these changes Nov 16, 2023
Copy link
Member

@Dinonard Dinonard left a comment

Choose a reason for hiding this comment

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

LGTM

pallets/block-rewards-hybrid/Cargo.toml Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/Cargo.toml Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/src/lib.rs Show resolved Hide resolved
pallets/block-rewards-hybrid/src/lib.rs Show resolved Hide resolved
pallets/block-rewards-hybrid/src/lib.rs Outdated Show resolved Hide resolved
pallets/block-rewards-hybrid/src/lib.rs Outdated Show resolved Hide resolved
runtime/shibuya/src/lib.rs Outdated Show resolved Hide resolved
@PierreOssun
Copy link
Member Author

/bench shibuya-dev pallet_block_rewards_hybrid

Copy link

Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/6889749488.
Please wait for a while.
Branch: feat/hybrid-inflation
SHA: 6b22734

Copy link

Benchmarks have been finished.
You can download artifacts if exists https://github.com/AstarNetwork/Astar/actions/runs/6889749488.

Copy link
Member

@shaunxw shaunxw left a comment

Choose a reason for hiding this comment

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

Looks good in general!

pallets/block-rewards-hybrid/src/tests.rs Outdated Show resolved Hide resolved
@PierreOssun
Copy link
Member Author

/bench shibuya-dev block_rewards_hybrid

Copy link

Benchmarks job is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/6892810709.
Please wait for a while.
Branch: feat/hybrid-inflation
SHA: 0077580

Copy link

Benchmarks have been finished.
You can download artifacts if exists https://github.com/AstarNetwork/Astar/actions/runs/6892810709.

Copy link

Code Coverage

Package Line Rate Branch Rate Health
precompiles/utils/macro/src 0% 0%
pallets/unified-accounts/src 83% 0%
chain-extensions/pallet-assets/src 0% 0%
precompiles/dapps-staking/src 93% 0%
pallets/block-reward/src 85% 0%
primitives/src/xcm 66% 0%
chain-extensions/types/unified-accounts/src 0% 0%
pallets/dapps-staking/src 81% 0%
chain-extensions/unified-accounts/src 0% 0%
pallets/dapps-staking/src/pallet 85% 0%
primitives/src 65% 0%
pallets/contracts-migration/src 0% 0%
precompiles/utils/src 55% 0%
pallets/xc-asset-config/src 53% 0%
chain-extensions/types/xvm/src 0% 0%
pallets/collator-selection/src 69% 0%
chain-extensions/dapps-staking/src 0% 0%
pallets/xvm/src 40% 0%
pallets/block-rewards-hybrid/src 87% 0%
pallets/dynamic-evm-base-fee/src 81% 0%
pallets/ethereum-checked/src 48% 0%
chain-extensions/xvm/src 0% 0%
precompiles/xvm/src 75% 0%
precompiles/xcm/src 75% 0%
precompiles/utils/src/testing 38% 0%
precompiles/assets-erc20/src 77% 0%
chain-extensions/types/dapps-staking/src 0% 0%
chain-extensions/types/assets/src 0% 0%
precompiles/sr25519/src 79% 0%
precompiles/substrate-ecdsa/src 78% 0%
Summary 57% (2275 / 4014) 0% (0 / 0)

Minimum allowed line rate is 50%

@Dinonard Dinonard merged commit 298a8e4 into master Nov 17, 2023
8 checks passed
@Dinonard Dinonard deleted the feat/hybrid-inflation branch November 17, 2023 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hybrid Inflation Model
3 participants