diff --git a/rs/nns/governance/api/src/types.rs b/rs/nns/governance/api/src/types.rs index d6108d532d31..b15f9b960aef 100644 --- a/rs/nns/governance/api/src/types.rs +++ b/rs/nns/governance/api/src/types.rs @@ -4567,6 +4567,7 @@ pub enum SelfDescribingValue { Text(String), Nat(Nat), Int(Int), + Null, Array(Vec), Map(HashMap), } diff --git a/rs/nns/governance/canister/governance.did b/rs/nns/governance/canister/governance.did index 9e9c053ba045..cf779e6ef412 100644 --- a/rs/nns/governance/canister/governance.did +++ b/rs/nns/governance/canister/governance.did @@ -1480,6 +1480,7 @@ type SelfDescribingValue = variant { Int : int; Array : vec SelfDescribingValue; Map : vec record { text; SelfDescribingValue }; + Null; }; type GetPendingProposalsRequest = record { diff --git a/rs/nns/governance/derive_self_describing/src/lib.rs b/rs/nns/governance/derive_self_describing/src/lib.rs index c2a385fb6cc8..d4ada48ec084 100644 --- a/rs/nns/governance/derive_self_describing/src/lib.rs +++ b/rs/nns/governance/derive_self_describing/src/lib.rs @@ -188,11 +188,11 @@ fn derive_mixed_enum(name: &Ident, data_enum: &DataEnum) -> Result { - // Unit variant in mixed enum: map with variant name as key, empty array as value + // Unit variant in mixed enum: map with variant name as key, null value as value Ok(quote! { #name::#variant_name => { crate::proposals::self_describing::ValueBuilder::new() - .add_empty_field(#variant_name_str) + .add_field(#variant_name_str, crate::proposals::self_describing::SelfDescribingValue::NULL) .build() } }) diff --git a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto index d406ab9f9db9..5ef8a6938c36 100644 --- a/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto +++ b/rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto @@ -2651,6 +2651,7 @@ message SelfDescribingValue { bytes int = 4; SelfDescribingValueArray array = 5; SelfDescribingValueMap map = 6; + Empty null = 7; } } diff --git a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs index 2fb2b7e60043..b7795d5807a1 100644 --- a/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs @@ -4138,7 +4138,7 @@ pub struct FinalizeDisburseMaturity { ::prost::Message, )] pub struct SelfDescribingValue { - #[prost(oneof = "self_describing_value::Value", tags = "1, 2, 3, 4, 5, 6")] + #[prost(oneof = "self_describing_value::Value", tags = "1, 2, 3, 4, 5, 6, 7")] pub value: ::core::option::Option, } /// Nested message and enum types in `SelfDescribingValue`. @@ -4166,6 +4166,8 @@ pub mod self_describing_value { Array(super::SelfDescribingValueArray), #[prost(message, tag = "6")] Map(super::SelfDescribingValueMap), + #[prost(message, tag = "7")] + Null(super::Empty), } } #[derive( diff --git a/rs/nns/governance/src/pb/conversions/mod.rs b/rs/nns/governance/src/pb/conversions/mod.rs index 2d9292fe1a6a..61c21285f491 100644 --- a/rs/nns/governance/src/pb/conversions/mod.rs +++ b/rs/nns/governance/src/pb/conversions/mod.rs @@ -4202,6 +4202,7 @@ impl From for pb_api::SelfDescribingValue { .map(|(k, v)| (k, Self::from(v))) .collect(), ), + pb::self_describing_value::Value::Null(_) => Self::Null, } } } @@ -4231,6 +4232,9 @@ impl From for pb::SelfDescribingValue { values: v.into_iter().map(|(k, v)| (k, Self::from(v))).collect(), }) } + pb_api::SelfDescribingValue::Null => { + pb::self_describing_value::Value::Null(pb::Empty {}) + } }; Self { value: Some(value) } } diff --git a/rs/nns/governance/src/proposals/add_or_remove_node_provider_tests.rs b/rs/nns/governance/src/proposals/add_or_remove_node_provider_tests.rs index 1ac9a85b8557..b5e0d606757f 100644 --- a/rs/nns/governance/src/proposals/add_or_remove_node_provider_tests.rs +++ b/rs/nns/governance/src/proposals/add_or_remove_node_provider_tests.rs @@ -281,9 +281,7 @@ fn test_to_self_describing_value() { SelfDescribingValue::Map(hashmap! { "ToAdd".to_string() => SelfDescribingValue::Map(hashmap! { "id".to_string() => SelfDescribingValue::Text("6fyp7-3ibaa-aaaaa-aaaap-4ai".to_string()), - "reward_account".to_string() => SelfDescribingValue::Array(vec![ - SelfDescribingValue::Text(account.to_hex()) - ]) + "reward_account".to_string() => SelfDescribingValue::Text(account.to_hex()) }) }) ); diff --git a/rs/nns/governance/src/proposals/deregister_known_neuron.rs b/rs/nns/governance/src/proposals/deregister_known_neuron.rs index e6927d2a46bb..e492eb1379ee 100644 --- a/rs/nns/governance/src/proposals/deregister_known_neuron.rs +++ b/rs/nns/governance/src/proposals/deregister_known_neuron.rs @@ -60,7 +60,7 @@ impl LocallyDescribableProposalAction for DeregisterKnownNeuron { fn to_self_describing_value(&self) -> SelfDescribingValue { ValueBuilder::new() - .add_field_with_empty_as_fallback("neuron_id", self.id.map(|id| id.id)) + .add_field("neuron_id", self.id.map(|id| id.id)) .build() } } diff --git a/rs/nns/governance/src/proposals/install_code.rs b/rs/nns/governance/src/proposals/install_code.rs index 64c1c89194dd..152c40b04802 100644 --- a/rs/nns/governance/src/proposals/install_code.rs +++ b/rs/nns/governance/src/proposals/install_code.rs @@ -187,10 +187,10 @@ impl LocallyDescribableProposalAction for InstallCode { let install_mode = install_mode.map(SelfDescribingProstEnum::::new); ValueBuilder::new() - .add_field_with_empty_as_fallback("canister_id", *canister_id) - .add_field_with_empty_as_fallback("install_mode", install_mode) - .add_field_with_empty_as_fallback("wasm_module_hash", wasm_module_hash.clone()) - .add_field_with_empty_as_fallback("arg_hash", arg_hash.clone()) + .add_field("canister_id", *canister_id) + .add_field("install_mode", install_mode) + .add_field("wasm_module_hash", wasm_module_hash.clone()) + .add_field("arg_hash", arg_hash.clone()) .add_field( "skip_stopping_before_installing", skip_stopping_before_installing.unwrap_or_default(), diff --git a/rs/nns/governance/src/proposals/manage_neuron.rs b/rs/nns/governance/src/proposals/manage_neuron.rs index b9b164bba3d2..2ade0599f5da 100644 --- a/rs/nns/governance/src/proposals/manage_neuron.rs +++ b/rs/nns/governance/src/proposals/manage_neuron.rs @@ -51,13 +51,11 @@ impl LocallyDescribableProposalAction for ManageNeuron { "A ManageNeuron proposal is created with an empty or conflicting \ values of id and neuron_id_or_subaccount. This should never happen." ); - builder.add_empty_field("neuron_id_or_subaccount") + builder.add_field("neuron_id_or_subaccount", SelfDescribingValue::NULL) } }; - builder - .add_field_with_empty_as_fallback("command", self.command.clone()) - .build() + builder.add_field("command", self.command.clone()).build() } } @@ -99,13 +97,13 @@ impl From for SelfDescribingValue { println!( "A ManageNeuron proposal is created with a MergeMaturity command. This should never happen." ); - Self::singleton_map("MergeMaturity", Self::EMPTY) + Self::singleton_map("MergeMaturity", Self::NULL) } C::MakeProposal(_) => { println!( "A ManageNeuron proposal is created with a MakeProposal command. This should never happen." ); - Self::singleton_map("MakeProposal", Self::EMPTY) + Self::singleton_map("MakeProposal", Self::NULL) } } } @@ -117,7 +115,7 @@ impl From for SelfDescribingValue { println!( "A ManageNeuron proposal is created with an empty operation. This should never happen." ); - return Self::EMPTY; + return Self::NULL; }; use Operation as O; @@ -161,14 +159,14 @@ impl From for SelfDescribingValue { impl From for SelfDescribingValue { fn from(value: StartDissolving) -> Self { let StartDissolving {} = value; - SelfDescribingValue::EMPTY + SelfDescribingValue::NULL } } impl From for SelfDescribingValue { fn from(value: StopDissolving) -> Self { let StopDissolving {} = value; - SelfDescribingValue::EMPTY + SelfDescribingValue::NULL } } @@ -198,14 +196,14 @@ impl From for SelfDescribingValue { impl From for SelfDescribingValue { fn from(value: JoinCommunityFund) -> Self { let JoinCommunityFund {} = value; - SelfDescribingValue::EMPTY + SelfDescribingValue::NULL } } impl From for SelfDescribingValue { fn from(value: LeaveCommunityFund) -> Self { let LeaveCommunityFund {} = value; - SelfDescribingValue::EMPTY + SelfDescribingValue::NULL } } @@ -320,7 +318,7 @@ impl From for SelfDescribingValue { println!( "A ManageNeuron proposal is created with an empty by. This should never happen." ); - return Self::EMPTY; + return Self::NULL; }; match by { @@ -359,7 +357,7 @@ impl From for SelfDescribingValue { impl From for SelfDescribingValue { fn from(value: RefreshVotingPower) -> Self { let RefreshVotingPower {} = value; - SelfDescribingValue::EMPTY + SelfDescribingValue::NULL } } diff --git a/rs/nns/governance/src/proposals/manage_neuron_tests.rs b/rs/nns/governance/src/proposals/manage_neuron_tests.rs index bc22905c2669..cd34d2e46487 100644 --- a/rs/nns/governance/src/proposals/manage_neuron_tests.rs +++ b/rs/nns/governance/src/proposals/manage_neuron_tests.rs @@ -10,7 +10,7 @@ use crate::{ use ic_base_types::PrincipalId; use ic_nns_common::pb::v1::{NeuronId, ProposalId}; -use ic_nns_governance_api::SelfDescribingValue::{self, Array, Blob, Map, Nat, Text}; +use ic_nns_governance_api::SelfDescribingValue::{self, Array, Blob, Map, Nat, Null, Text}; use icp_ledger::protobuf::AccountIdentifier; use maplit::hashmap; @@ -42,7 +42,7 @@ fn test_manage_neuron_to_self_describing_with_neuron_id() { Map(hashmap! { "neuron_id".to_string() => Nat(candid::Nat::from(123_u64)), "command".to_string() => Map(hashmap! { - "RefreshVotingPower".to_string() => Array(vec![]), + "RefreshVotingPower".to_string() => Null, }), }), ); @@ -60,7 +60,7 @@ fn test_manage_neuron_to_self_describing_with_subaccount() { Map(hashmap! { "subaccount".to_string() => Blob(subaccount), "command".to_string() => Map(hashmap! { - "RefreshVotingPower".to_string() => Array(vec![]), + "RefreshVotingPower".to_string() => Null, }), }), ); @@ -77,7 +77,7 @@ fn test_manage_neuron_to_self_describing_with_legacy_id() { Map(hashmap! { "neuron_id".to_string() => Nat(candid::Nat::from(456_u64)), "command".to_string() => Map(hashmap! { - "RefreshVotingPower".to_string() => Array(vec![]), + "RefreshVotingPower".to_string() => Null, }), }), ); @@ -88,7 +88,7 @@ fn test_command_refresh_voting_power_to_self_describing() { assert_command_self_describing_value_is( Command::RefreshVotingPower(RefreshVotingPower {}), Map(hashmap! { - "RefreshVotingPower".to_string() => Array(vec![]), + "RefreshVotingPower".to_string() => Null, }), ); } @@ -116,7 +116,7 @@ fn test_command_configure_start_dissolving_to_self_describing() { operation: Some(Operation::StartDissolving(StartDissolving {})), }), Map(hashmap! { - "StartDissolving".to_string() => Array(vec![]), + "StartDissolving".to_string() => Null, }), ); } @@ -128,7 +128,7 @@ fn test_command_configure_stop_dissolving_to_self_describing() { operation: Some(Operation::StopDissolving(StopDissolving {})), }), Map(hashmap! { - "StopDissolving".to_string() => Array(vec![]), + "StopDissolving".to_string() => Null, }), ); } @@ -144,9 +144,7 @@ fn test_command_configure_add_hot_key_to_self_describing() { }), Map(hashmap! { "AddHotKey".to_string() => Map(hashmap! { - "new_hot_key".to_string() => Array(vec![ - Text(principal.to_string()), - ]), + "new_hot_key".to_string() => Text(principal.to_string()), }), }), ); @@ -163,12 +161,8 @@ fn test_command_disburse_to_self_describing() { }), Map(hashmap! { "Disburse".to_string() => Map(hashmap! { - "amount_e8s".to_string() => Array(vec![ - Nat(candid::Nat::from(100_000_000_u64)), - ]), - "to_account".to_string() => Array(vec![ - Blob(vec![1_u8; 28]), - ]), + "amount_e8s".to_string() => Nat(candid::Nat::from(100_000_000_u64)), + "to_account".to_string() => Blob(vec![1_u8; 28]), }), }), ); @@ -184,9 +178,7 @@ fn test_command_split_to_self_describing() { Map(hashmap! { "Split".to_string() => Map(hashmap! { "amount_e8s".to_string() => Nat(candid::Nat::from(50_000_000_u64)), - "memo".to_string() => Array(vec![ - Nat(candid::Nat::from(12_345_u64)), - ]), + "memo".to_string() => Nat(candid::Nat::from(12_345_u64)), }), }), ); @@ -220,9 +212,7 @@ fn test_command_register_vote_to_self_describing() { }), Map(hashmap! { "RegisterVote".to_string() => Map(hashmap! { - "proposal".to_string() => Array(vec![ - Nat(candid::Nat::from(42_u64)), - ]), + "proposal".to_string() => Nat(candid::Nat::from(42_u64)), "vote".to_string() => Text("Yes".to_string()), }), }), @@ -243,13 +233,11 @@ fn test_command_disburse_maturity_with_to_account_to_self_describing() { Map(hashmap! { "DisburseMaturity".to_string() => Map(hashmap! { "percentage_to_disburse".to_string() => Nat(candid::Nat::from(25_u32)), - "to_account".to_string() => Array(vec![ - Map(hashmap! { - "owner".to_string() => Text(PrincipalId::new_user_test_id(1).to_string()), - "subaccount".to_string() => Array(vec![]), - }), - ]), - "to_account_identifier".to_string() => Array(vec![]), + "to_account".to_string() => Map(hashmap! { + "owner".to_string() => Text(PrincipalId::new_user_test_id(1).to_string()), + "subaccount".to_string() => Null, + }), + "to_account_identifier".to_string() => Null, }), }), ); @@ -268,10 +256,8 @@ fn test_command_disburse_maturity_with_to_account_identifier_to_self_describing( Map(hashmap! { "DisburseMaturity".to_string() => Map(hashmap! { "percentage_to_disburse".to_string() => Nat(candid::Nat::from(50_u32)), - "to_account".to_string() => Array(vec![]), - "to_account_identifier".to_string() => Array(vec![ - Blob(vec![2_u8; 28]), - ]), + "to_account".to_string() => Null, + "to_account_identifier".to_string() => Blob(vec![2_u8; 28]), }), }), ); @@ -288,9 +274,7 @@ fn test_command_configure_remove_hot_key_to_self_describing() { }), Map(hashmap! { "RemoveHotKey".to_string() => Map(hashmap! { - "hot_key_to_remove".to_string() => Array(vec![ - Text(principal.to_string()), - ]), + "hot_key_to_remove".to_string() => Text(principal.to_string()), }), }), ); @@ -319,7 +303,7 @@ fn test_command_configure_join_community_fund_to_self_describing() { operation: Some(Operation::JoinCommunityFund(JoinCommunityFund {})), }), Map(hashmap! { - "JoinCommunityFund".to_string() => Array(vec![]), + "JoinCommunityFund".to_string() => Null, }), ); } @@ -331,7 +315,7 @@ fn test_command_configure_leave_community_fund_to_self_describing() { operation: Some(Operation::LeaveCommunityFund(LeaveCommunityFund {})), }), Map(hashmap! { - "LeaveCommunityFund".to_string() => Array(vec![]), + "LeaveCommunityFund".to_string() => Null, }), ); } @@ -364,9 +348,7 @@ fn test_command_configure_set_visibility_to_self_describing() { }), Map(hashmap! { "SetVisibility".to_string() => Map(hashmap! { - "visibility".to_string() => Array(vec![ - Text("Public".to_string()), - ]), + "visibility".to_string() => Text("Public".to_string()), }), }), ); @@ -383,15 +365,9 @@ fn test_command_spawn_to_self_describing() { }), Map(hashmap! { "Spawn".to_string() => Map(hashmap! { - "new_controller".to_string() => Array(vec![ - Text(principal.to_string()), - ]), - "nonce".to_string() => Array(vec![ - Nat(candid::Nat::from(999_u64)), - ]), - "percentage_to_spawn".to_string() => Array(vec![ - Nat(candid::Nat::from(50_u32)), - ]), + "new_controller".to_string() => Text(principal.to_string()), + "nonce".to_string() => Nat(candid::Nat::from(999_u64)), + "percentage_to_spawn".to_string() => Nat(candid::Nat::from(50_u32)), }), }), ); @@ -410,9 +386,7 @@ fn test_command_disburse_to_neuron_to_self_describing() { }), Map(hashmap! { "DisburseToNeuron".to_string() => Map(hashmap! { - "new_controller".to_string() => Array(vec![ - Text(principal.to_string()), - ]), + "new_controller".to_string() => Text(principal.to_string()), "amount_e8s".to_string() => Nat(candid::Nat::from(200_000_000_u64)), "dissolve_delay_seconds".to_string() => Nat(candid::Nat::from(31536000_u64)), "kyc_verified".to_string() => Nat(candid::Nat::from(1_u8)), @@ -453,9 +427,7 @@ fn test_command_claim_or_refresh_memo_and_controller_to_self_describing() { "ClaimOrRefresh".to_string() => Map(hashmap! { "By".to_string() => Text("MemoAndController".to_string()), "memo".to_string() => Nat(candid::Nat::from(98_765_u32)), - "controller".to_string() => Array(vec![ - Text(principal.to_string()), - ]), + "controller".to_string() => Text(principal.to_string()), }), }), ); @@ -483,9 +455,7 @@ fn test_command_merge_to_self_describing() { }), Map(hashmap! { "Merge".to_string() => Map(hashmap! { - "source_neuron_id".to_string() => Array(vec![ - Nat(candid::Nat::from(1_700_u64)), - ]), + "source_neuron_id".to_string() => Nat(candid::Nat::from(1_700_u64)), }), }), ); @@ -499,9 +469,7 @@ fn test_command_stake_maturity_to_self_describing() { }), Map(hashmap! { "StakeMaturity".to_string() => Map(hashmap! { - "percentage_to_stake".to_string() => Array(vec![ - Nat(candid::Nat::from(100_u32)), - ]), + "percentage_to_stake".to_string() => Nat(candid::Nat::from(100_u32)), }), }), ); @@ -525,18 +493,14 @@ fn test_command_set_following_to_self_describing() { Map(hashmap! { "SetFollowing".to_string() => Array(vec![ Map(hashmap! { - "topic".to_string() => Array(vec![ - Text("Governance".to_string()), - ]), + "topic".to_string() => Text("Governance".to_string()), "followees".to_string() => Array(vec![ Nat(candid::Nat::from(3_u64)), Nat(candid::Nat::from(4_u64)), ]), }), Map(hashmap! { - "topic".to_string() => Array(vec![ - Text("SnsAndCommunityFund".to_string()), - ]), + "topic".to_string() => Text("SnsAndCommunityFund".to_string()), "followees".to_string() => Array(vec![ Nat(candid::Nat::from(5_u64)), ]), diff --git a/rs/nns/governance/src/proposals/register_known_neuron.rs b/rs/nns/governance/src/proposals/register_known_neuron.rs index 0b595542c2d0..008869b4bc63 100644 --- a/rs/nns/governance/src/proposals/register_known_neuron.rs +++ b/rs/nns/governance/src/proposals/register_known_neuron.rs @@ -185,8 +185,8 @@ impl LocallyDescribableProposalAction for KnownNeuron { fn to_self_describing_value(&self) -> SelfDescribingValue { ValueBuilder::new() - .add_field_with_empty_as_fallback("neuron_id", self.id.map(|id| id.id)) - .add_field_with_empty_as_fallback("known_neuron_data", self.known_neuron_data.clone()) + .add_field("neuron_id", self.id.map(|id| id.id)) + .add_field("known_neuron_data", self.known_neuron_data.clone()) .build() } } diff --git a/rs/nns/governance/src/proposals/register_known_neuron_tests.rs b/rs/nns/governance/src/proposals/register_known_neuron_tests.rs index b923be6a133b..fd428ce2d94e 100644 --- a/rs/nns/governance/src/proposals/register_known_neuron_tests.rs +++ b/rs/nns/governance/src/proposals/register_known_neuron_tests.rs @@ -522,9 +522,7 @@ fn test_known_neuron_to_self_describing() { "neuron_id".to_string() => SelfDescribingValue::Nat(candid::Nat::from(123u64)), "known_neuron_data".to_string() => SelfDescribingValue::Map(hashmap! { "name".to_string() => SelfDescribingValue::Text("Test Neuron".to_string()), - "description".to_string() => SelfDescribingValue::Array(vec![ - SelfDescribingValue::Text("Description".to_string()) - ]), + "description".to_string() => SelfDescribingValue::Text("Description".to_string()), "links".to_string() => SelfDescribingValue::Array(vec![ SelfDescribingValue::Text("https://test.com".to_string()) ]), @@ -557,7 +555,7 @@ fn test_known_neuron_to_self_describing_empty_fields() { "neuron_id".to_string() => SelfDescribingValue::Nat(candid::Nat::from(123u64)), "known_neuron_data".to_string() => SelfDescribingValue::Map(hashmap! { "name".to_string() => SelfDescribingValue::Text("Test Neuron".to_string()), - "description".to_string() => SelfDescribingValue::Array(vec![]), + "description".to_string() => SelfDescribingValue::Null, "links".to_string() => SelfDescribingValue::Array(vec![]), "committed_topics".to_string() => SelfDescribingValue::Array(vec![]), }), diff --git a/rs/nns/governance/src/proposals/self_describing.rs b/rs/nns/governance/src/proposals/self_describing.rs index cc95b164a7a1..42af3016271e 100644 --- a/rs/nns/governance/src/proposals/self_describing.rs +++ b/rs/nns/governance/src/proposals/self_describing.rs @@ -1,5 +1,5 @@ use crate::pb::v1::{ - Account, ApproveGenesisKyc, Motion, NetworkEconomics, SelfDescribingProposalAction, + Account, ApproveGenesisKyc, Empty, Motion, NetworkEconomics, SelfDescribingProposalAction, SelfDescribingValue, SelfDescribingValueArray, SelfDescribingValueMap, self_describing_value::Value::{self, Array, Blob, Map, Text}, }; @@ -88,32 +88,6 @@ impl ValueBuilder { self } - /// Adds a field with an empty array value. This is useful for fields that don't have a meaningful - /// payload (e.g., StartDissolving, StopDissolving). - pub fn add_empty_field(self, key: impl ToString) -> Self { - self.add_field(key, SelfDescribingValue::EMPTY) - } - - /// Given an `value: Option`, if `value` is `Some(inner)`, add the `inner` to the builder. If - /// `value` is `None`, add an empty array to the builder. This is useful for cases where a field - /// is designed to be required, while we want to still add an empty field to the builder in case - /// of a bug. - pub fn add_field_with_empty_as_fallback( - self, - key: impl ToString, - value: Option>, - ) -> Self { - if let Some(value) = value { - self.add_field(key, value) - } else { - println!( - "A field {} is added with an empty value while we think it should be impossible", - key.to_string() - ); - self.add_empty_field(key) - } - } - pub fn build(self) -> SelfDescribingValue { let Self { fields } = self; SelfDescribingValue { @@ -197,10 +171,10 @@ where SelfDescribingValue: From, { fn from(value: Option) -> Self { - SelfDescribingValue { - value: Some(Array(SelfDescribingValueArray { - values: value.into_iter().map(SelfDescribingValue::from).collect(), - })), + if let Some(value) = value { + SelfDescribingValue::from(value) + } else { + SelfDescribingValue::NULL } } } @@ -285,15 +259,15 @@ impl From for SelfDescribingValue { let Account { owner, subaccount } = account; let subaccount = subaccount.map(|subaccount| subaccount.subaccount); ValueBuilder::new() - .add_field_with_empty_as_fallback("owner", owner) + .add_field("owner", owner) .add_field("subaccount", subaccount) .build() } } impl SelfDescribingValue { - pub const EMPTY: Self = Self { - value: Some(Array(SelfDescribingValueArray { values: vec![] })), + pub const NULL: Self = Self { + value: Some(Value::Null(Empty {})), }; pub fn singleton_map(key: impl ToString, value: impl Into) -> Self { diff --git a/rs/nns/governance/src/proposals/self_describing_tests.rs b/rs/nns/governance/src/proposals/self_describing_tests.rs index ce99fa885bdf..b32440e77e63 100644 --- a/rs/nns/governance/src/proposals/self_describing_tests.rs +++ b/rs/nns/governance/src/proposals/self_describing_tests.rs @@ -96,42 +96,36 @@ fn test_network_economics_to_self_describing_all_fields() { "max_proposals_to_keep_per_topic".to_string() => Nat(candid::Nat::from(100_u32)), "neurons_fund_economics".to_string() => - Array(vec![ - Map(hashmap! { - "max_theoretical_neurons_fund_participation_amount_xdr".to_string() => - Array(vec![Text("750_000.0".to_string())]), - "neurons_fund_matched_funding_curve_coefficients".to_string() => - Array(vec![ - Map(hashmap! { - "contribution_threshold_xdr".to_string() => - Array(vec![Text("75_000.0".to_string())]), - "one_third_participation_milestone_xdr".to_string() => - Array(vec![Text("225_000.0".to_string())]), - "full_participation_milestone_xdr".to_string() => - Array(vec![Text("375_000.0".to_string())]), - }), - ]), - "minimum_icp_xdr_rate".to_string() => - Array(vec![Map(hashmap! { - "basis_points".to_string() => Nat(candid::Nat::from(10000_u64)), - })]), - "maximum_icp_xdr_rate".to_string() => - Array(vec![Map(hashmap! { - "basis_points".to_string() => Nat(candid::Nat::from(1000000_u64)), - })]), - }), - ]), + Map(hashmap! { + "max_theoretical_neurons_fund_participation_amount_xdr".to_string() => + Text("750_000.0".to_string()), + "neurons_fund_matched_funding_curve_coefficients".to_string() => + Map(hashmap! { + "contribution_threshold_xdr".to_string() => + Text("75_000.0".to_string()), + "one_third_participation_milestone_xdr".to_string() => + Text("225_000.0".to_string()), + "full_participation_milestone_xdr".to_string() => + Text("375_000.0".to_string()), + }), + "minimum_icp_xdr_rate".to_string() => + Map(hashmap! { + "basis_points".to_string() => Nat(candid::Nat::from(10000_u64)), + }), + "maximum_icp_xdr_rate".to_string() => + Map(hashmap! { + "basis_points".to_string() => Nat(candid::Nat::from(1000000_u64)), + }), + }), "voting_power_economics".to_string() => - Array(vec![ - Map(hashmap! { - "start_reducing_voting_power_after_seconds".to_string() => - Array(vec![Nat(candid::Nat::from(15_778_800_u64))]), - "clear_following_after_seconds".to_string() => - Array(vec![Nat(candid::Nat::from(2_629_800_u64))]), - "neuron_minimum_dissolve_delay_to_vote_seconds".to_string() => - Array(vec![Nat(candid::Nat::from(15_778_800_u64))]), - }), - ]), + Map(hashmap! { + "start_reducing_voting_power_after_seconds".to_string() => + Nat(candid::Nat::from(15_778_800_u64)), + "clear_following_after_seconds".to_string() => + Nat(candid::Nat::from(2_629_800_u64)), + "neuron_minimum_dissolve_delay_to_vote_seconds".to_string() => + Nat(candid::Nat::from(15_778_800_u64)), + }), }), ); } @@ -167,9 +161,9 @@ fn test_network_economics_to_self_describing_minimal() { "max_proposals_to_keep_per_topic".to_string() => Nat(candid::Nat::from(100_u32)), "neurons_fund_economics".to_string() => - Array(vec![]), + Null, "voting_power_economics".to_string() => - Array(vec![]), + Null, }), ); } @@ -232,9 +226,7 @@ fn test_derive_named_struct() { }, SelfDescribingValue::Map(hashmap! { "name".to_string() => SelfDescribingValue::Text("test".to_string()), - "count".to_string() => SelfDescribingValue::Array( - vec![SelfDescribingValue::Nat(candid::Nat::from(42u64))] - ), + "count".to_string() => SelfDescribingValue::Nat(candid::Nat::from(42u64)), }), ); } @@ -270,7 +262,7 @@ fn test_derive_mixed_enum_unit_variant() { assert_self_describing_value_is( TestMixedEnum::Empty, SelfDescribingValue::Map(hashmap! { - "Empty".to_string() => SelfDescribingValue::Array(vec![]), + "Empty".to_string() => SelfDescribingValue::Null, }), ); } diff --git a/rs/nns/governance/src/proposals/stop_or_start_canister.rs b/rs/nns/governance/src/proposals/stop_or_start_canister.rs index b28586ac566e..5ea034130aef 100644 --- a/rs/nns/governance/src/proposals/stop_or_start_canister.rs +++ b/rs/nns/governance/src/proposals/stop_or_start_canister.rs @@ -110,8 +110,8 @@ impl LocallyDescribableProposalAction for StopOrStartCanister { let action = action.map(SelfDescribingProstEnum::::new); ValueBuilder::new() - .add_field_with_empty_as_fallback("canister_id", *canister_id) - .add_field_with_empty_as_fallback("action", action) + .add_field("canister_id", *canister_id) + .add_field("action", action) .build() } } diff --git a/rs/nns/governance/src/proposals/update_canister_settings.rs b/rs/nns/governance/src/proposals/update_canister_settings.rs index 0734558ad3b6..086908d407ac 100644 --- a/rs/nns/governance/src/proposals/update_canister_settings.rs +++ b/rs/nns/governance/src/proposals/update_canister_settings.rs @@ -144,8 +144,8 @@ impl LocallyDescribableProposalAction for UpdateCanisterSettings { fn to_self_describing_value(&self) -> SelfDescribingValue { ValueBuilder::new() - .add_field_with_empty_as_fallback("canister_id", self.canister_id) - .add_field_with_empty_as_fallback("settings", self.settings.clone()) + .add_field("canister_id", self.canister_id) + .add_field("settings", self.settings.clone()) .build() } } @@ -426,17 +426,15 @@ mod tests { "canister_id".to_string() => Text(SNS_WASM_CANISTER_ID.get().to_string()), "settings".to_string() => Map(hashmap! { "controllers".to_string() => Array(vec![ - Array(vec![ - Text(ROOT_CANISTER_ID.get().to_string()), - Text(LEDGER_CANISTER_ID.get().to_string()), - ]), + Text(ROOT_CANISTER_ID.get().to_string()), + Text(LEDGER_CANISTER_ID.get().to_string()), ]), - "memory_allocation".to_string() => Array(vec![Nat(candid::Nat::from(1_u64 << 32))]), - "wasm_memory_limit".to_string() => Array(vec![Nat(candid::Nat::from(1_u64 << 31))]), - "wasm_memory_threshold".to_string() => Array(vec![Nat(candid::Nat::from(1_u64 << 30))]), - "compute_allocation".to_string() => Array(vec![Nat(candid::Nat::from(10_u64))]), - "freezing_threshold".to_string() => Array(vec![Nat(candid::Nat::from(100_u64))]), - "log_visibility".to_string() => Array(vec![Text("Public".to_string())]), + "memory_allocation".to_string() => Nat(candid::Nat::from(1_u64 << 32)), + "wasm_memory_limit".to_string() => Nat(candid::Nat::from(1_u64 << 31)), + "wasm_memory_threshold".to_string() => Nat(candid::Nat::from(1_u64 << 30)), + "compute_allocation".to_string() => Nat(candid::Nat::from(10_u64)), + "freezing_threshold".to_string() => Nat(candid::Nat::from(100_u64)), + "log_visibility".to_string() => Text("Public".to_string()), }), }) ); @@ -462,13 +460,13 @@ mod tests { SelfDescribingValue::Map(hashmap! { "canister_id".to_string() => Text(LEDGER_CANISTER_ID.get().to_string()), "settings".to_string() => Map(hashmap! { - "controllers".to_string() => Array(vec![]), - "memory_allocation".to_string() => Array(vec![Nat(candid::Nat::from(1_u64 << 30))]), - "compute_allocation".to_string() => Array(vec![]), - "freezing_threshold".to_string() => Array(vec![]), - "wasm_memory_limit".to_string() => Array(vec![]), - "wasm_memory_threshold".to_string() => Array(vec![]), - "log_visibility".to_string() => Array(vec![]), + "controllers".to_string() => Null, + "memory_allocation".to_string() => Nat(candid::Nat::from(1_u64 << 30)), + "compute_allocation".to_string() => Null, + "freezing_threshold".to_string() => Null, + "wasm_memory_limit".to_string() => Null, + "wasm_memory_threshold".to_string() => Null, + "log_visibility".to_string() => Null, }), }) );