Skip to content

remove duplicate sudo_set_subnet_owner_hotkey#2777

Open
JohnReedV wants to merge 4 commits into
devnet-readyfrom
remove-duplicate-set-sn-owner-key
Open

remove duplicate sudo_set_subnet_owner_hotkey#2777
JohnReedV wants to merge 4 commits into
devnet-readyfrom
remove-duplicate-set-sn-owner-key

Conversation

@JohnReedV

@JohnReedV JohnReedV commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Description

Removes the duplicate sudo_set_subnet_owner_hotkey admin-utils extrinsic. The remaining sudo_set_sn_owner_hotkey extrinsic continues to dispatch to the same underlying subnet owner hotkey setter.

Changes

  • Removes the duplicate sudo_set_subnet_owner_hotkey dispatch path from pallets/admin-utils/src/lib.rs.
  • Leaves the existing sudo_set_sn_owner_hotkey benchmark and weight path in place.
  • Bumps runtime/src/lib.rs spec_version from 421 to 422 for the runtime metadata change.

Behavioral Impact

The duplicate call name is no longer available in runtime metadata. Callers should use sudo_set_sn_owner_hotkey; the underlying owner-hotkey update behavior is unchanged.

Testing

Not specified in the original PR body. Auditor review was static only.

@JohnReedV JohnReedV added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jun 22, 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 thread pallets/admin-utils/src/lib.rs
@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

BASELINE scrutiny: established opentensor-associated contributor with repo write permission and substantial prior merged PR history; branch remove-duplicate-set-sn-owner-key -> devnet-ready.

Static-only review found a narrow runtime diff: the duplicate sudo_set_subnet_owner_hotkey dispatch path is removed, the Owner proxy exception for that removed call is also removed, and the remaining sudo_set_sn_owner_hotkey path continues through the existing checked implementation. No .github, dependency, lockfile, or build-script changes are present.

Findings

No findings.

Prior-comment reconciliation

  • 9954136f: addressed — The runtime versioning concern was already addressed; the current branch is merged over devnet-ready where runtime/src/lib.rs has spec_version: 423.

Conclusion

I found no malicious indicators or security vulnerabilities in the current diff. The prior runtime-versioning concern remains addressed on the current base/head state.


🔍 AI Review — Auditor (domain review)

VERDICT: 👍

Gittensor: LIKELY by recent subtensor PR history; author has repo write access and substantial prior contribution history, so calibrated as an established contributor.

Description discrepancy: the PR body still says this bumps runtime/src/lib.rs spec_version from 421 to 422, but the current patch no longer changes spec_version; both the merge base and PR worktree show spec_version: 423 after the devnet-ready merge.

Duplicate-work check found overlapping open PRs on broad runtime/pallet files, but none are direct duplicates of this removal. No auto-fix was applied. I attempted the configured devnet runtime-version query for spec-version validation, but DNS resolution failed in this environment; review was otherwise static only.

Findings

Sev File Finding
LOW pallets/admin-utils/src/lib.rs:1545 Delete the commented-out extrinsic instead of leaving dead code inline
LOW runtime/tests/ghsa_repro.rs:145 Fix the truncated GHSA-2026-003 test comment inline

Prior-comment reconciliation

  • bb1b9df5: not addressed — The deleted extrinsic is still present as a commented-out block in pallets/admin-utils/src/lib.rs.

Conclusion

The runtime-facing alias removal is consistent with the proxy filter and the remaining call path. Approving with low-severity cleanup comments for stale commented code and test documentation.


📜 Previous run (superseded)
Sev File Finding Status
LOW pallets/admin-utils/src/lib.rs:1545 Delete the commented-out extrinsic instead of leaving dead code ➡️ Carried forward to current findings
The deleted extrinsic is still present as a commented-out block in pallets/admin-utils/src/lib.rs.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@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 thread pallets/admin-utils/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@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 thread pallets/admin-utils/src/lib.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

@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 thread pallets/admin-utils/src/lib.rs
Comment thread runtime/tests/ghsa_repro.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

Comment on lines +1535 to +1545
// Deprecated for sudo_set_sn_owner_hotkey
// #[pallet::call_index(64)]
// #[pallet::weight(Weight::from_parts(3_918_000, 0) // TODO: add benchmarks
// .saturating_add(T::DbWeight::get().writes(1_u64)))]
// pub fn sudo_set_subnet_owner_hotkey(
// origin: OriginFor<T>,
// netuid: NetUid,
// hotkey: <T as frame_system::Config>::AccountId,
// ) -> DispatchResult {
// pallet_subtensor::Pallet::<T>::do_set_sn_owner_hotkey(origin, netuid, &hotkey)
// }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we just remove the comment maybe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I left it so we don't re-use the call index.

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.

3 participants