Add min_childkey_take_per_subnet hyperparameter#2734
Conversation
gztensor
added
the
skip-cargo-audit
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE BASELINE scrutiny: write-permission author with substantive contribution history; no Gittensor allowlist hit found; branch feat/min-ck-take-hyperparam -> devnet-ready. Static review found a narrow runtime/pallet diff: a read-only FindingsNo findings. ConclusionNo malicious pattern or security vulnerability was found. The changes do not introduce a runtime panic path, origin bypass, dependency surface, or build-time execution surface. 🔍 AI Review — Auditor (domain review)VERDICT: 👍 Gittensor: LIKELY; author is not in the trusted allowlists but has write permission and substantial recent subtensor history. Description discrepancies: the PR body describes adding Auto-fix: none. The prior weight-accounting concern is addressed in the current diff: the new FindingsNo findings. Prior-comment reconciliation
ConclusionThe added hyperparameter surface is narrow and covered by the V3 hyperparameter tests. I did not find a remaining blocking domain issue in the current diff. 📜 Previous run (superseded)
|
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
hotkey-swap check `new_hotkey`
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // 3. If the new hotkey already exists globally, ensure the coldkey owns it | ||
| if Self::hotkey_account_exists(new_hotkey) { | ||
| ensure!( | ||
| Self::coldkey_owns_hotkey(&coldkey, new_hotkey), | ||
| Error::<T>::NonAssociatedColdKey | ||
| ); | ||
| } | ||
|
|
||
| // 4. Initialize the weight for this operation | ||
| let mut weight = T::DbWeight::get().reads(2); |
There was a problem hiding this comment.
[HIGH] Account for the new hotkey-owner reads in swap weights
This new check performs extra Owner storage reads before weight is initialized: hotkey_account_exists(new_hotkey) reads once, and when it exists coldkey_owns_hotkey reads again via contains_key plus get. Those reads are not included in the returned post-dispatch weight, and the pre-dispatch weights for swap_hotkey / swap_hotkey_v2 were not updated or rebenchmarked. Since this is a runtime extrinsic, changed storage access must be reflected in weight. Initialize/accrue the dynamic weight before this check or collapse it into a single charged storage access, then update the generated swap_hotkey weight and the hardcoded swap_hotkey_v2 weight.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
| // 3. If the new hotkey already exists globally, ensure the coldkey owns it | ||
| if Self::hotkey_account_exists(new_hotkey) { | ||
| ensure!( | ||
| Self::coldkey_owns_hotkey(&coldkey, new_hotkey), | ||
| Error::<T>::NonAssociatedColdKey | ||
| ); | ||
| } | ||
|
|
||
| // 4. Initialize the weight for this operation | ||
| let mut weight = T::DbWeight::get().reads(2); |
There was a problem hiding this comment.
[HIGH] Account for the new hotkey-owner reads in swap weights
This new ownership gate performs additional Owner storage reads before weight is initialized. On the successful path where new_hotkey already exists and is owned by the caller, hotkey_account_exists(new_hotkey) reads once and coldkey_owns_hotkey(&coldkey, new_hotkey) reads again via contains_key plus get, but the returned post-dispatch weight still starts at only the two reads for the old-hotkey ownership check. That under-reports the actual runtime work and can over-refund swap_hotkey / swap_hotkey_v2.
| // 3. If the new hotkey already exists globally, ensure the coldkey owns it | |
| if Self::hotkey_account_exists(new_hotkey) { | |
| ensure!( | |
| Self::coldkey_owns_hotkey(&coldkey, new_hotkey), | |
| Error::<T>::NonAssociatedColdKey | |
| ); | |
| } | |
| // 4. Initialize the weight for this operation | |
| let mut weight = T::DbWeight::get().reads(2); | |
| // 3. Initialize the weight for ownership checks | |
| let mut weight = T::DbWeight::get().reads(2); | |
| // 4. If the new hotkey already exists globally, ensure the coldkey owns it | |
| if Self::hotkey_account_exists(new_hotkey) { | |
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | |
| ensure!( | |
| Self::coldkey_owns_hotkey(&coldkey, new_hotkey), | |
| Error::<T>::NonAssociatedColdKey | |
| ); | |
| weight.saturating_accrue(T::DbWeight::get().reads(2)); | |
| } else { | |
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | |
| } |
|
🔄 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: SAFE Auditor: 👍 |
3 participants
Description
Add a new hyperparameter
min_childkey_take_per_subnetfor the existing admin util extrinsic with matching name.Type of Change
Breaking Change
Introduces a new
min_childkey_take_per_subnetfield inget_subnet_hyperparams_v3.Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctly