Skip to content

Commit 4e2a8f7

Browse files
jasonz-dfinityjasonz-dfinity
committed
Implement self-describing for DeregisterKnownNeuron/RegisterKnownNeuron
1 parent e625642 commit 4e2a8f7

9 files changed

+242
-56
lines changed

‎rs/nns/governance/src/proposals/add_or_remove_node_provider_tests.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use crate::pb::v1::{AddOrRemoveNodeProvider, NodeProvider, add_or_remove_node_pr
44

55
use assert_matches::assert_matches;
66
use ic_base_types::PrincipalId;
7-
use ic_nns_governance_api::SelfDescribingValue as ApiValue;
7+
use ic_nns_governance_api::SelfDescribingValue;
88
use icp_ledger::protobuf::AccountIdentifier as AccountIdentifierProto;
99
use maplit::hashmap;
1010

@@ -331,17 +331,17 @@ fn test_to_self_describing_value() {
331331
};
332332

333333
assert_eq!(
334-
ApiValue::from(
334+
SelfDescribingValue::from(
335335
ValidAddOrRemoveNodeProvider::try_from(add_node_provider)
336336
.unwrap()
337337
.to_self_describing_value()
338338
),
339-
ApiValue::Map(hashmap! {
340-
"to_add".to_string() => ApiValue::Map(hashmap! {
341-
"id".to_string() => ApiValue::Text("6fyp7-3ibaa-aaaaa-aaaap-4ai".to_string()),
342-
"reward_account".to_string() => ApiValue::Array(vec![
343-
ApiValue::Map(hashmap! {
344-
"account_identifier".to_string() => ApiValue::Text(
339+
SelfDescribingValue::Map(hashmap! {
340+
"to_add".to_string() => SelfDescribingValue::Map(hashmap! {
341+
"id".to_string() => SelfDescribingValue::Text("6fyp7-3ibaa-aaaaa-aaaap-4ai".to_string()),
342+
"reward_account".to_string() => SelfDescribingValue::Array(vec![
343+
SelfDescribingValue::Map(hashmap! {
344+
"account_identifier".to_string() => SelfDescribingValue::Text(
345345
account_identifer_hex.to_string()
346346
)
347347
})
@@ -358,14 +358,14 @@ fn test_to_self_describing_value() {
358358
};
359359

360360
assert_eq!(
361-
ApiValue::from(
361+
SelfDescribingValue::from(
362362
ValidAddOrRemoveNodeProvider::try_from(remove_node_provider)
363363
.unwrap()
364364
.to_self_describing_value()
365365
),
366-
ApiValue::Map(hashmap! {
367-
"to_remove".to_string() => ApiValue::Map(hashmap! {
368-
"id".to_string() => ApiValue::Text("6fyp7-3ibaa-aaaaa-aaaap-4ai".to_string())
366+
SelfDescribingValue::Map(hashmap! {
367+
"to_remove".to_string() => SelfDescribingValue::Map(hashmap! {
368+
"id".to_string() => SelfDescribingValue::Text("6fyp7-3ibaa-aaaaa-aaaap-4ai".to_string())
369369
})
370370
})
371371
);

‎rs/nns/governance/src/proposals/decode_candid_args_to_self_describing_value.rs‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1-
use crate::pb::v1::{
2-
SelfDescribingValue, SelfDescribingValueArray, SelfDescribingValueMap,
3-
self_describing_value::Value,
1+
use crate::{
2+
pb::v1::{
3+
SelfDescribingValue, SelfDescribingValueArray, SelfDescribingValueMap,
4+
self_describing_value::Value,
5+
},
6+
proposals::self_describing::{to_self_describing_int, to_self_describing_nat},
47
};
58

69
use candid::{
7-
IDLValue, Int, Nat,
10+
IDLValue,
811
types::value::{IDLField, VariantValue},
912
};
1013
use candid_parser::{IDLArgs, IDLProg, TypeEnv, check_prog};
@@ -147,20 +150,6 @@ fn convert_variant_to_self_describing(
147150
}
148151
}
149152

150-
fn to_self_describing_nat(n: impl Into<Nat>) -> Value {
151-
let n = n.into();
152-
let mut bytes = Vec::new();
153-
n.encode(&mut bytes).expect("Failed to encode Nat");
154-
Value::Nat(bytes)
155-
}
156-
157-
fn to_self_describing_int(i: impl Into<Int>) -> Value {
158-
let i = i.into();
159-
let mut bytes = Vec::new();
160-
i.encode(&mut bytes).expect("Failed to encode Int");
161-
Value::Int(bytes)
162-
}
163-
164153
#[path = "decode_candid_args_to_self_describing_value_tests.rs"]
165154
#[cfg(test)]
166155
pub mod tests;

‎rs/nns/governance/src/proposals/deregister_known_neuron.rs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
use crate::{
22
neuron_store::NeuronStore,
3-
pb::v1::{DeregisterKnownNeuron, GovernanceError, governance_error::ErrorType},
3+
pb::v1::{
4+
DeregisterKnownNeuron, GovernanceError, SelfDescribingValue, governance_error::ErrorType,
5+
},
6+
proposals::self_describing::{LocallyDescribableProposalAction, ValueBuilder},
47
};
58

69
impl DeregisterKnownNeuron {
@@ -50,6 +53,18 @@ impl DeregisterKnownNeuron {
5053
}
5154
}
5255

56+
impl LocallyDescribableProposalAction for DeregisterKnownNeuron {
57+
const TYPE_NAME: &'static str = "Deregister Known Neuron";
58+
const TYPE_DESCRIPTION: &'static str = "Deregisters a known neuron, removing its known neuron \
59+
data (name, description, etc.) and making it a regular neuron.";
60+
61+
fn to_self_describing_value(&self) -> SelfDescribingValue {
62+
ValueBuilder::new()
63+
.add_field_with_empty_as_fallback("neuron_id", self.id.map(|id| id.id))
64+
.build()
65+
}
66+
}
67+
5368
#[cfg(test)]
5469
#[path = "deregister_known_neuron_tests.rs"]
5570
mod tests;

‎rs/nns/governance/src/proposals/deregister_known_neuron_tests.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
1+
use crate::proposals::self_describing::LocallyDescribableProposalAction;
12
use crate::{
23
neuron::{DissolveStateAndAge, NeuronBuilder},
34
neuron_store::NeuronStore,
45
pb::v1::{DeregisterKnownNeuron, KnownNeuronData, governance_error::ErrorType},
56
};
67
use assert_matches::assert_matches;
78
use ic_nns_common::pb::v1::NeuronId;
9+
use ic_nns_governance_api::SelfDescribingValue;
10+
use maplit::hashmap;
811
use std::collections::BTreeMap;
912

1013
fn create_test_neuron_store() -> NeuronStore {
@@ -184,3 +187,20 @@ fn test_execute_nonexistent_neuron() {
184187
let result = request.execute(&mut neuron_store);
185188
assert_matches!(result, Err(_), "Execute should fail for nonexistent neuron");
186189
}
190+
191+
#[test]
192+
fn test_deregister_known_neuron_to_self_describing() {
193+
let deregister = DeregisterKnownNeuron {
194+
id: Some(NeuronId { id: 456 }),
195+
};
196+
197+
let action = deregister.to_self_describing_action();
198+
let value = SelfDescribingValue::from(action.value.unwrap());
199+
200+
assert_eq!(
201+
value,
202+
SelfDescribingValue::Map(hashmap! {
203+
"neuron_id".to_string() => SelfDescribingValue::Nat(candid::Nat::from(456u64)),
204+
})
205+
);
206+
}

‎rs/nns/governance/src/proposals/mod.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,12 @@ impl ValidProposalAction {
198198
ValidProposalAction::ExecuteNnsFunction(execute_nns_function) => {
199199
execute_nns_function.to_self_describing_action(env).await
200200
}
201+
ValidProposalAction::RegisterKnownNeuron(register_known_neuron) => {
202+
Ok(register_known_neuron.to_self_describing_action())
203+
}
204+
ValidProposalAction::DeregisterKnownNeuron(deregister_known_neuron) => {
205+
Ok(deregister_known_neuron.to_self_describing_action())
206+
}
201207
_ => Err(GovernanceError::new_with_message(
202208
ErrorType::InvalidProposal,
203209
"Self describing proposal actions are not supported for this proposal action yet.",

‎rs/nns/governance/src/proposals/register_known_neuron.rs‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
use crate::{
22
neuron_store::NeuronStore,
3-
pb::v1::{GovernanceError, KnownNeuron, governance_error::ErrorType},
3+
pb::v1::{
4+
GovernanceError, KnownNeuron, KnownNeuronData, SelfDescribingValue, Topic,
5+
governance_error::ErrorType,
6+
},
7+
proposals::self_describing::{LocallyDescribableProposalAction, ValueBuilder},
48
};
5-
use std::collections::HashSet;
69

710
use ic_nervous_system_common_validation::validate_url;
11+
use std::collections::HashSet;
812

913
/// Maximum size in bytes for a neuron's name, in KnownNeuronData.
1014
pub const KNOWN_NEURON_NAME_MAX_LEN: usize = 200;
@@ -172,6 +176,45 @@ impl KnownNeuron {
172176
}
173177
}
174178

179+
impl LocallyDescribableProposalAction for KnownNeuron {
180+
const TYPE_NAME: &'static str = "Register Known Neuron";
181+
const TYPE_DESCRIPTION: &'static str = "Registers a neuron as a known neuron. This allows the \
182+
neuron to be looked up by name and displayed more prominently in the NNS UI.";
183+
184+
fn to_self_describing_value(&self) -> SelfDescribingValue {
185+
ValueBuilder::new()
186+
.add_field_with_empty_as_fallback("neuron_id", self.id.map(|id| id.id))
187+
.add_field_with_empty_as_fallback("known_neuron_data", self.known_neuron_data.clone())
188+
.build()
189+
}
190+
}
191+
192+
impl From<KnownNeuronData> for SelfDescribingValue {
193+
fn from(data: KnownNeuronData) -> Self {
194+
let KnownNeuronData {
195+
name,
196+
description,
197+
links,
198+
committed_topics,
199+
} = data;
200+
201+
let committed_topics: Vec<_> = committed_topics
202+
.into_iter()
203+
.map(|topic| {
204+
let topic = Topic::try_from(topic).unwrap_or(Topic::Unspecified);
205+
format!("{:?}", topic)
206+
})
207+
.collect();
208+
209+
ValueBuilder::new()
210+
.add_field("name", name)
211+
.add_field("description", description)
212+
.add_field("links", links)
213+
.add_field("committed_topics", committed_topics)
214+
.build()
215+
}
216+
}
217+
175218
#[cfg(test)]
176219
#[path = "register_known_neuron_tests.rs"]
177220
mod tests;

‎rs/nns/governance/src/proposals/register_known_neuron_tests.rs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use super::*;
22

3+
use crate::proposals::self_describing::LocallyDescribableProposalAction;
34
use crate::{
45
neuron::{DissolveStateAndAge, NeuronBuilder},
56
neuron_store::NeuronStore,
@@ -10,6 +11,8 @@ use crate::{
1011
};
1112
use assert_matches::assert_matches;
1213
use ic_nns_common::pb::v1::NeuronId;
14+
use ic_nns_governance_api::SelfDescribingValue;
15+
use maplit::hashmap;
1316
use std::collections::BTreeMap;
1417

1518
fn create_test_neuron_store() -> NeuronStore {
@@ -497,3 +500,67 @@ fn test_validate_duplicate_committed_topics() {
497500
&& error.error_message.contains("3")
498501
);
499502
}
503+
504+
#[test]
505+
fn test_known_neuron_to_self_describing() {
506+
let known_neuron = KnownNeuron {
507+
id: Some(NeuronId { id: 123 }),
508+
known_neuron_data: Some(KnownNeuronData {
509+
name: "Test Neuron".to_string(),
510+
description: Some("Description".to_string()),
511+
links: vec!["https://test.com".to_string()],
512+
committed_topics: vec![Topic::Governance as i32],
513+
}),
514+
};
515+
516+
let action = known_neuron.to_self_describing_action();
517+
let value = SelfDescribingValue::from(action.value.unwrap());
518+
519+
assert_eq!(
520+
value,
521+
SelfDescribingValue::Map(hashmap! {
522+
"neuron_id".to_string() => SelfDescribingValue::Nat(candid::Nat::from(123u64)),
523+
"known_neuron_data".to_string() => SelfDescribingValue::Map(hashmap! {
524+
"name".to_string() => SelfDescribingValue::Text("Test Neuron".to_string()),
525+
"description".to_string() => SelfDescribingValue::Array(vec![
526+
SelfDescribingValue::Text("Description".to_string())
527+
]),
528+
"links".to_string() => SelfDescribingValue::Array(vec![
529+
SelfDescribingValue::Text("https://test.com".to_string())
530+
]),
531+
"committed_topics".to_string() => SelfDescribingValue::Array(vec![
532+
SelfDescribingValue::Text("Governance".to_string())
533+
]),
534+
}),
535+
})
536+
);
537+
}
538+
539+
#[test]
540+
fn test_known_neuron_to_self_describing_empty_fields() {
541+
let known_neuron = KnownNeuron {
542+
id: Some(NeuronId { id: 123 }),
543+
known_neuron_data: Some(KnownNeuronData {
544+
name: "Test Neuron".to_string(),
545+
description: None,
546+
links: vec![],
547+
committed_topics: vec![],
548+
}),
549+
};
550+
551+
let action = known_neuron.to_self_describing_action();
552+
let value = SelfDescribingValue::from(action.value.unwrap());
553+
554+
assert_eq!(
555+
value,
556+
SelfDescribingValue::Map(hashmap! {
557+
"neuron_id".to_string() => SelfDescribingValue::Nat(candid::Nat::from(123u64)),
558+
"known_neuron_data".to_string() => SelfDescribingValue::Map(hashmap! {
559+
"name".to_string() => SelfDescribingValue::Text("Test Neuron".to_string()),
560+
"description".to_string() => SelfDescribingValue::Array(vec![]),
561+
"links".to_string() => SelfDescribingValue::Array(vec![]),
562+
"committed_topics".to_string() => SelfDescribingValue::Array(vec![]),
563+
}),
564+
})
565+
);
566+
}

0 commit comments

Comments
 (0)