Skip to content

Commit

Permalink
refactor(nns): Delete private neuron flags. (#3689)
Browse files Browse the repository at this point in the history 
# Motivation

This feature ([private & public neurons][feat]) has completed its
probation.

[feat]: https://dashboard.internetcomputer.org/proposal/130832

# References

Closes [NNS1-3228] and [NNS1-3248].

[NNS1-3228]: https://dfinity.atlassian.net/browse/NNS1-3228
[NNS1-3248]: https://dfinity.atlassian.net/browse/NNS1-3248
  • Loading branch information
@daniel-wong-dfinity-org
daniel-wong-dfinity-org authored Jan 30, 2025
1 parent de17b0d commit 2eae439
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 422 deletions.
29 changes: 1 addition & 28 deletions rs/nns/governance/src/governance.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
are_set_visibility_proposals_enabled, decoder_config,
decoder_config,
governance::{
merge_neurons::{
build_merge_neurons_response, calculate_merge_neurons_effect,
Expand Down Expand Up @@ -527,18 +527,6 @@ impl ManageNeuron {
(Some(nid), None) => Ok(Some(NeuronIdOrSubaccount::NeuronId(*nid))),
}
}

// TODO(NNS1-3228): Delete this.
fn is_set_visibility(&self) -> bool {
let Some(Command::Configure(ref configure)) = self.command else {
return false;
};

matches!(
configure.operation,
Some(manage_neuron::configure::Operation::SetVisibility(_)),
)
}
}

impl Command {
Expand Down Expand Up @@ -4978,21 +4966,6 @@ impl Governance {
&self,
manage_neuron: &ManageNeuron,
) -> Result<(), GovernanceError> {
// TODO(NNS1-3228): Delete this.
if manage_neuron.is_set_visibility() &&
// But SetVisibility proposals are disabled
!are_set_visibility_proposals_enabled()
{
return Err(GovernanceError::new_with_message(
ErrorType::Unavailable,
"Setting neuron visibility via proposal is not allowed yet, \
but it will be in the not too distant future. If you need \
this sooner, please, start a new thread at forum.dfinity.org \
and describe your use case."
.to_string(),
));
}

let managed_id = manage_neuron
.get_neuron_id_or_subaccount()?
.ok_or_else(|| {
Expand Down
41 changes: 0 additions & 41 deletions rs/nns/governance/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,6 @@ thread_local! {

static IS_PRUNE_FOLLOWING_ENABLED: Cell<bool> = const { Cell::new(true) };

// TODO(NNS1-3247): To release the feature, set this to true. Do not simply
// delete. That way, if we need to recall the feature, we can do that via a
// 1-line change (by replacing true with `cfg!(feature = "test")`). After
// the feature has been released, it will go through its "probation" period.
// If that goes well, then, this can be deleted.
static IS_PRIVATE_NEURON_ENFORCEMENT_ENABLED: Cell<bool> = const { Cell::new(true) };

static ARE_SET_VISIBILITY_PROPOSALS_ENABLED: Cell<bool> = const { Cell::new(true) };

static ALLOW_ACTIVE_NEURONS_IN_STABLE_MEMORY: Cell<bool> = const { Cell::new(true) };

static USE_STABLE_MEMORY_FOLLOWING_INDEX: Cell<bool> = const { Cell::new(true) };
Expand Down Expand Up @@ -255,38 +246,6 @@ pub fn temporarily_disable_prune_following() -> Temporary {
Temporary::new(&IS_PRUNE_FOLLOWING_ENABLED, false)
}

pub fn is_private_neuron_enforcement_enabled() -> bool {
IS_PRIVATE_NEURON_ENFORCEMENT_ENABLED.with(|ok| ok.get())
}

/// Only integration tests should use this.
#[cfg(any(test, feature = "canbench-rs", feature = "test"))]
pub fn temporarily_enable_private_neuron_enforcement() -> Temporary {
Temporary::new(&IS_PRIVATE_NEURON_ENFORCEMENT_ENABLED, true)
}

/// Only integration tests should use this.
#[cfg(any(test, feature = "canbench-rs", feature = "test"))]
pub fn temporarily_disable_private_neuron_enforcement() -> Temporary {
Temporary::new(&IS_PRIVATE_NEURON_ENFORCEMENT_ENABLED, false)
}

pub fn are_set_visibility_proposals_enabled() -> bool {
ARE_SET_VISIBILITY_PROPOSALS_ENABLED.with(|ok| ok.get())
}

/// Only integration tests should use this.
#[cfg(any(test, feature = "canbench-rs", feature = "test"))]
pub fn temporarily_enable_set_visibility_proposals() -> Temporary {
Temporary::new(&ARE_SET_VISIBILITY_PROPOSALS_ENABLED, true)
}

/// Only integration tests should use this.
#[cfg(any(test, feature = "canbench-rs", feature = "test"))]
pub fn temporarily_disable_set_visibility_proposals() -> Temporary {
Temporary::new(&ARE_SET_VISIBILITY_PROPOSALS_ENABLED, false)
}

pub fn allow_active_neurons_in_stable_memory() -> bool {
ALLOW_ACTIVE_NEURONS_IN_STABLE_MEMORY.with(|ok| ok.get())
}
Expand Down
11 changes: 3 additions & 8 deletions rs/nns/governance/src/neuron/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::{
LOG_PREFIX, MAX_DISSOLVE_DELAY_SECONDS, MAX_NEURON_AGE_FOR_AGE_BONUS,
MAX_NEURON_RECENT_BALLOTS, MAX_NUM_HOT_KEYS_PER_NEURON,
},
is_private_neuron_enforcement_enabled, is_voting_power_adjustment_enabled,
is_voting_power_adjustment_enabled,
neuron::{combine_aged_stakes, dissolve_state_and_age::DissolveStateAndAge, neuron_stake_e8s},
neuron_store::NeuronStoreError,
pb::v1::{
Expand Down Expand Up @@ -265,11 +265,7 @@ impl Neuron {
return Some(Visibility::Public);
}

if is_private_neuron_enforcement_enabled() {
return self.visibility.or(Some(Visibility::Private));
}

self.visibility
self.visibility.or(Some(Visibility::Private))
}

/// Sets a neuron's dissolve state and age.
Expand Down Expand Up @@ -955,8 +951,7 @@ impl Neuron {
let mut recent_ballots = vec![];
let mut joined_community_fund_timestamp_seconds = None;

let show_full = !is_private_neuron_enforcement_enabled()
|| self.visibility() == Some(Visibility::Public)
let show_full = self.visibility() == Some(Visibility::Public)
|| self.is_hotkey_or_controller(&requester);
if show_full {
let mut additional_recent_ballots = self
Expand Down
95 changes: 31 additions & 64 deletions rs/nns/governance/src/neuron/types/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ use crate::{
manage_neuron::{SetDissolveTimestamp, StartDissolving},
VotingPowerEconomics,
},
temporarily_disable_private_neuron_enforcement, temporarily_disable_voting_power_adjustment,
temporarily_enable_private_neuron_enforcement, temporarily_enable_voting_power_adjustment,
temporarily_disable_voting_power_adjustment, temporarily_enable_voting_power_adjustment,
};
use ic_cdk::println;

Expand Down Expand Up @@ -437,63 +436,37 @@ fn test_visibility_when_converting_neuron_to_neuron_info_and_neuron_proto() {
);

// Case 1: visibility is explicitly set.
for set_enforcement in [
temporarily_enable_private_neuron_enforcement,
temporarily_disable_private_neuron_enforcement,
] {
let _restore_on_drop = set_enforcement();

for visibility in [Visibility::Public, Visibility::Private] {
let neuron = builder.clone().with_visibility(Some(visibility)).build();
for visibility in [Visibility::Public, Visibility::Private] {
let neuron = builder.clone().with_visibility(Some(visibility)).build();

assert_eq!(neuron.visibility(), Some(visibility),);

let neuron_info = neuron.get_neuron_info(
&VotingPowerEconomics::DEFAULT,
timestamp_seconds,
principal_id,
);
assert_eq!(neuron_info.visibility, Some(visibility as i32),);

let neuron_proto = neuron.into_proto(&VotingPowerEconomics::DEFAULT, timestamp_seconds);
assert_eq!(neuron_proto.visibility, Some(visibility as i32),);
}
}

// Case 2: visibility is not set.
let neuron = builder.clone().build();
{
let _restore_on_drop = temporarily_disable_private_neuron_enforcement();

assert_eq!(neuron.visibility(), None,);
assert_eq!(neuron.visibility(), Some(visibility),);

let neuron_info = neuron.get_neuron_info(
&VotingPowerEconomics::DEFAULT,
timestamp_seconds,
principal_id,
);
assert_eq!(neuron_info.visibility, None,);
assert_eq!(neuron_info.visibility, Some(visibility as i32),);

let neuron_proto = neuron
.clone()
.into_proto(&VotingPowerEconomics::DEFAULT, timestamp_seconds);
assert_eq!(neuron_proto.visibility, None,);
let neuron_proto = neuron.into_proto(&VotingPowerEconomics::DEFAULT, timestamp_seconds);
assert_eq!(neuron_proto.visibility, Some(visibility as i32),);
}
{
let _restore_on_drop = temporarily_enable_private_neuron_enforcement();

assert_eq!(neuron.visibility(), Some(Visibility::Private),);
// Case 2: visibility is not set.

let neuron_info = neuron.get_neuron_info(
&VotingPowerEconomics::DEFAULT,
timestamp_seconds,
principal_id,
);
assert_eq!(neuron_info.visibility, Some(Visibility::Private as i32),);
let neuron = builder.clone().build();

let neuron_proto = neuron.into_proto(&VotingPowerEconomics::DEFAULT, timestamp_seconds);
assert_eq!(neuron_proto.visibility, Some(Visibility::Private as i32),);
}
assert_eq!(neuron.visibility(), Some(Visibility::Private),);

let neuron_info = neuron.get_neuron_info(
&VotingPowerEconomics::DEFAULT,
timestamp_seconds,
principal_id,
);
assert_eq!(neuron_info.visibility, Some(Visibility::Private as i32),);

let neuron_proto = neuron.into_proto(&VotingPowerEconomics::DEFAULT, timestamp_seconds);
assert_eq!(neuron_proto.visibility, Some(Visibility::Private as i32),);

// Case 3: Known neurons are always public.
let neuron = builder
Expand All @@ -502,26 +475,20 @@ fn test_visibility_when_converting_neuron_to_neuron_info_and_neuron_proto() {
description: Some("neuron description".to_string()),
}))
.build();
for set_enforcement in [
temporarily_enable_private_neuron_enforcement,
temporarily_disable_private_neuron_enforcement,
] {
let _restore_on_drop = set_enforcement();

assert_eq!(neuron.visibility(), Some(Visibility::Public),);
assert_eq!(neuron.visibility(), Some(Visibility::Public),);

let neuron_info = neuron.get_neuron_info(
&VotingPowerEconomics::DEFAULT,
timestamp_seconds,
principal_id,
);
assert_eq!(neuron_info.visibility, Some(Visibility::Public as i32),);
let neuron_info = neuron.get_neuron_info(
&VotingPowerEconomics::DEFAULT,
timestamp_seconds,
principal_id,
);
assert_eq!(neuron_info.visibility, Some(Visibility::Public as i32),);

let neuron_proto = neuron
.clone()
.into_proto(&VotingPowerEconomics::DEFAULT, timestamp_seconds);
assert_eq!(neuron_proto.visibility, Some(Visibility::Public as i32),);
}
let neuron_proto = neuron
.clone()
.into_proto(&VotingPowerEconomics::DEFAULT, timestamp_seconds);
assert_eq!(neuron_proto.visibility, Some(Visibility::Public as i32),);
}

#[test]
Expand Down
Loading

0 comments on commit 2eae439

Please sign in to comment.