Skip to content

[Feature] Configurable Tempo & Owner-Triggered Epochs#2638

Open
evgeny-s wants to merge 39 commits into
devnet-readyfrom
feature/dynamic-tempo
Open

[Feature] Configurable Tempo & Owner-Triggered Epochs#2638
evgeny-s wants to merge 39 commits into
devnet-readyfrom
feature/dynamic-tempo

Conversation

@evgeny-s
Copy link
Copy Markdown
Collaborator

@evgeny-s evgeny-s commented May 6, 2026

Description

Implementation of the specification.

Demo (non-technical): https://dynamic-tempo-non-technical.netlify.app/
Demo (technical): https://dynamic-tempo-technical.netlify.app/#1

Related Issue(s)

  • Closes #[issue number]

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):

Breaking Change

Check the 11. Breaking changes & compatibility section.

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

Screenshots (if applicable)

Please include any relevant screenshots or GIFs that demonstrate the changes made.

Additional Notes

Please provide any additional information or context that may be helpful for reviewers.

Simulations (miner rewards Events):

Trigger epoch:
image
Set tempo:
image

@evgeny-s evgeny-s added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label May 6, 2026
@evgeny-s evgeny-s changed the title Dynamic tempo implementation [Feature] Configurable Tempo & Owner-Triggered Epochs May 6, 2026
Self::blocks_until_next_epoch(netuid, Self::get_tempo(netuid), current_block) == 0
let tempo = Self::get_tempo(netuid);
if tempo == 0 {
return false;
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.

This means that if tempo is set to 0, manual trigger or max cap of MAX_TEMPO will not work. Is this by design?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the previous implementation meant that if tempo is 0, we don't run tempo on this subnet.
We don't have this case in Mainnet, but we support it. There is still a possibility to set the tempo to 0 by the root.
So we support the same semantics.

/// stateful scheduler (period `tempo + 1`, anchored on `LastEpochBlock`). Used by the
/// admin-freeze-window predicate and external tooling. Returns `u64::MAX` when
/// `tempo == 0` (legacy defensive short-circuit).
pub fn blocks_until_next_epoch(netuid: NetUid, tempo: u16, block_number: u64) -> u64 {
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.

"Period is tempo + 1: next firing at last + tempo + 1." comment and the code below is not anymore correct.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated the function name and the comment to avoid the confusion: ad7ba80


let now = Self::get_current_block_as_u64();

Tempo::<T>::insert(netuid, tempo);
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.

Should this be a function since Tempo and LastEpochBlock are always updated together?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added a helper function: df184e3

@evgeny-s
Copy link
Copy Markdown
Collaborator Author

Hey, @basfroman , we have a bunch of tests that failed on the SDK side.
The main reason is now we have a stateful calculation for the next epoch block, because the tempo is dynamic.
So, here you can just call the new runtime API - get_next_epoch_start_block.


/// Effective activity cutoff in blocks, derived from `ActivityCutoffFactorMilli` and `Tempo`.
/// `cutoff_blocks = (factor × tempo) / 1000`, clamped to ≥ 1.
pub fn get_activity_cutoff_blocks(netuid: NetUid) -> u64 {
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.

should we use it to replace get_activity_cutoff everywhere? for instance in metagraph.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, because ActivityCutoff is deprecated and will be removed in the follow-up release.
Who can help with updating it?

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.

It is fine to remove the storage later. Actually, my question is for https://github.com/opentensor/subtensor/blob/devnet-ready/pallets/subtensor/src/rpc_info/metagraph.rs#L713, if we should use get_activity_cutoff_blocks to replace get_activity_cutoff? It will change the return type, from u16 to u64.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, for me to update:

  • metagraph call
  • breaking changes section in the description, regarding the metagraph

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.

Could you please mark ActivityCutoff as #[deprecated(note = "Some explanation")]? It will propagate to metadata.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Added: be695df

Copy link
Copy Markdown
Collaborator Author

@evgeny-s evgeny-s May 25, 2026

Choose a reason for hiding this comment

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

@l0r1s , BTW, it generates a lot of warnings with the deprecation notice in tests. But I planned to remove it in the follow-up PR. Are we ok to keep warnings?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Updated Metagraph + for now commented the deprecation note, otherwise clippy is failing. We will remove it fully in the follow up PR: a8fee82

evgeny-s added 3 commits May 15, 2026 14:07
# Conflicts:
#	pallets/subtensor/src/utils/rate_limiting.rs
# Conflicts:
#	pallets/subtensor/src/coinbase/run_coinbase.rs
#	pallets/subtensor/src/macros/dispatches.rs
#	pallets/subtensor/src/macros/events.rs
#	pallets/subtensor/src/macros/hooks.rs
#	pallets/subtensor/src/tests/migration.rs
#	pallets/subtensor/src/tests/mod.rs
#	pallets/subtensor/src/utils/rate_limiting.rs
@github-actions github-actions Bot mentioned this pull request May 21, 2026
4 tasks
- Updated reveal/commit logic based on stateful epoch index
- Updated migration for CR-v2 storage item
@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

This runtime migration still materializes every WeightCommits entry into a Vec before rewriting them. WeightCommits is live chain storage with no global bound, so a large commit set can make the upgrade block allocate excessive Wasm memory and fail the runtime upgrade. Iterate destructively, for example with drain() or another bounded/chunked migration pattern, so the upgrade never holds the whole map in memory at once.

Comment thread runtime/src/lib.rs
SubtensorModule::get_subnet_account_id(netuid)
}

fn get_next_epoch_start_block(netuid: NetUid) -> Option<u64> {
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] Runtime changes need a spec_version bump

This PR adds runtime API surface, storage items, dispatchables, weights, and an on_runtime_upgrade migration, but VERSION.spec_version remains unchanged at 407. Nodes use spec_version to distinguish runtime code, so a runtime-affecting upgrade must bump it before merge. Pinning this to the added runtime API because the unchanged spec_version line is not part of the diff.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every live CR-v2 commit entry before rewriting any of them. This migration runs inside the runtime upgrade path, so the allocation is bounded only by current chain storage and can exhaust Wasm memory or make the upgrade block overweight. Rewrite entries incrementally, or use a bounded cursor/translate-style migration that never holds the full map in memory.

Comment thread runtime/src/lib.rs
SubtensorModule::get_subnet_account_id(netuid)
}

fn get_next_epoch_start_block(netuid: NetUid) -> Option<u64> {
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] Runtime changes need a spec_version bump

This PR changes runtime behavior, storage, calls, events, migrations, and the runtime API, but runtime/src/lib.rs still has spec_version: 407 and the diff does not bump it. Nodes use spec_version to detect and apply runtime upgrades; merging runtime-affecting changes without incrementing it risks clients treating incompatible Wasm/native code as the same runtime.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

This migration materializes every WeightCommits entry into one Vec inside runtime Wasm before processing. WeightCommits is live user-driven storage, so the migration's peak memory scales with all unrevealed CR-v2 commits across all subnets/accounts; a sufficiently large state can exhaust Wasm memory during the runtime upgrade and brick block execution. Iterate and rewrite entries directly instead of collecting the whole map first, or otherwise prove a hard storage bound that fits runtime memory.

Comment thread runtime/src/lib.rs
SubtensorModule::get_subnet_account_id(netuid)
}

fn get_next_epoch_start_block(netuid: NetUid) -> Option<u64> {
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] Runtime changes need a spec_version bump

This PR changes runtime behavior and metadata by adding pallet storage, dispatchables, a migration, config, weights, and a runtime API, but VERSION.spec_version remains 407. Nodes will not reliably treat the native/runtime pair as a new runtime without a spec-version bump, and release tooling can miss the required upgrade boundary. Bump spec_version in runtime/src/lib.rs before merging.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

This migration still materializes every WeightCommits entry into a Vec before processing it. WeightCommits is live per-subnet/per-hotkey commit storage, so a large state can allocate beyond Wasm memory or exceed upgrade execution limits before the migration reaches its reads/writes accounting. Process entries in a streaming/bounded way, or split this into a bounded multi-block migration with persisted progress.

Comment thread runtime/src/lib.rs
SubtensorModule::get_subnet_account_id(netuid)
}

fn get_next_epoch_start_block(netuid: NetUid) -> Option<u64> {
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] Runtime changes need a spec_version bump

This PR changes runtime storage, dispatchables, pallet config, a runtime migration, and a runtime API, but RuntimeVersion still has spec_version: 407. Without bumping spec_version, upgrade coordination can fail because nodes do not see this as a new runtime version. Pinned here because the unchanged version line is outside the diff; bump spec_version in VERSION for this runtime-affecting change.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

…to make it cohesive with the emission distribution
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every CR-v2 commit entry across all subnets/accounts before rewriting them. This is user-scaled chain state, so a large live map can make the runtime migration exhaust Wasm memory or exceed block limits during on_runtime_upgrade. Stream the iterator and rewrite entries one at a time instead of collecting the full map first; account reads/writes incrementally inside the loop.

Comment thread runtime/src/lib.rs
SubtensorModule::get_subnet_account_id(netuid)
}

fn get_next_epoch_start_block(netuid: NetUid) -> Option<u64> {
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] Runtime changes need a spec_version bump

This PR changes the runtime API surface and pallet runtime behavior, but runtime/src/lib.rs still reports spec_version: 407. Without a spec version bump, nodes can treat incompatible native/Wasm runtimes as equivalent and clients may miss the runtime upgrade. Bump spec_version for this runtime-affecting change.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

This migration materializes every WeightCommits entry before rewriting them. Because it runs from on_runtime_upgrade, the entire live CR-v2 commit set is allocated in Wasm memory in one block; WeightCommits cardinality is not bounded by this migration, so a sufficiently large live state can exhaust Wasm memory or exceed the upgrade block budget and stall/brick the runtime upgrade. Rewrite this as a bounded/streaming migration or split it into cursor-based chunks, and only mark the migration complete after all chunks are processed.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have a few WeightCommits on mainnet, so we should be good.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every CR-v2 commit entry before rewriting any of them. This runs inside on_runtime_upgrade; if live commit storage is large enough, the upgrade can exceed Wasm memory or block weight and fail the runtime upgrade. Rewrite entries in a streaming/chunked way instead of collecting the full storage map first.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() materializes every CR-v2 commit queue in Wasm memory before the rewrite loop starts. WeightCommits is live runtime storage with no global bound across accounts/subnets, so a sufficiently large committed set can make the runtime upgrade exceed Wasm memory or block limits and abort the upgrade. Rewrite entries while streaming the iterator, or split this into a bounded/stepped migration that limits work and memory per block.

@github-actions
Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

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.

// `(hash, commit_epoch, commit_block, _)` layout. Field 1 was the absolute
// `commit_block`; it becomes `commit_epoch` (legacy modulo epoch). Field 2
// keeps the absolute `commit_block` (used by the epoch column-mask).
let crv2_entries: Vec<_> = WeightCommits::<T>::iter().collect();
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] Migration collects unbounded commit storage into Wasm memory

WeightCommits::<T>::iter().collect() loads every live CR-v2 commit row into one Vec during on_runtime_upgrade. This storage is keyed by subnet/account and can scale with live users, so a sufficiently large state can exhaust Wasm memory or trap before HasMigrationRun is written, blocking the runtime upgrade. Rewrite the storage incrementally with a bounded/paged migration state, or otherwise avoid collecting the full map before processing.

@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

breaking-change This PR introduces a noteworthy breaking change 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.

4 participants