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

dApp Staking v3 & Tokenomics 2.0 #990

Merged
merged 18 commits into from
Dec 20, 2023
Merged

dApp Staking v3 & Tokenomics 2.0 #990

merged 18 commits into from
Dec 20, 2023

Conversation

Dinonard
Copy link
Member

@Dinonard Dinonard commented Jul 31, 2023

dApp Staking v3 & Tokenomics 2.0

This PR's underlying branch aggregates all of the PRs via which dApp staking v3 & Tokenomics 2.0
functionality was implemented (with the exception of fee alignment).

For details about concrete steps of the implementation, please refer to the linked PRs.
Lots of documentation can be found in the code as rustdoc and in the dedicated README file.

Some TODOs remain, but this functionality is intended to be tested on Shibuya.
Any further modifications will be done to the master branch.

TODO

Architecture & Functionality

  • use BTreeMap instead of sorted vector for rewards?
  • Oracle task definition
  • pallet to handle cycles,eras,periods in order to align inflation & dApp staking v3

Documentation

  • decide about naming with the team

Inflation

  • re-run benchmarks on proper machine
  • remove forced config set before mainnet launch

Follow-up PRs

  • integrity test
  • storage migration logic
  • precompiles
  • storage version setting

PR List

@Dinonard Dinonard added the other PR/issue is related to third-party, scripts or other things which don't have definite label. label Jul 31, 2023
Dinonard and others added 2 commits August 30, 2023 17:16
* Phase2 progress

* Adapt for 0.9.43

* Abstract sparse vector type

* Further modifications

* claim unlocked functionality WIP

* claim unlocked tested

* Relock unlocking

* Check era when adding chunk

* Custom error for some types

* Additional type tests - WIP

* More type tests

* Review comments

* Additional changes
@Dinonard Dinonard mentioned this pull request Oct 10, 2023
10 tasks
Dinonard and others added 15 commits October 12, 2023 08:17
* dApp staking v3 - PR3

* SingularStakingInfo

* SingularStakingInfo tests

* Stake work

* Stake & lots of tests

* TODOs, renaming, improvements

* Refactoring & cleanup

* More fixes

* Rework series

* Series tests

* Minor adjustment

* stake test utils & first test

* Stake test

* stake extrinsic tests

* Era & Period change logic

* Update era/period transition in mock & tests

* on_init tests & refactoring

* Unstake & some minor improvements

* Lots of type tests for unstake

* More tests

* More types test

* Testing utils for unstake, some TODOs

* Unstake tests

* Minor adjustments

* Fixes

* Additional scenario

* Address review comments

* Correct unstake amount

* Tests
* EraRewardSpan

* Initial version of claim_staker_reward

* Tests

* Test utils for claim-staker

* Bug fixes, improvements

* Claim improvements & some tests

* Refactoring in progress

* Refactoring continued

* Refactoring progress

* Refactoring finished

* Bonus rewards

* Docs & some minor changes

* Comments, tests, improved coverage

* Tier params & config init solution

* Tier reward calculation WIP

* Tier assignemnt

* Minor cleanup

* Claim dapp rewards

* Claim dapp reward tests

* unstake from unregistered call

* Extra traits

* fixes

* Extra calls

* Refactoring

* More refactoring, improvements, TODO solving

* Local integration

* Genesis config

* Add forcing call

* try runtime build fix

* Minor changes

* Minor

* Formatting

* Benchmarks INIT

* Compiling benchmarks

* Fix

* dapp tier calculation benchmark

* Measured tier assignment

* Decending rewards in benchmarks

* Series refactoring & partial tests

* Comments, minor changes

* Tests, improvements

* More tests, some minor refactoring

* Formatting

* More benchmarks & experiments, refactoring

* Readme, docs

* Minor renaming, docs

* More docs

* More docs

* Minor addition

* Review comment fixes & changes

* Minor change

* Review comments

* Update frontier to make CI pass
* EraRewardSpan

* Initial version of claim_staker_reward

* Tests

* Test utils for claim-staker

* Bug fixes, improvements

* Claim improvements & some tests

* Refactoring in progress

* Refactoring continued

* Refactoring progress

* Refactoring finished

* Bonus rewards

* Docs & some minor changes

* Comments, tests, improved coverage

* Tier params & config init solution

* Tier reward calculation WIP

* Tier assignemnt

* Minor cleanup

* Claim dapp rewards

* Claim dapp reward tests

* unstake from unregistered call

* Extra traits

* fixes

* Extra calls

* Refactoring

* More refactoring, improvements, TODO solving

* Local integration

* Genesis config

* Add forcing call

* try runtime build fix

* Minor changes

* Minor

* Formatting

* Benchmarks INIT

* Compiling benchmarks

* Fix

* dapp tier calculation benchmark

* Measured tier assignment

* Decending rewards in benchmarks

* Series refactoring & partial tests

* Comments, minor changes

* Tests, improvements

* More tests, some minor refactoring

* Formatting

* More benchmarks & experiments, refactoring

* Readme, docs

* Minor renaming, docs

* More docs

* More docs

* Minor addition

* Review comment fixes & changes

* Minor change

* Review comments

* Update frontier to make CI pass

* dApp staking v3 - part 5

* Make check work

* Limit map size to improve benchmarks

* Fix for cleanup

* Minor fixes, more tests

* More fixes & test

* Minor changes

* More tests

* More tests, some clippy fixes

* Finished with type tests for now

* Log, remove some TODOs

* Changes

* Bug fix for unstake era, more tests

* Formatting, extra test

* Unregistered unstake, expired cleanup, test utils & tests

* Cleanup tests, minor fixes

* force tests

* Remove TODOs

* Tier assignment test WIP

* Finish dapp tier assignment test, fix reward calculation bug

* Some renaming, test utils for era change WIP

* Fix minor issues, propagaste renaming

* More checks

* Finish on_init tests

* Comments, docs, some benchmarks

* Benchmark progress

* More benchmarks

* Even more benchmarks

* All extrinsics benchmarked

* Expand tests

* Comment

* Missed error test

* Review comments
* Pallet inflation

* Hooks

* Functionality, mock & benchmarks

* Empty commit since it seems last push didn't work properly

* Tests, fixes

* More tests, minor fixes&changes

* Zero divison protection, frontier update

* More tests, minor changes

* Integration & renaming

* Genesis integration, more tests

* Final integration

* Cleanup deps

* Division with zero test

* Comments

* More minor changes

* Improve test coverage

* Integrate into pallet-timestamp

* Remove timestamp

* Fix

* review comments

* toml format
* dApp staking v3 part 6

* Minor refactor of benchmarks

* Weights integration

* Fix

* remove orig file

* Minor refactoring, more benchmark code

* Extract on_init logic

* Some renaming

* More benchmarks

* Full benchmarks integration

* Testing primitives

* staking primitives

* dev fix

* Integration part1

* Integration part2

* Reward payout integration

* Replace lock functionality with freeze

* Cleanup TODOs

* More negative tests

* Frozen balance test

* Zero div

* Docs for inflation

* Rename is_active & add some more docs

* More docs

* pallet docs

* text

* scripts

* More tests

* Test, docs

* Review comment
* dApp staking v3 part 6

* Minor refactor of benchmarks

* Weights integration

* Fix

* remove orig file

* Minor refactoring, more benchmark code

* Extract on_init logic

* Some renaming

* More benchmarks

* Full benchmarks integration

* Testing primitives

* staking primitives

* dev fix

* Integration part1

* Integration part2

* Reward payout integration

* Replace lock functionality with freeze

* Cleanup TODOs

* More negative tests

* Frozen balance test

* Zero div

* Docs for inflation

* Rename is_active & add some more docs

* More docs

* pallet docs

* text

* scripts

* More tests

* Test, docs

* Review comment

* Runtime API

* Changes

* Change dep

* Comment

* Formatting

* Expired entry cleanup

* Cleanup logic test

* Remove tier labels

* fix

* Improve README

* Improved cleanup

* Review comments

* Docs

* Formatting

* Typo
* Bonus claim reduced contract_stake_count

* Extra test
* dApp Staking Migration

* Events, benchmark prep

* benchmarks

* Benchmarks & mock

* weights

* limit

* Use extrinsic call

* Solved todos, docs, improvements

* Maintenance mode, on_runtime_upgrade logic

* Tests, refactoring

* Finish

* More tests

* Type cleanup

* try-runtime checks

* deps

* Improve docs

* Fixes & try-runtime testing modifications

* Fix test

* repeated

* Minor improvements

* Improvements

* taplo

* Minor rename
* Init commit

* All legacy calls covered

* v2 interface first definition

* Rename

* Resolve merge errors

* TODO

* v2 init implementation

* Prepared mock

* todo

* Migration to v2 utils

* More adjustments

* Finish adapting implementation to v2

* Mock & fixes

* Primitive smart contract

* Fix dsv3 test

* Prepare impl & mock

* Remove redundant code, adjust mock & prepare tests

* Tests & utils

* Test for legacy getters/view functions

* More legacy tests

* v1 interface covered with tests

* Minor refactor, organization, improvements

* v2 tests

* Cleanup TODOs

* More tests

* Updates

* docs

* Fixes

* Address review comments

* Adjustments

* Audit comments

* Fix mock

* FMT

* Review comments
* dApp staking v3 - Shibuya integration

* Init

* Fix build

* Fix find/replace doc mess

* Migration & disable

* More integration

* Progress

* Adjusted integration

* Finished integration

* Additional modifications & cleanup

* Move comment

* Fixes

* Shibuya integration tests fix & proxy

* Renaming

* Integration test fixes & legacy support

* Adjust for benchmarks

* Remove chain-extension, small updates

* fixes

* Partial weights

* Minor changes

* Benchmark fixes

* dApp staking weights

* Weights, deps

* Remove redundant storage item

* Inflation params, resolve TODOs

* Optimize lengthy benchmark

* Integration test

* Sort out more TODOs

* Benchmark optimization

* Fix seed

* Remove spec version bump

* Fix integration test

* Weights update
@Dinonard Dinonard marked this pull request as ready for review December 20, 2023 15:37
@Dinonard Dinonard changed the title dApp Staking v3 [WIP] dApp Staking v3 & Tokenomics 2.0 Dec 20, 2023
Copy link

Code Coverage

Package Line Rate Branch Rate Health
precompiles/sr25519/src 64% 0%
chain-extensions/types/assets/src 0% 0%
pallets/dapp-staking-v3/rpc/runtime-api/src 0% 0%
precompiles/unified-accounts/src 100% 0%
pallets/ethereum-checked/src 48% 0%
pallets/unified-accounts/src 84% 0%
pallets/inflation/src 70% 0%
precompiles/substrate-ecdsa/src 74% 0%
pallets/collator-selection/src 69% 0%
precompiles/dapp-staking-v3/src/test 0% 0%
chain-extensions/unified-accounts/src 0% 0%
primitives/src/xcm 66% 0%
pallets/dapp-staking-migration/src 39% 0%
pallets/dapps-staking/src/pallet 85% 0%
pallets/dynamic-evm-base-fee/src 81% 0%
primitives/src 66% 0%
pallets/dapp-staking-v3/src/test 0% 0%
precompiles/xvm/src 74% 0%
pallets/xc-asset-config/src 53% 0%
pallets/dapps-staking/src 81% 0%
precompiles/assets-erc20/src 81% 0%
precompiles/dapp-staking-v3/src 90% 0%
chain-extensions/types/xvm/src 0% 0%
precompiles/xcm/src 72% 0%
pallets/block-rewards-hybrid/src 87% 0%
pallets/xvm/src 40% 0%
pallets/dapp-staking-v3/src/benchmarking 0% 0%
chain-extensions/pallet-assets/src 0% 0%
precompiles/dapps-staking/src 94% 0%
pallets/dapp-staking-v3/src 79% 0%
chain-extensions/types/unified-accounts/src 0% 0%
chain-extensions/xvm/src 0% 0%
Summary 68% (3141 / 4632) 0% (0 / 0)

Minimum allowed line rate is 50%

@Dinonard Dinonard merged commit 9536303 into master Dec 20, 2023
14 checks passed
@Dinonard Dinonard deleted the feat/dapp-staking-v3 branch December 20, 2023 16:09
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.

1st area looks good. Just some small consistency findings.

Comment on lines +717 to +718
let mut dapp_info =
IntegratedDApps::<T>::get(&smart_contract).ok_or(Error::<T>::ContractNotFound)?;
Copy link
Member

Choose a reason for hiding this comment

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

I noticed in stake and unstake it returns NotOperatedDApp error instead. Maybe worth to unify them?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer to keep them since it tells about different error types.
At least to me it's useful 🙂

IntegratedDApps::<T>::get(&smart_contract).ok_or(Error::<T>::ContractNotFound)?;

ensure!(
dapp_info.state == DAppState::Registered,
Copy link
Member

Choose a reason for hiding this comment

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

This could be replaced with dapp_info.is_registered().

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch!


ensure!(dapp_info.owner == dev_account, Error::<T>::OriginNotOwner);

dapp_info.reward_destination = beneficiary.clone();
Copy link
Member

Choose a reason for hiding this comment

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

Maybe rename reward_destination to reward_beneficiary for consistency? The DAppRewardDestinationUpdated error uses Destination as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good catch!

Copy link
Member

@PierreOssun PierreOssun left a comment

Choose a reason for hiding this comment

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

Review finished of 4th area: Just some suggestions.
There is also a lot of typo in com lines but I guess we can skip those

//!
//! ## Rewards
//!
//! ### Staker & Treasury Rewards
Copy link
Member

Choose a reason for hiding this comment

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

//! ### Collators & Treasury Rewards

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch!

// 3.0 Convert all 'per cycle' values to the correct type (Balance).
// Also include a safety check that none of the values is zero since this would cause a division by zero.
// The configuration & integration tests must ensure this never happens, so the following code is just an additional safety measure.
let blocks_per_cycle = match T::CycleConfiguration::blocks_per_cycle() {
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 this ensure the safety check of values not being zero:

let blocks_per_cycle = Balance::from(T::CycleConfiguration::blocks_per_cycle().max(1));
let build_and_earn_eras_per_cycle =
    Balance::from(T::CycleConfiguration::build_and_earn_eras_per_cycle().max(1));
let periods_per_cycle =
    Balance::from(T::CycleConfiguration::periods_per_cycle().max(1));

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks much better, thanks!

));

let new_config = ActiveInflationConfig::<Test>::get();
assert!(
Copy link
Member

Choose a reason for hiding this comment

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

    assert_ne!(
        old_config, new_config,
        "Config should change, otherwise test doesn't make sense."
    );

Copy link
Member Author

Choose a reason for hiding this comment

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

Tbh, I didn't even know this existed 😂

pallets/inflation/src/tests.rs Show resolved Hide resolved
log::error!("Issuance cap has been exceeded. Please report this issue ASAP!");
}

// Allow for 1% safety cap overflow, to prevent bad UX for users in case of rounding errors.
Copy link
Member

Choose a reason for hiding this comment

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

We should create a follow-up ticket/issue to ensure this.

Copy link
Member Author

Choose a reason for hiding this comment

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

The follow-up is to remove this completely 🙂

This is just an initial safety measure to prevent accidental excessive minting.

counter.saturating_inc();

// Skip dApps which don't have ANY amount staked
let stake_amount = match stake_amount.get(era, period) {
Copy link
Member

Choose a reason for hiding this comment

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

I found this more readable:

                if let Some(stake_amount) = stake_amount.get(era, period) {
                    if !stake_amount.total().is_zero() {
                        dapp_stakes.push((dapp_id, stake_amount.total()));
                    }
                }

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, applied your suggestion 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
other PR/issue is related to third-party, scripts or other things which don't have definite label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants