-
Notifications
You must be signed in to change notification settings - Fork 264
Fix SubnetAlphaOut recycling in recycle_alpha.rs (# Issue #2274: Root Cause & Fix Analysis) #2297
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
base: devnet-ready
Are you sure you want to change the base?
Conversation
|
One question: if the desynchronization already happened, should we add a migration to sum up alpha hold by each (cold, hot), then set the alpha out for each subnet accordingly. |
|
Yes, a migration should be added to correct any existing desynchronization. |
bdmason
left a comment
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 doesn't fix the problem, recycle alpha is already reducing SubnetAlphaOut inside Self::recycle_subnet_alpha so your change in do_recycle_alpha will create a double deduction and a new bug.
When burning a diff between TotalHotkeyAlpha and SubnetAlphaOut is supposed to be created, this is how burning works on Bittensor. The burnt tokens remain in issuance, but are no longer owned/accessible. Your change in do_burn_alpha is changing burning to recycling.
|
@bdmason check this new update The Correct Fix: Use Actual DecrementWe must ensure we only recycle what was actually removed from the user. File: pub(crate) fn do_recycle_alpha(
origin: T::RuntimeOrigin,
hotkey: T::AccountId,
amount: AlphaCurrency,
netuid: NetUid,
) -> DispatchResult {
// ... checks ...
// 1. Decrease User Stake
// capture the return value 'actual_alpha_decrease'
let actual_alpha_decrease = Self::decrease_stake_for_hotkey_and_coldkey_on_subnet(
&hotkey, &coldkey, netuid, amount,
);
// 2. Recycle ONLY the Actual Amount
// OLD (Buggy): Self::recycle_subnet_alpha(netuid, amount);
// NEW (Fixed): Use actual_alpha_decrease
Self::recycle_subnet_alpha(netuid, actual_alpha_decrease);
Self::deposit_event(Event::AlphaRecycled(
coldkey,
hotkey,
actual_alpha_decrease,
netuid,
));
Ok(())
}@open-junius please help review the new fix |
shall we also update the check before. ensure!( |
|
@open-junius please help me review again I just implemented a new fix CI Fix: Update Rust Toolchain for Cargo AuditProblemThe SolutionExplicitly updated the Rust toolchain version in the GitHub Actions workflow to ChangesFile: - name: Install Rust
uses: actions-rs/toolchain@v1
with:
- toolchain: stable
+ toolchain: 1.89.0 |
|
@Dairus01 this definitely isn't the solution. The latest change you suggested (to use This particular issue is incredibly hard to pin down. It is almost certainly somewhere in |
|
@bdmason this new fix would solve it, try it and see. @open-junius please can you review this |
It definitely wont. You are claiming that a cosmetic change to an extrinsic ( |
I have implemented a direct fix to this problem in my latest push, please check it out |
Fix: Correct TotalStake Accounting in Alpha RecyclingThis new update fixes an accounting issue where ProblemWhen alpha is recycled (burned/removed), the SolutionModified recycle_subnet_alpha in pallets/subtensor/src/staking/helpers.rs to explicitly decrement Changes
This change ensures that every time recycle_subnet_alpha is called (e.g., during alpha reduction events), the global stake tracker stays perfectly synchronized. @open-junius please can you help me review this |
|
@open-junius, please can I get a review of my code |
|
@open-junius any update on the review |
The Root Cause
The core accounting bug stems from a desynchronization between individual user ledgers and global subnet ledgers during specific "admin-like" operations (Burning and Recycling).
In the Subtensor architecture, two layers of accounting must always move in unison:
The Logic Gap
In
recycle_alpha.rs, the functionsdo_recycle_alphaanddo_burn_alphacorrectly called:This helper function successfully removed tokens from the User's Balance.
However, these functions failed to decrement the corresponding
SubnetAlphaOutglobal counter.The Consequence (The Bug)
Because the user's balance went down but the global subnet counter remained unchanged:
TotalIssuancewould drift away from the actual circulating supply.Sum(UserStakes) != SubnetAlphaOut.✅ The Fix
To fix this, we must manually enforce the double-entry accounting rule. Whenever we decrement a user's stake in these specific functions, we must immediately decrement the subnet's total.
1. Fix in
do_recycle_alphaFile:
pallets/subtensor/src/staking/recycle_alpha.rs2. Fix in
do_burn_alphaFile:
pallets/subtensor/src/staking/recycle_alpha.rs🧪 Verification
After applying this fix:
Sum(UserStakes) == SubnetAlphaOutis preservedFixes #2274