Skip to content

Commit

Permalink
Merge branch 'jason/NNS1-3083' into 'master'
Browse files Browse the repository at this point in the history
feat(nns): NNS1-3201 Create 2 new topics while not allowing following to be set on them

More context: https://forum.dfinity.org/t/refine-nns-proposals-topics/32125

We don't allow setting following on them yet because the downstream isn't ready.

Closes NNS1-3201 

Closes NNS1-3201

See merge request dfinity-lab/public/ic!20398
  • Loading branch information
jasonz-dfinity committed Jul 13, 2024
2 parents e998f71 + 891c742 commit 731eb5a
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 4 deletions.
10 changes: 10 additions & 0 deletions rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3522,6 +3522,12 @@ pub enum Topic {
ApiBoundaryNodeManagement = 15,
/// Proposals related to subnet rental.
SubnetRental = 16,
/// Proposals to manage protocol canisters. Those are canisters that are considered part of the IC
/// protocol, without which the IC will not be able to function properly.
ProtocolCanisterManagement = 17,
/// Proposals related to Service Nervous System (SNS) - (1) upgrading SNS-W, (2) upgrading SNS
/// Aggregator, and (3) adding WASM's or custom upgrade paths to SNS-W.
ServiceNervousSystemManagement = 18,
}
impl Topic {
/// String value of the enum field names used in the ProtoBuf definition.
Expand All @@ -3546,6 +3552,8 @@ impl Topic {
Topic::SnsAndCommunityFund => "TOPIC_SNS_AND_COMMUNITY_FUND",
Topic::ApiBoundaryNodeManagement => "TOPIC_API_BOUNDARY_NODE_MANAGEMENT",
Topic::SubnetRental => "TOPIC_SUBNET_RENTAL",
Topic::ProtocolCanisterManagement => "TOPIC_PROTOCOL_CANISTER_MANAGEMENT",
Topic::ServiceNervousSystemManagement => "TOPIC_SERVICE_NERVOUS_SYSTEM_MANAGEMENT",
}
}
/// Creates an enum from field names used in the ProtoBuf definition.
Expand All @@ -3567,6 +3575,8 @@ impl Topic {
"TOPIC_SNS_AND_COMMUNITY_FUND" => Some(Self::SnsAndCommunityFund),
"TOPIC_API_BOUNDARY_NODE_MANAGEMENT" => Some(Self::ApiBoundaryNodeManagement),
"TOPIC_SUBNET_RENTAL" => Some(Self::SubnetRental),
"TOPIC_PROTOCOL_CANISTER_MANAGEMENT" => Some(Self::ProtocolCanisterManagement),
"TOPIC_SERVICE_NERVOUS_SYSTEM_MANAGEMENT" => Some(Self::ServiceNervousSystemManagement),
_ => None,
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ enum Topic {
// Proposals related to subnet rental.
TOPIC_SUBNET_RENTAL = 16;

// Proposals to manage protocol canisters. Those are canisters that are considered part of the IC
// protocol, without which the IC will not be able to function properly.
TOPIC_PROTOCOL_CANISTER_MANAGEMENT = 17;

// Proposals related to Service Nervous System (SNS) - (1) upgrading SNS-W, (2) upgrading SNS
// Aggregator, and (3) adding WASM's or custom upgrade paths to SNS-W.
TOPIC_SERVICE_NERVOUS_SYSTEM_MANAGEMENT = 18;

// Superseded by SNS_COMMUNITY_FUND.
reserved 11;
reserved "TOPIC_SNS_DECENTRALIZATION_SALE";
Expand Down
10 changes: 10 additions & 0 deletions rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3522,6 +3522,12 @@ pub enum Topic {
ApiBoundaryNodeManagement = 15,
/// Proposals related to subnet rental.
SubnetRental = 16,
/// Proposals to manage protocol canisters. Those are canisters that are considered part of the IC
/// protocol, without which the IC will not be able to function properly.
ProtocolCanisterManagement = 17,
/// Proposals related to Service Nervous System (SNS) - (1) upgrading SNS-W, (2) upgrading SNS
/// Aggregator, and (3) adding WASM's or custom upgrade paths to SNS-W.
ServiceNervousSystemManagement = 18,
}
impl Topic {
/// String value of the enum field names used in the ProtoBuf definition.
Expand All @@ -3546,6 +3552,8 @@ impl Topic {
Topic::SnsAndCommunityFund => "TOPIC_SNS_AND_COMMUNITY_FUND",
Topic::ApiBoundaryNodeManagement => "TOPIC_API_BOUNDARY_NODE_MANAGEMENT",
Topic::SubnetRental => "TOPIC_SUBNET_RENTAL",
Topic::ProtocolCanisterManagement => "TOPIC_PROTOCOL_CANISTER_MANAGEMENT",
Topic::ServiceNervousSystemManagement => "TOPIC_SERVICE_NERVOUS_SYSTEM_MANAGEMENT",
}
}
/// Creates an enum from field names used in the ProtoBuf definition.
Expand All @@ -3567,6 +3575,8 @@ impl Topic {
"TOPIC_SNS_AND_COMMUNITY_FUND" => Some(Self::SnsAndCommunityFund),
"TOPIC_API_BOUNDARY_NODE_MANAGEMENT" => Some(Self::ApiBoundaryNodeManagement),
"TOPIC_SUBNET_RENTAL" => Some(Self::SubnetRental),
"TOPIC_PROTOCOL_CANISTER_MANAGEMENT" => Some(Self::ProtocolCanisterManagement),
"TOPIC_SERVICE_NERVOUS_SYSTEM_MANAGEMENT" => Some(Self::ServiceNervousSystemManagement),
_ => None,
}
}
Expand Down
18 changes: 14 additions & 4 deletions rs/nns/governance/src/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1324,7 +1324,7 @@ impl Topic {
pub const MIN: Topic = Topic::Unspecified;
// A unit test will fail if this value does not stay up to date (e.g. when a new value is
// added).
pub const MAX: Topic = Topic::SubnetRental;
pub const MAX: Topic = Topic::ServiceNervousSystemManagement;

/// When voting rewards are distributed, the voting power of
/// neurons voting on proposals are weighted by this amount. The
Expand Down Expand Up @@ -5643,19 +5643,29 @@ impl Governance {
}

// Validate topic exists
Topic::try_from(follow_request.topic).map_err(|_| {
let topic = Topic::try_from(follow_request.topic).map_err(|_| {
GovernanceError::new_with_message(
ErrorType::InvalidCommand,
format!("Not a known topic number. Follow:\n{:#?}", follow_request),
)
})?;

#[cfg(not(feature = "test"))]
if topic == Topic::ProtocolCanisterManagement
|| topic == Topic::ServiceNervousSystemManagement
{
return Err(GovernanceError::new_with_message(
ErrorType::InvalidCommand,
format!("Cannot follow the {:?} topic yet", topic),
));
}

self.with_neuron_mut(id, |neuron| {
if follow_request.followees.is_empty() {
neuron.followees.remove(&follow_request.topic)
neuron.followees.remove(&(topic as i32))
} else {
neuron.followees.insert(
follow_request.topic,
topic as i32,
Followees {
followees: follow_request.followees.clone(),
},
Expand Down
105 changes: 105 additions & 0 deletions rs/nns/governance/src/governance/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1540,6 +1540,111 @@ fn test_validate_execute_nns_function() {
}
}

// TODO(NNS1-3204): Remove this test when the new topics are enabled.
#[test]
fn test_follow_new_topics() {
// Step 1: set up a neuron with no followees.
let mut governance = Governance::new(
GovernanceProto {
economics: Some(NetworkEconomics::with_default_values()),
..Default::default()
},
Box::new(MockEnvironment::new(vec![], 100)),
Box::new(StubIcpLedger {}),
Box::new(StubCMC {}),
);
let neuron_id = NeuronId { id: 1 };
let controller = PrincipalId::new_user_test_id(1);
governance
.neuron_store
.add_neuron(
NeuronBuilder::new(
neuron_id,
Subaccount::try_from(vec![0u8; 32].as_slice()).unwrap(),
controller,
DissolveStateAndAge::NotDissolving {
dissolve_delay_seconds: 42,
aging_since_timestamp_seconds: 1,
},
123_456_789,
)
.build(),
)
.unwrap();

// Step 2: sanity check to make sure `follow()` works.
governance
.follow(
&neuron_id,
&controller,
&manage_neuron::Follow {
topic: Topic::Unspecified as i32,
followees: [NeuronId { id: 2 }].to_vec(),
},
)
.unwrap();

// Step 3: following a new topic works with feature = "test".
#[cfg(feature = "test")]
{
assert_eq!(
governance.follow(
&neuron_id,
&controller,
&manage_neuron::Follow {
topic: Topic::ProtocolCanisterManagement as i32,
followees: [NeuronId { id: 2 }].to_vec(),
},
),
Ok(())
);
assert_eq!(
governance.follow(
&neuron_id,
&controller,
&manage_neuron::Follow {
topic: Topic::ServiceNervousSystemManagement as i32,
followees: [NeuronId { id: 2 }].to_vec(),
},
),
Ok(())
);
}

// Step 4: following a new topic fails without feature = "test".
#[cfg(not(feature = "test"))]
{
assert_eq!(
governance.follow(
&neuron_id,
&controller,
&manage_neuron::Follow {
topic: Topic::ProtocolCanisterManagement as i32,
followees: [NeuronId { id: 2 }].to_vec(),
},
),
Err(GovernanceError::new_with_message(
ErrorType::InvalidCommand,
"Cannot follow the ProtocolCanisterManagement topic yet".to_string()
))
);
assert_eq!(
governance.follow(
&neuron_id,
&controller,
&manage_neuron::Follow {
topic: Topic::ServiceNervousSystemManagement as i32,
followees: [NeuronId { id: 2 }].to_vec(),
},
),
Err(GovernanceError::new_with_message(
ErrorType::InvalidCommand,
"Cannot follow the ServiceNervousSystemManagement topic yet".to_string()
))
);
}
}

#[test]
fn topic_min_max_test() {
use strum::IntoEnumIterator;
Expand Down

0 comments on commit 731eb5a

Please sign in to comment.