Skip to content

Commit 55dd71d

Browse files
Define a skeleton for self-describing proposals
1 parent f07e989 commit 55dd71d

File tree

4 files changed

+135
-6
lines changed

4 files changed

+135
-6
lines changed

rs/nns/governance/src/governance.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5124,6 +5124,14 @@ impl Governance {
51245124
));
51255125
}
51265126

5127+
let self_describing_action = if is_self_describing_proposal_actions_enabled() {
5128+
// TODO(NNS1-4271): handle the error case when the self-describing action is fully
5129+
// implemented.
5130+
action.to_self_describing(self.env.clone()).await.ok()
5131+
} else {
5132+
None
5133+
};
5134+
51275135
// Before actually modifying anything, we first make sure that
51285136
// the neuron is allowed to make this proposal and create the
51295137
// electoral roll.
@@ -5263,10 +5271,14 @@ impl Governance {
52635271

52645272
Proposal {
52655273
title,
5274+
self_describing_action,
52665275
..proposal.clone()
52675276
}
52685277
} else {
5269-
proposal.clone()
5278+
Proposal {
5279+
self_describing_action,
5280+
..proposal.clone()
5281+
}
52705282
};
52715283

52725284
// Wait-For-Quiet is not enabled for ManageNeuron

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,22 @@
11
use crate::{
2-
governance::LOG_PREFIX,
2+
governance::{Environment, LOG_PREFIX},
33
pb::v1::{
44
AddOrRemoveNodeProvider, ApproveGenesisKyc, CreateServiceNervousSystem,
55
DeregisterKnownNeuron, FulfillSubnetRentalRequest, GovernanceError, InstallCode,
66
KnownNeuron, ManageNeuron, Motion, NetworkEconomics, ProposalData, RewardNodeProvider,
7-
RewardNodeProviders, StopOrStartCanister, Topic, UpdateCanisterSettings, Vote,
8-
governance_error::ErrorType, proposal::Action,
7+
RewardNodeProviders, SelfDescribingProposalAction, StopOrStartCanister, Topic,
8+
UpdateCanisterSettings, Vote, governance_error::ErrorType, proposal::Action,
9+
},
10+
proposals::{
11+
execute_nns_function::ValidExecuteNnsFunction,
12+
self_describing::LocallyDescribableProposalAction,
913
},
10-
proposals::execute_nns_function::ValidExecuteNnsFunction,
1114
};
1215
use ic_base_types::CanisterId;
1316
use ic_cdk::println;
1417
use ic_nns_common::pb::v1::NeuronId;
1518
use ic_nns_constants::{PROTOCOL_CANISTER_IDS, SNS_AGGREGATOR_CANISTER_ID, SNS_WASM_CANISTER_ID};
16-
use std::collections::HashMap;
19+
use std::{collections::HashMap, sync::Arc};
1720

1821
pub mod call_canister;
1922
pub mod create_service_nervous_system;
@@ -22,6 +25,7 @@ pub mod execute_nns_function;
2225
pub mod fulfill_subnet_rental_request;
2326
pub mod install_code;
2427
pub mod register_known_neuron;
28+
pub mod self_describing;
2529
pub mod stop_or_start_canister;
2630
pub mod update_canister_settings;
2731

@@ -170,6 +174,20 @@ impl ValidProposalAction {
170174
None
171175
}
172176
}
177+
178+
/// Converts the proposal action to a self describing representation of itself.
179+
pub async fn to_self_describing(
180+
&self,
181+
_env: Arc<dyn Environment>,
182+
) -> Result<SelfDescribingProposalAction, GovernanceError> {
183+
match self {
184+
ValidProposalAction::Motion(motion) => Ok(motion.to_self_describing()),
185+
_ => Err(GovernanceError::new_with_message(
186+
ErrorType::InvalidProposal,
187+
"Self describing proposal actions are not supported for this proposal action yet.",
188+
)),
189+
}
190+
}
173191
}
174192

175193
const SNS_RELATED_CANISTER_IDS: [&CanisterId; 2] =
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
use std::collections::HashMap;
2+
3+
use crate::pb::v1::{
4+
Motion, SelfDescribingProposalAction, Value, ValueMap,
5+
value::Value::{Map, Text},
6+
};
7+
8+
/// A proposal action that can be described locally, without having to call `canister_metadata`
9+
/// management canister method to get the candid file of an external canister. Every proposal action
10+
/// except for `ExecuteNnsFunction` should implement this trait.
11+
pub trait LocallyDescribableProposalAction {
12+
const TYPE_NAME: &'static str;
13+
const TYPE_DESCRIPTION: &'static str;
14+
15+
fn to_value(&self) -> Value;
16+
17+
fn to_self_describing(&self) -> SelfDescribingProposalAction {
18+
SelfDescribingProposalAction {
19+
type_name: Self::TYPE_NAME.to_string(),
20+
type_description: Self::TYPE_DESCRIPTION.to_string(),
21+
value: Some(self.to_value()),
22+
}
23+
}
24+
}
25+
26+
impl LocallyDescribableProposalAction for Motion {
27+
const TYPE_NAME: &'static str = "Motion";
28+
const TYPE_DESCRIPTION: &'static str = "A motion is a text that can be adopted or rejected. \
29+
No code is executed when a motion is adopted. An adopted motion should guide the future \
30+
strategy of the Internet Computer ecosystem.";
31+
32+
fn to_value(&self) -> Value {
33+
ValueBuilder::new()
34+
.add_string_field("motion_text".to_string(), self.motion_text.clone())
35+
.build()
36+
}
37+
}
38+
39+
/// A builder for `Value` objects.
40+
pub(crate) struct ValueBuilder {
41+
fields: HashMap<String, Value>,
42+
}
43+
44+
impl ValueBuilder {
45+
pub fn new() -> Self {
46+
Self {
47+
fields: HashMap::new(),
48+
}
49+
}
50+
51+
pub fn add_string_field(mut self, key: String, value: String) -> Self {
52+
self.fields.insert(
53+
key,
54+
Value {
55+
value: Some(Text(value)),
56+
},
57+
);
58+
self
59+
}
60+
61+
pub fn build(self) -> Value {
62+
let Self { fields } = self;
63+
Value {
64+
value: Some(Map(ValueMap { values: fields })),
65+
}
66+
}
67+
}
68+
69+
#[path = "self_describing_tests.rs"]
70+
#[cfg(test)]
71+
pub mod tests;
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
use super::*;
2+
3+
use ic_nns_governance_api::Value as ApiValue;
4+
use maplit::hashmap;
5+
6+
fn assert_self_describing_value_is(
7+
action: impl LocallyDescribableProposalAction,
8+
expected: ApiValue,
9+
) {
10+
let SelfDescribingProposalAction { value, .. } = action.to_self_describing();
11+
// Use ApiValue for testing because: (1) it should only be used for API responses, and (2) it's
12+
// more straightforward to construct (while the protobuf type only exists for storage).
13+
let value = ApiValue::from(value.unwrap());
14+
assert_eq!(value, expected);
15+
}
16+
17+
#[test]
18+
fn test_motion_to_self_describing() {
19+
let motion = Motion {
20+
motion_text: "This is a motion".to_string(),
21+
};
22+
assert_self_describing_value_is(
23+
motion,
24+
ApiValue::Map(hashmap! {
25+
"motion_text".to_string() => ApiValue::Text("This is a motion".to_string()),
26+
}),
27+
);
28+
}

0 commit comments

Comments
 (0)