Skip to content

Consensus modes devnet#2202

Closed
konrad0960 wants to merge 16 commits intoopentensor:devnet-readyfrom
konrad0960:consensus-modes-devnet
Closed

Consensus modes devnet#2202
konrad0960 wants to merge 16 commits intoopentensor:devnet-readyfrom
konrad0960:consensus-modes-devnet

Conversation

@konrad0960
Copy link
Copy Markdown
Contributor

Description

Introducing consensus modes for liquid alpha calculations (using previous epoch or current epoch consensus)
The AUTO mode on default sets to CURRENT if bond_penalty != 1 and PREVIOUS if bond_penalty == 1

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

Additional Notes

This is my first Subtensor contribution so please review it carefully and be lenient.

@sam0x17 sam0x17 added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Dec 1, 2025
@sam0x17
Copy link
Copy Markdown
Contributor

sam0x17 commented Dec 1, 2025

needs to merge in latest devnet-ready

Comment on lines +1433 to +1440
let previous_consensus_u16 = Consensus::<T>::get(netuid);
previous_consensus_u16
.iter()
.map(|&c| {
I32F32::saturating_from_num(c)
.safe_div(I32F32::saturating_from_num(u16::MAX))
})
.collect()
Copy link
Copy Markdown
Collaborator

@l0r1s l0r1s Jan 6, 2026

Choose a reason for hiding this comment

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

Duplicated below, should be extracted to a function

Comment on lines +214 to +222
let diff = if *res > expected {
*res - expected
} else {
expected - *res
};
assert!(
diff < I32F32::from_num(0.001),
"Values should be approximately equal"
);
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.

assert_abs_diff_eq! can be useful here

Comment on lines +260 to +268
let diff = if *res > expected {
*res - expected
} else {
expected - *res
};
assert!(
diff < I32F32::from_num(0.001),
"Should use previous consensus when bond_penalty == 1"
);
Copy link
Copy Markdown
Collaborator

@l0r1s l0r1s Jan 6, 2026

Choose a reason for hiding this comment

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

assert_abs_diff_eq! can be useful here

Copy link
Copy Markdown
Collaborator

@l0r1s l0r1s left a comment

Choose a reason for hiding this comment

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

Not sure if it is missing a dispatch call

@panniki panniki force-pushed the consensus-modes-devnet branch from 958be6e to 92b39a0 Compare February 8, 2026 12:52
ales-otf
ales-otf previously approved these changes Feb 18, 2026

/// Sets up a network with full owner permissions (root registration + network creation)
/// Returns (netuid, hotkey, coldkey, signer)
fn setup_network_with_owner() -> (NetUid, U256, U256, RuntimeOrigin) {
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 helper is generic enough to put it in the mock module. that would prevent local duplications in the future.

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.

and it makes sense to consider moving there the following two helpers as well

(converted_low, converted_high)
}

pub fn get_liquid_alpha_consensus_mode(netuid: NetUid) -> ConsensusMode {
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's better to omit these kind of getters/setters. the storage api works pretty well and is self-descriptive.

@ppolewicz
Copy link
Copy Markdown
Collaborator

Rebased this PR onto origin/skills-devnet-ready and pushed the updated branch to konrad0960/consensus-modes-devnet.

Addressed the remaining review feedback from l0r1s and ales-otf:

  • kept the consensus mode change dispatchable and wired tests through the extrinsic path where appropriate
  • removed the extra get_/set_ liquid-alpha consensus storage wrappers and used the storage API directly
  • moved the reusable consensus-mode test setup helpers into tests/mock.rs
  • kept the duplicated consensus conversion extracted into a helper and retained the assert_abs_diff_eq! assertions
  • fixed the rebased extrinsic metadata so set_liquid_alpha_consensus_mode uses the next free call index (134) instead of colliding with the existing 133

Verification run locally:

  • cargo fmt --check
  • cargo test -p pallet-subtensor consensus_mode
  • cargo test -p pallet-subtensor test_liquid_alpha_equal_values_against_itself

@ppolewicz
Copy link
Copy Markdown
Collaborator

This is continued in #2568

@ppolewicz ppolewicz closed this Apr 7, 2026
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.

6 participants