Skip to content

Hotfix/propagate hotkey swap conviction#2736

Open
gztensor wants to merge 11 commits into
devnet-readyfrom
hotfix/propagate-hotkey-swap-conviction
Open

Hotfix/propagate hotkey swap conviction#2736
gztensor wants to merge 11 commits into
devnet-readyfrom
hotfix/propagate-hotkey-swap-conviction

Conversation

@gztensor

@gztensor gztensor commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Description

Propagates the hotkey-swap conviction lock hotfix from #2731 onto devnet-ready after the reverse lock index work landed.

The migration in pallets/subtensor/src/migrations/migrate_fix_subnet_hotkey_lock_swaps.rs now uses LockingColdkeys when repairing subnet-scoped hotkey swap lock state. It removes old reverse-index entries when taking source locks and inserts the new reverse-index entry when a lock is moved to the destination hotkey, keeping Lock and LockingColdkeys consistent after the migration.

The migration test in pallets/subtensor/src/tests/migration.rs was extended to seed and assert LockingColdkeys state for moved locks, discarded conflict locks, and chained hotkey-swap fixes. The runtime spec_version is bumped from 417 to 418 for the runtime migration.

Behavioral impact: affected stale subnet hotkey-swap lock records are repaired without falling back to a full Lock scan, and the reverse index remains usable for subsequent lock operations. Existing destination lock conflicts continue to preserve the destination lock and discard the old source lock as before.

Testing: targeted migration coverage was updated to validate the new reverse-index behavior.

@gztensor gztensor self-assigned this Jun 9, 2026
@gztensor gztensor added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jun 9, 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/subtensor/src/migrations/migrate_fix_subnet_hotkey_lock_swaps.rs Outdated
Comment thread pallets/subtensor/src/swap/swap_hotkey.rs
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

BASELINE scrutiny: author has write permission, substantial prior subtensor history, no trusted Gittensor allowlist match, and branch hotfix/propagate-hotkey-swap-conviction -> devnet-ready.

The PR does not modify .github/ai-review/*, .github/copilot-instructions.md, dependencies, or build scripts. The runtime migration now relies on LockingColdkeys only after the migration sequence populates that reverse index, removes stale source index entries, writes destination index entries for moved locks, and keeps saturating migration weight accounting for the indexed reads/removals and Lock::take operations.

Findings

No findings.

Prior-comment reconciliation

  • a8760b5d: addressed — The migration charges for the LockingColdkeys prefix walk, index removals, Lock::take reads, and successful old-lock removals before moving or discarding locks.

Conclusion

No malicious pattern or security vulnerability was found in the current diff. The prior migration weight-accounting concern remains addressed.


🔍 AI Review — Auditor (domain review)

VERDICT: 👍

Gittensor: UNKNOWN by trusted allowlists; author has write permission and substantial prior subtensor history, so calibrated as an established contributor.

The prior Auditor concern is addressed: the changed swap_hotkey guard now has the corresponding subnet-swap test expectation updated to NonAssociatedColdKey.

The migration changes are consistent with the reverse LockingColdkeys index and run after the index-population migration in on_runtime_upgrade. The PR body is substantive, spec_version is bumped to 418, and I did not apply any auto-fixes. git diff --check passed.

I attempted the two targeted tests touched by the PR, but this sandbox cannot write rustup temp files under /home/runner/.rustup, so cargo could not start.

Findings

No findings.

Prior-comment reconciliation

  • 2ac89255: addressed — The test expectation in pallets/subtensor/src/tests/swap_hotkey_with_subnet.rs now matches the new NonAssociatedColdKey guard behavior.

Conclusion

The migration and test updates look aligned with the intended hotfix, and the previously reported behavior/test mismatch has been corrected. No blocking domain issues found.


📜 Previous run (superseded)
Sev File Finding Status
MEDIUM pallets/subtensor/src/swap/swap_hotkey.rs:55 New guard changes the existing registered-destination error path ✅ Addressed
The test expectation in pallets/subtensor/src/tests/swap_hotkey_with_subnet.rs now matches the new NonAssociatedColdKey guard behavior.

@github-actions

github-actions Bot commented Jun 9, 2026

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/subtensor/src/migrations/migrate_fix_subnet_hotkey_lock_swaps.rs Outdated
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@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/subtensor/src/swap/swap_hotkey.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

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