Skip to content

Purge neuron commitments when trimming uids - #3062

Open
UnArbosSix wants to merge 1 commit into
mainfrom
fix/purge-commitments-on-uid-trim
Open

Purge neuron commitments when trimming uids#3062
UnArbosSix wants to merge 1 commit into
mainfrom
fix/purge-commitments-on-uid-trim

Conversation

@UnArbosSix

@UnArbosSix UnArbosSix commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Purges commitment state for neurons deregistered when a subnet's max_uids is trimmed.

This introduces a purge_neuron commitment interface that removes:

  • Active and revealed commitments
  • Commitment metadata and usage tracking
  • Timelock index entries
  • Associated commitment deposits

Normal UID replacement behavior remains unchanged.

Testing

  • Added a regression test confirming the trimmed neuron's commitment is removed.
  • Confirmed commitments belonging to retained neurons remain intact.
  • Verified the runtime and affected packages compile successfully.

@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
subtensor Ready Ready Preview Aug 7, 2026 7:32pm

Request Review

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

eco-tests changed — indexer review required

This PR modifies files under eco-tests/. and may affect downstream indexing.
cc @evgeny-s — please review manually

Changed files
  • eco-tests/src/mock.rs

@github-actions
github-actions Bot requested a review from evgeny-s August 7, 2026 19:29

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.


// 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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🛡️ 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

Sev File Finding
HIGH pallets/subtensor/src/subnets/uids.rs:227 UID trimming now performs unaccounted unbounded commitment cleanup inline

Conclusion

The 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.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant