Skip to content

Enforce conviction#2800

Open
gztensor wants to merge 1 commit into
devnet-readyfrom
feat/enforce-conviction
Open

Enforce conviction#2800
gztensor wants to merge 1 commit into
devnet-readyfrom
feat/enforce-conviction

Conversation

@gztensor

Copy link
Copy Markdown
Contributor

Description

This PR enables conviction enforcement. If

  • a non-owner hotkey A has conviction higher than owner's hotkey,
  • total conviction on the subnet is >= 10% of SubnetAlphaOut, and
  • subnet is at least 1 year old,

the ownership will be changed to coldkey that is owner of hotkey A.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other (please describe):

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have run ./scripts/fix_rust.sh to ensure my code is formatted and linted correctly
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@gztensor gztensor self-assigned this Jun 26, 2026
@gztensor gztensor added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jun 26, 2026
@github-actions github-actions Bot added the hotfix This PR needs to be merged very quickly and will likely skip testing on devnet and testnet label Jun 26, 2026
@gztensor gztensor changed the base branch from main to devnet-ready June 26, 2026 08:45
@gztensor gztensor removed the hotfix This PR needs to be merged very quickly and will likely skip testing on devnet and testnet label Jun 26, 2026
@opentensor opentensor deleted a comment from github-actions Bot Jun 26, 2026

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

Comment on lines +414 to +415
// Change subnet owner based on conviction.
Self::change_subnet_owner_if_needed(netuid);

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] Re-enabled conviction scan can make on_initialize unbounded

run_coinbase is invoked from block_step() in on_initialize, which returns a fixed weight. This call reaches change_subnet_owner_if_needed(), which calls get_total_conviction() and subnet_king(); those iterate HotkeyLock::<T>::iter_prefix(netuid) and DecayingHotkeyLock::<T>::iter_prefix(netuid) and build a BTreeMap over all matching hotkeys. Those maps are not bounded in the fixed hook weight, and the PR now runs the scans on every successful subnet epoch. An attacker can grow lock aggregate entries and force epoch blocks to do storage work proportional to lock-map size, risking overweight/slow block execution. Keep this disabled until the ownership check is made bounded, amortized, or otherwise accurately accounted in block weight.

Suggested change
// Change subnet owner based on conviction.
Self::change_subnet_owner_if_needed(netuid);
// Reserved for potential future enhancements.
// Ownership update logic based on conviction is currently inactive by design.
// Self::change_subnet_owner_if_needed(netuid);

@github-actions

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: VULNERABLE

BASELINE scrutiny: established repo write contributor, author/committer match, no Gittensor allowlist hit; branch feat/enforce-conviction -> devnet-ready.

No prior Skeptic findings were present. The PR does not modify .github/ai-review/*, .github/copilot-instructions.md, or dependency files.

Findings

Sev File Finding
HIGH pallets/subtensor/src/coinbase/run_coinbase.rs:415 Re-enabled conviction scan can make on_initialize unbounded inline

Conclusion

The change re-enables a runtime hook that performs unbounded lock-map scans during coinbase/on-initialize processing, creating a steady-state block production DoS risk. This should stay disabled until conviction enforcement is implemented with bounded per-block work or separately accounted execution.


# 🔍 AI Review — Auditor (domain review) has not yet run on this PR.

@github-actions

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

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants