Feat/recycle burn chain exts#2560
Conversation
|
Thanks for the contribution. Can you add the ticket for the requirement? I am wondering the extension like AddStakeRecycleV1, who really want to do it. just confirm all is from the real requirements. |
Just got it ticketed up here - #2564 Particular snippet from the issue to elaborate on the AddStakeBurn + AddStakeRecycle funcs and why those 2 are included:
|
|
Looks good to me, please fix the 'Cargo fmt' |
👍 |
Strip development log::info!/log::error! calls from dispatch entry and AddStakeRecycleV1 handler. Normalize AddStakeRecycleV1 to use the same concise ? pattern as all other handlers.
e527c74 to
1c4f180
Compare
|
it also needed |
|
Addressed the review feedback:
@evgeny-s / @open-junius if you could give this a re-look when you have a chance, thanks Commit of changes: 65357d3 |
|
|
||
| match call_result { | ||
| Ok(_) => { | ||
| env.write_output(&amount.encode()) |
There was a problem hiding this comment.
I can see you remove the real_amount, and put the amount into result directly. but it could be wrong if the amount is more than you staked. So I suggest we can update the recycle_alpha function with the real recycled alpha as result, or doesn't return any value in RecycleAlphaV1. and the same for burn function.
There was a problem hiding this comment.
Good call, I got it updated w this commit - 60dffb3
Trying to keep the diff concise with what i add/change. Reran unit tests and passing
Replaces hardcoded Weight::from_parts values with calls to pallet_subtensor::weights::WeightInfo, matching the pattern junius introduced in PR 2550. Weights now auto-track benchmark updates. AddStakeRecycleV1 sums add_stake() + recycle_alpha() since the pallet has no add_stake_recycle weight; AddStakeBurnV1 uses add_stake_burn() directly (1:1 with pallet's do_add_stake_burn).
|
|
||
| let caller = env.caller(); | ||
|
|
||
| let result = transactional::with_transaction(|| { |
There was a problem hiding this comment.
There is do_add_stake_burn function, can we use it instead of call both do_add_stake and do_burn_alpha
There was a problem hiding this comment.
Looked into this but I don't think do_add_stake_burn is usable from a chain ext
- It's
pub(crate), so it's not accessible from thechain-extensionscrate. - Also it calls
ensure_subnet_owneron the origin which we won't want
Because of this, I kept it as the do_add_stake + do_burn_alpha composition
| <<T as pallet_subtensor::Config>::WeightInfo as SubtensorWeightInfo>::recycle_alpha(), | ||
| ); | ||
|
|
||
| env.charge_weight(weight)?; |
There was a problem hiding this comment.
we can charge the add_stake part first. If do_add_stake is Ok, then continue with recycle_alpha weight
There was a problem hiding this comment.
Okay, I updated this.. AddStakeRecycleV1 now charges add_stake() upfront and only charges recycle_alpha() after do_add_stake returns Ok. Applied the same split to AddStakeBurnV1 for consistency. Both ops still run inside with_transaction, & tests updated and passing.
For AddStakeRecycleV1 and AddStakeBurnV1, charge add_stake() upfront and only charge the second-stage weight (recycle_alpha()/burn_alpha()) after do_add_stake returns Ok. Atomicity is preserved by keeping both ops inside with_transaction and tracking attempt state via a stack flag.
- cargo fmt wraps the add_stake() call in AddStakeRecycleV1/AddStakeBurnV1 - mock charge_weight replaces saturating_add with checked_add().unwrap() to satisfy the ForbidSaturatingMath custom lint
evgeny-s
left a comment
There was a problem hiding this comment.
I don't like duplicating the composition logic - AddStakeBurnV1 re-implements do_add_stake_burn inline, and AddStakeRecycleV1 introduces a brand-new atomic primitive that doesn't exist as a pallet function at all. Ideally, it would be great to have the functions in the pallet itself and wire them here, something like do_add_stake_burn_permissionless. I still not sure, why we need a guard for the subnet owner, if we can call both functions separately, but this question out of scope for this PR.
@shamil-gadelshin - could you weigh in?
Per your subnet_buyback PR (#2381), the owner-check there is a privilege gate (priority boost + canonical event + per-tempo cadence), not a security control on the underlying effect.
Could you confirm:
- That reading is correct, and the underlying atomic
add_stake + burn_alphacomposition is safe to expose permissionlessly via chain extension? - Are we ok bypassing the rate limit via calling 2 extrinsics separately?
|
|
||
| env.charge_weight(weight)?; | ||
|
|
||
| let (hotkey, amount, netuid): (T::AccountId, AlphaBalance, NetUid) = |
There was a problem hiding this comment.
Can we make arguments order consistent with AddStakeRecycleV1?
The composition function is needed for the priority boost for subnet owners, so it makes sense. The architectural point about extracting |
Yes, otherwise it could be MEVed. |

Description
Add 4 new chain extension functions (IDs 16-19) enabling WASM smart contracts to recycle and burn alpha stake.
RecycleAlphaV1(16): Recycle alpha stakeBurnAlphaV1(17): Burn alpha stakeAddStakeRecycleV1(18): Atomically add TAO stake then recycleAddStakeBurnV1(19): Atomically add TAO stake then burnAll functions return the actual alpha amount via the output buffer.
Related Issue(s)
Closes #2564
Type of Change
Breaking Change
No breaking changes. Existing chain extension functions (IDs 0-15) are unaffected. New function IDs 16-19 are additive.
Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctlyAdditional Notes
7 unit tests cover: success paths for all 4 functions, root subnet rejection, nonexistent subnet, and insufficient balance. E2E validated on localnet with an ink! contract calling
add_stake_recyclevia chain extension.I also ran some local E2E testing with a local running subtensor chain (with the new extensions), deployed contract, etc.
Screenshot 1 shows:
Screenshot 2 shows:
Unit tests successfully passing:
