-
Notifications
You must be signed in to change notification settings - Fork 95
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
Tier thresholds derivation support #1376
Conversation
Visit the preview URL for this PR (updated for commit 24af84e): https://astar-apps--pr1376-fix-tier-threshold-d-h3ydrku2.web.app (expires Thu, 29 Aug 2024 09:27:21 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: dd76fe72958fe2910fef9d53f0b4539b82b849db |
@@ -172,13 +172,7 @@ export interface TiersConfiguration { | |||
readonly numberOfSlots: number; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps the numberOfSlots
field can be removed here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to leave it because it is used on the portal UI. The variable is now calculated as below
numberOfSlots: configuration.slotsPerTier.reduce((acc, val) => acc + val.toNumber()
@@ -127,13 +127,12 @@ export interface PalletDappStakingV3ContractStakeAmount extends Struct { | |||
} | |||
|
|||
export interface PalletDappStakingV3TiersConfiguration extends Struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export interface PalletDappStakingV3TiersConfiguration extends Struct { | |
export interface PalletDappStakingV3TiersConfigurationLegacy extends Struct { | |
export interface PalletDappStakingV3TiersConfiguration extends Struct { |
@bobo-k2 you need 2 different versions here and try to decode with new one and fallback to legacy one. Otherwise this will fail until we perform the migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested new version on local node and on Astar. Decoding works, but I will check how to improve
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bobo-k2 It will decode because you're telling the type but the values will be incorrect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Fixed in this commit
tierThresholds: configuration.tierThresholds.map((threshold) => { | ||
// Support both u128 and PalletDappStakingV3TierThreshold. | ||
// If threshold has isUnsigned property it's u128. | ||
// memo: remove isUnsigned check when u128 is used for all thresholds. Most likely in Astar period 003. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prefix should be Todo
🙂
* dApps loading error fix for Shibuya (#1377) * Tier thresholds derivation support (#1376) * Tier thresholds derivation support * Additional comments * Pallet version check * Fix: estimated realized inflation not showing (#1379) * Re-try fetch from cbridge (#1382) --------- Co-authored-by: Bobo <[email protected]>
Pull Request Summary
Support for dApp staking pallet changes explained in #1367
Runtime changes are not deployed on any network yet, but the code can be tested with local node after pulling the latest
main
branch. The code should work without errors on local node, Shibuya, Shiden, Astar.Tier threshold value should be visible on UI

Closes #1367
Check list