Make setting ck take extrinsics payable#2750
Conversation
gztensor
added
the
skip-cargo-audit
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| /// | ||
| #[pallet::call_index(65)] | ||
| #[pallet::weight((<T as crate::pallet::Config>::WeightInfo::decrease_take(), DispatchClass::Normal, Pays::No))] | ||
| #[pallet::weight((<T as crate::pallet::Config>::WeightInfo::decrease_take(), DispatchClass::Normal, Pays::Yes))] |
There was a problem hiding this comment.
[HIGH] Runtime fee semantics changed without a spec_version bump
This changes dispatch metadata for decrease_take from Pays::No to Pays::Yes, and the same PR also changes increase_take. That is a runtime-affecting semantic change, but the diff does not update runtime/src/lib.rs spec_version (currently 417). Shipping upgraded Wasm with the same spec version can leave nodes treating old native runtime code as compatible with the new on-chain runtime, which is especially risky here because old and new code disagree on whether these calls withdraw transaction fees. Bump spec_version in this PR.
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE BASELINE scrutiny: established contributor with repo write permission and substantial prior subtensor activity; no Gittensor allowlist match found; branch feat/ck-take-settings-pays-yes -> devnet-ready. Static-only review found no FindingsNo findings. Prior-comment reconciliation
ConclusionNo malicious pattern or security vulnerability was found in the current diff. The prior weight-accounting concern remains addressed. 🔍 AI Review — Auditor (domain review)VERDICT: 👍 Gittensor: UNKNOWN by allowlist; author has repo write permission and substantial recent subtensor contribution history, so calibrated as established. Description discrepancies: the PR body is substantive but stale. It says Overlapping PRs appear to share broad runtime/test files only; I did not identify a duplicate candidate for this exact fee-metadata fix. No auto-fixes were applied. FindingsNo findings. ConclusionApproving: the fee metadata change is directly covered, the prior swap-hotkey test concern no longer blocks, and I found no remaining domain issue that should block merge. The PR body should still be refreshed before merge so reviewers have an accurate change summary. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
🔄 AI review updated — Skeptic: VULNERABLE |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
…r/subtensor into feat/ck-take-settings-pays-yes
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
2 participants
Motivation
increase_takeanddecrease_takewere marked asPays::No, which means these state-mutating delegate take changes did not pay normal transaction fees. These calls should be payable like other user-submitted staking operations.What Changed
decrease_takedispatch metadata fromPays::NotoPays::Yes.increase_takedispatch metadata fromPays::NotoPays::Yes.DispatchClass::Normaland now reportPays::Yes.spec_versionfrom417to418.Behavioral Impact
Delegate take increase/decrease transactions now participate in normal fee payment. The existing origin checks, take bounds, ownership checks, rate limiting, and underlying staking logic are otherwise unchanged.
Migration / Spec Version
No storage migration is required. This is a runtime-affecting metadata change and includes the required
spec_versionbump.Testing
test_delegate_take_dispatch_info_pays_feefor the affected dispatch metadata../scripts/fix_rust.shand unit tests were run locally.