Conversation
| PendingEmission::<T>::remove(netuid); | ||
| PendingRootDivs::<T>::remove(netuid); | ||
| PendingAlphaSwapped::<T>::remove(netuid); | ||
| PendingRootAlphaDivs::<T>::remove(netuid); |
There was a problem hiding this comment.
PendingRootAlphaDivs::<T>::remove(netuid) is called twice: line 318 & 351
antlerminator
left a comment
There was a problem hiding this comment.
Pardon for the intrusion but I've heard this PR was close/ready to be merged yet after reading it I found a bunch of things that could be addressed 😬
| log::debug!("hotkey: {hotkey:?} tao_take: {tao_take:?}"); | ||
| let validator_stake = Self::increase_stake_for_hotkey_and_coldkey_on_subnet( | ||
| log::debug!("hotkey: {hotkey:?} alpha_take: {alpha_take:?}"); | ||
| let _validator_stake = Self::increase_stake_for_hotkey_and_coldkey_on_subnet( |
There was a problem hiding this comment.
| let _validator_stake = Self::increase_stake_for_hotkey_and_coldkey_on_subnet( | |
| Self::increase_stake_for_hotkey_and_coldkey_on_subnet( |
Is there a reason we keep the variable here?
| pub fn add_stake_adjust_root_claimed_for_hotkey_and_coldkey( | ||
| hotkey: &T::AccountId, | ||
| coldkey: &T::AccountId, | ||
| amount: u64, |
There was a problem hiding this comment.
Shouldn't this be AlphaCurrency, similarly to how it's done in remove_stake_adjust_root_claimed_for_hotkey_and_coldkey?
| #[pallet::storage] // --- MAP ( hot ) --> MAP(netuid ) --> claimable_dividends | Root claimable dividends. | ||
| pub type RootClaimable<T: Config> = StorageMap< | ||
| _, | ||
| Blake2_128Concat, | ||
| T::AccountId, | ||
| BTreeMap<NetUid, I96F32>, | ||
| ValueQuery, | ||
| DefaultRootClaimable<T>, | ||
| >; |
There was a problem hiding this comment.
Shouldn't this be renamed to RootClaimableRatio to reflect it better what this exactly is? This wording is already used elsewhere in the code
Thanks for the review. I addressed the majority of your comments. |
|
Per discussion on the Open Dev call and the overall initiative of improvement of bookkeeping in Bittensor, can we please implement proper bookkeeping from the start for root claim? Back when I was reviewing this PR I saw two possibilities:
Relevant discussion from CoR: https://discord.com/channels/1120750674595024897/1245487232928714863/1431026852008427572 |
dougsillars
left a comment
There was a problem hiding this comment.
it looks like root claim now charges fees to convert to tao?
| netuid, | ||
| owed_u64.into(), | ||
| T::SwapInterface::min_price::<TaoCurrency>(), | ||
| false, |
There was a problem hiding this comment.
so we are charging a swap fee on root stakeholders now?
There was a problem hiding this comment.
Adding fees has not been part of the public description of how this would function, afaik. Previous open discussions were that the default would function as it is now (just the option to continue to receive in TAO). The recent Discord announcement does say "If you do nothing, your claims use Swap", but this could be construed as the two options of root_claim_type and not the traditional swap with fees.
Is the intention for all root stakers to pay fees to receive dividends?
|
Hey @shamil-gadelshin it looks like (apologies if I've misunderstood) the following is correct:
If this is correct, there is likely to be an awful lot of dust that's effectively burned, but not actually burned as it's still technically accessible, though a user may have to earn more divs first to get over the threshold required to make a claim. Would it not be better to just claim any outstanding divs when the user unstakes? Whilst there would be more operations on the unstake call, there won't be a never ending accumulation of storage in |
We plan to update this feature after the release. |
|
@shamil-gadelshin The PR description is confusing, it says:
and
Neither of which are true. In fact, |
|
@shamil-gadelshin doing this after the release will mean that the off-chain bookkeeping is impossible between the release and that update... Basically the historical state will be "corrupted" so why not do it now? This is not a huge change in itself but it's impossible to do by non-core devs |
Root Claim
This PR introduces a root claim mechanism. It replaces the automatic selling of root-alpha dividends with accumulation. Users can either:
New extrinsics
set_root_claim_type— Set root-claim mode:autoormanual.claim_root— Manually claim accumulated root dividends.Storage changes
PendingRootAlphaDivs— Tracks root-alpha dividends between epochs.RootClaimType— User’s preferred claim mode (autoormanual).RootClaimable— User’s claimable share of alpha dividends.RootClaimed— Accounting helper for alpha dividends (stake changes, claimed amounts).StakingColdkeysByIndex— Supports randomized distribution of auto-claims per block.StakingColdkeys— Supports root-dividends distribution.NumStakingColdkeys— Count to support randomized auto-claim distribution per block.NumRootClaim— Number of auto-claims to fulfill for a subnet in the current epoch.AlphaMapLastKey— Supports staking coldkey migration.Removed (obsolete storage maps)
TaoDividendsPerSubnetPendingAlphaSwappedOriginal PRs