Purge neuron commitments when trimming uids - #3062
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
eco-tests changed — indexer review requiredThis PR modifies files under Changed files
|
|
|
||
| // Remove hotkey related storage items if hotkey exists | ||
| if let Ok(hotkey) = Keys::<T>::try_get(netuid, neuron_uid) { | ||
| T::CommitmentsInterface::purge_neuron(netuid, &hotkey); |
There was a problem hiding this comment.
[HIGH] UID trimming now performs unaccounted unbounded commitment cleanup
Each removed UID now performs five additional storage removals and mutates the global, unbounded TimelockedIndex, which requires decoding and re-encoding the entire set. This occurs repeatedly in the trimming loop, while sudo_trim_to_max_allowed_uids still declares a fixed weight accounting for only 8 reads and 1 write. Its benchmark initializes an empty subnet and therefore never exercises this path. Users can enlarge the global timelock index through commitment state, and a subnet owner can invoke trimming, making the extrinsic substantially exceed its declared execution and proof-size limits. Batch the index cleanup, bound/meter the work, and regenerate the benchmark and weights using the worst-case number of removed neurons and commitment state.
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE HIGH scrutiny: account is under 90 days old with no public repositories, but has repository write permission and substantive merged contributions; no Gittensor association was found. Branch targets main. Findings
ConclusionThe cleanup itself is appropriately scoped, but invoking it inside UID trimming introduces attacker-amplifiable, unaccounted runtime work. The inaccurate fixed weight creates a credible block-exhaustion risk. # 🔍 AI Review — Auditor (domain review) has not yet run on this PR. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
Summary
Purges commitment state for neurons deregistered when a subnet's
max_uidsis trimmed.This introduces a
purge_neuroncommitment interface that removes:Normal UID replacement behavior remains unchanged.
Testing