Feat/dyn tier thresholds #1306
Merged
Feat/dyn tier thresholds #1306
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ipapandinas
added
enhancement
|
/runtime-upgrade-test runtime=shibuya |
|
Runtime upgrade test is scheduled at https://github.com/AstarNetwork/Astar/actions/runs/10108041768. |
Dinonard
reviewed
Jul 29, 2024
ermalkaleci
reviewed
Jul 29, 2024
ermalkaleci
reviewed
Aug 1, 2024
Dinonard
reviewed
Aug 5, 2024
Dinonard
reviewed
Aug 5, 2024
Dinonard
approved these changes
Aug 6, 2024
| @@ -1258,6 +1258,7 @@ pub type Executive = frame_executive::Executive< | |||
| >; | |||
|
|
|||
| parameter_types! { | |||
| // percentages below are calulated based on total issuance at the time when dApp staking v3 was launched (8.4B) | |||
There was a problem hiding this comment.
Might be worth considering adding more checks for this into the try-runtime check 🙂
For another PR though.
There was a problem hiding this comment.
Are you thinking of sanity checks in pre_upgrade, like:
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, TryRuntimeError> {
let tier_thresholds: Result<BoundedVec<TierThreshold, T::NumberOfTiers>, _> =
BoundedVec::try_from(TierThresholds::get().to_vec());
assert!(tier_thresholds.is_ok());
...
}There was a problem hiding this comment.
Both pre & post.
E.g. take current thresholds (active ones) in pre-upgrade, send them to post upgrade.
In post-upgrade, after you derive the new thresholds, ensure they are e.g. within 10% of the old ones.
Minimum allowed line rate is |
|
@ermalkaleci any more comments here? |
I'm reviewing now. The only concern is to double check percentage |
ermalkaleci
approved these changes
Aug 6, 2024
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Summary
New tier thresholds as percentage of the total issuance. Should be merged after v5.42.2 is deployed to Astar/Shiden.
Closes #1295
Types: TierThreshold enum updated
For
TierThresholdenum: previous DynamicTvlAmount & FixedTvlAmount variants were deleted and new ones are added instead: DynamicPercentage & FixedPercentage. They support Perbill percentages and are intended to be used in the same way as the previous ones for TVL amounts. Genesis configurations, mocks and tests were updated accordingly.Migrations
Two migrations are introduced:
number_of_slotsis removed andtier_thresholdsis translated from BoundedVec<TierThreshold, NT> to BoundedVec<Balance, NT>.tier_thresholdsis updated for each runtime with calculated percentage based on the the total issuance when dApp staking v3 was launched.try-runtimeresultsshibuya
Check list