From 51319f0d7a26fcbdcf9ce475db749f83ebdeefae Mon Sep 17 00:00:00 2001 From: Andre Popovitch Date: Fri, 26 Jul 2024 16:24:37 -0500 Subject: [PATCH] Add controller and hotkeys information to ClaimSwapNeuronsRequest --- rs/sns/governance/canister/governance.did | 18 + .../governance/canister/governance_test.did | 18 + .../ic_sns_governance/pb/v1/governance.proto | 49 ++ .../src/gen/ic_sns_governance.pb.v1.rs | 75 +++ rs/sns/governance/src/governance.rs | 215 ++++-- rs/sns/governance/src/types.rs | 612 +++++++++++++++--- rs/sns/governance/tests/governance.rs | 103 +-- rs/sns/swap/src/swap.rs | 1 + rs/sns/swap/tests/swap.rs | 1 + 9 files changed, 905 insertions(+), 187 deletions(-) diff --git a/rs/sns/governance/canister/governance.did b/rs/sns/governance/canister/governance.did index 474db6672c4b..35f4b43253cb 100644 --- a/rs/sns/governance/canister/governance.did +++ b/rs/sns/governance/canister/governance.did @@ -49,6 +49,7 @@ type ChangeAutoStakeMaturity = record { type ClaimOrRefresh = record { by : opt By }; type ClaimOrRefreshResponse = record { refreshed_neuron_id : opt NeuronId }; type ClaimSwapNeuronsRequest = record { + recipes : opt NeuronRecipes; neuron_parameters : vec NeuronParameters; }; type ClaimSwapNeuronsResponse = record { @@ -323,6 +324,7 @@ type Neuron = record { neuron_fees_e8s : nat64; }; type NeuronId = record { id : blob }; +type NeuronIds = record { ids : vec NeuronId }; type NeuronInFlightCommand = record { command : opt Command_2; timestamp : nat64; @@ -341,6 +343,20 @@ type NeuronPermission = record { permission_type : vec int32; }; type NeuronPermissionList = record { permissions : vec int32 }; +type NeuronRecipe = record { + controller : opt principal; + dissolve_delay_seconds : opt nat64; + participant : opt Participant; + stake_e8s : opt nat64; + followees : opt NeuronIds; + neuron_id : opt NeuronId; +}; +type NeuronRecipes = record { recipes : vec NeuronRecipe }; +type NeuronsFund = record { + nns_neuron_hotkeys : opt Principals; + nns_neuron_controller : opt principal; + nns_neuron_id : opt nat64; +}; type Operation = variant { ChangeAutoStakeMaturity : ChangeAutoStakeMaturity; StopDissolving : record {}; @@ -348,7 +364,9 @@ type Operation = variant { IncreaseDissolveDelay : IncreaseDissolveDelay; SetDissolveTimestamp : SetDissolveTimestamp; }; +type Participant = variant { NeuronsFund : NeuronsFund; Direct : record {} }; type Percentage = record { basis_points : opt nat64 }; +type Principals = record { principals : vec principal }; type Proposal = record { url : text; title : text; diff --git a/rs/sns/governance/canister/governance_test.did b/rs/sns/governance/canister/governance_test.did index fa8190149450..80ebbc90b97f 100644 --- a/rs/sns/governance/canister/governance_test.did +++ b/rs/sns/governance/canister/governance_test.did @@ -51,6 +51,7 @@ type ChangeAutoStakeMaturity = record { type ClaimOrRefresh = record { by : opt By }; type ClaimOrRefreshResponse = record { refreshed_neuron_id : opt NeuronId }; type ClaimSwapNeuronsRequest = record { + recipes : opt NeuronRecipes; neuron_parameters : vec NeuronParameters; }; type ClaimSwapNeuronsResponse = record { @@ -329,6 +330,7 @@ type Neuron = record { neuron_fees_e8s : nat64; }; type NeuronId = record { id : blob }; +type NeuronIds = record { ids : vec NeuronId }; type NeuronInFlightCommand = record { command : opt Command_2; timestamp : nat64; @@ -347,6 +349,20 @@ type NeuronPermission = record { permission_type : vec int32; }; type NeuronPermissionList = record { permissions : vec int32 }; +type NeuronRecipe = record { + controller : opt principal; + dissolve_delay_seconds : opt nat64; + participant : opt Participant; + stake_e8s : opt nat64; + followees : opt NeuronIds; + neuron_id : opt NeuronId; +}; +type NeuronRecipes = record { recipes : vec NeuronRecipe }; +type NeuronsFund = record { + nns_neuron_hotkeys : opt Principals; + nns_neuron_controller : opt principal; + nns_neuron_id : opt nat64; +}; type Operation = variant { ChangeAutoStakeMaturity : ChangeAutoStakeMaturity; StopDissolving : record {}; @@ -354,7 +370,9 @@ type Operation = variant { IncreaseDissolveDelay : IncreaseDissolveDelay; SetDissolveTimestamp : SetDissolveTimestamp; }; +type Participant = variant { NeuronsFund : NeuronsFund; Direct : record {} }; type Percentage = record { basis_points : opt nat64 }; +type Principals = record { principals : vec principal }; type Proposal = record { url : text; title : text; diff --git a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto index 9623adf6116b..b7156cdd4cd8 100644 --- a/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto +++ b/rs/sns/governance/proto/ic_sns_governance/pb/v1/governance.proto @@ -68,6 +68,12 @@ message NeuronId { bytes id = 1; } +// A sequence of NeuronIds, which is used to get prost to generate a type isomorphic to Option>. +message NeuronIds { + repeated NeuronId ids = 1; +} + + // The id of a specific proposal. message ProposalId { uint64 id = 1; @@ -1950,6 +1956,7 @@ message GetModeResponse { message ClaimSwapNeuronsRequest { // NeuronParameters groups parameters for creating a neuron in the // `claim_swap_neurons` method. + // TODO(NNS1-3198): Remove this message once `NeuronRecipe` is used systematically. message NeuronParameters { reserved "memo"; reserved 4; @@ -1995,8 +2002,50 @@ message ClaimSwapNeuronsRequest { repeated NeuronId followees = 8; } + // Replacement for NeuronParameters. Contains the information needed to set up + // a neuron for a swap participant. + message NeuronRecipe { + + // The info that for a participant in the Neurons' Fund + message NeuronsFund { + optional uint64 nns_neuron_id = 1; + optional ic_base_types.pb.v1.PrincipalId nns_neuron_controller = 2; + optional ic_nervous_system.pb.v1.Principals nns_neuron_hotkeys = 3; + } + + // The info that for a direct participant + message Direct {} + + // The principal that should be the controller of the SNS neuron + optional ic_base_types.pb.v1.PrincipalId controller = 1; + // The ID of the SNS neuron + optional NeuronId neuron_id = 2; + + // The SNS neuron's stake in e8s (10E-8 of a token) + optional uint64 stake_e8s = 3; + + // The duration in seconds that the neuron's dissolve delay will be set to. + optional uint64 dissolve_delay_seconds = 4; + // + optional NeuronIds followees = 5; + + oneof participant { + Direct direct = 6; + NeuronsFund neurons_fund = 7; + } + } + + // Needed to cause prost to generate a type isomorphic to + // Optional>. + message NeuronRecipes { + repeated NeuronRecipe recipes = 1; + } + + optional NeuronRecipes recipes = 2; + // The set of parameters that define the neurons created in `claim_swap_neurons`. For // each NeuronParameter, one neuron will be created. + // Deprecated. Use [`recipes`] instead. repeated NeuronParameters neuron_parameters = 1; } diff --git a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs index c847bf8c337a..6e5c3a647d45 100644 --- a/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs +++ b/rs/sns/governance/src/gen/ic_sns_governance.pb.v1.rs @@ -20,6 +20,14 @@ pub struct NeuronId { #[serde(with = "serde_bytes")] pub id: ::prost::alloc::vec::Vec, } +/// A sequence of NeuronIds, which is used to get prost to generate a type isomorphic to Option>. +#[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] +#[allow(clippy::derive_partial_eq_without_eq)] +#[derive(Clone, PartialEq, ::prost::Message)] +pub struct NeuronIds { + #[prost(message, repeated, tag = "1")] + pub ids: ::prost::alloc::vec::Vec, +} /// The id of a specific proposal. #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable, Eq, Copy)] #[self_describing] @@ -2510,8 +2518,11 @@ pub struct GetModeResponse { #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct ClaimSwapNeuronsRequest { + #[prost(message, optional, tag = "2")] + pub recipes: ::core::option::Option, /// The set of parameters that define the neurons created in `claim_swap_neurons`. For /// each NeuronParameter, one neuron will be created. + /// Deprecated. Use \[`recipes`\] instead. #[prost(message, repeated, tag = "1")] pub neuron_parameters: ::prost::alloc::vec::Vec, } @@ -2519,6 +2530,7 @@ pub struct ClaimSwapNeuronsRequest { pub mod claim_swap_neurons_request { /// NeuronParameters groups parameters for creating a neuron in the /// `claim_swap_neurons` method. + /// TODO(NNS1-3198): Remove this message once `NeuronRecipe` is used systematically. #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] @@ -2564,6 +2576,69 @@ pub mod claim_swap_neurons_request { #[prost(message, repeated, tag = "8")] pub followees: ::prost::alloc::vec::Vec, } + /// Replacement for NeuronParameters. Contains the information needed to set up + /// a neuron for a swap participant. + #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] + #[allow(clippy::derive_partial_eq_without_eq)] + #[derive(Clone, PartialEq, ::prost::Message)] + pub struct NeuronRecipe { + /// The principal that should be the controller of the SNS neuron + #[prost(message, optional, tag = "1")] + pub controller: ::core::option::Option<::ic_base_types::PrincipalId>, + /// The ID of the SNS neuron + #[prost(message, optional, tag = "2")] + pub neuron_id: ::core::option::Option, + /// The SNS neuron's stake in e8s (10E-8 of a token) + #[prost(uint64, optional, tag = "3")] + pub stake_e8s: ::core::option::Option, + /// The duration in seconds that the neuron's dissolve delay will be set to. + #[prost(uint64, optional, tag = "4")] + pub dissolve_delay_seconds: ::core::option::Option, + /// + #[prost(message, optional, tag = "5")] + pub followees: ::core::option::Option, + #[prost(oneof = "neuron_recipe::Participant", tags = "6, 7")] + pub participant: ::core::option::Option, + } + /// Nested message and enum types in `NeuronRecipe`. + pub mod neuron_recipe { + /// The info that for a participant in the Neurons' Fund + #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] + #[allow(clippy::derive_partial_eq_without_eq)] + #[derive(Clone, PartialEq, ::prost::Message)] + pub struct NeuronsFund { + #[prost(uint64, optional, tag = "1")] + pub nns_neuron_id: ::core::option::Option, + #[prost(message, optional, tag = "2")] + pub nns_neuron_controller: ::core::option::Option<::ic_base_types::PrincipalId>, + #[prost(message, optional, tag = "3")] + pub nns_neuron_hotkeys: + ::core::option::Option<::ic_nervous_system_proto::pb::v1::Principals>, + } + /// The info that for a direct participant + #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] + #[allow(clippy::derive_partial_eq_without_eq)] + #[derive(Clone, PartialEq, ::prost::Message)] + pub struct Direct {} + #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] + #[allow(clippy::derive_partial_eq_without_eq)] + #[derive(Clone, PartialEq, ::prost::Oneof)] + pub enum Participant { + #[prost(message, tag = "6")] + Direct(Direct), + #[prost(message, tag = "7")] + NeuronsFund(NeuronsFund), + } + } + /// Needed to cause prost to generate a type isomorphic to + /// Optional>. + #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] + #[allow(clippy::derive_partial_eq_without_eq)] + #[derive(Clone, PartialEq, ::prost::Message)] + pub struct NeuronRecipes { + #[prost(message, repeated, tag = "1")] + pub recipes: ::prost::alloc::vec::Vec, + } } /// The response for the `claim_swap_neurons` method. #[derive(candid::CandidType, candid::Deserialize, comparable::Comparable)] diff --git a/rs/sns/governance/src/governance.rs b/rs/sns/governance/src/governance.rs index f9bf22b119a3..07ec2bcdd6dc 100644 --- a/rs/sns/governance/src/governance.rs +++ b/rs/sns/governance/src/governance.rs @@ -20,12 +20,13 @@ use crate::{ SetDappControllersResponse, }, v1::{ + claim_swap_neurons_request::NeuronRecipes, claim_swap_neurons_response::SwapNeuron, get_neuron_response, get_proposal_response, governance::{ - self, neuron_in_flight_command, - neuron_in_flight_command::Command as InFlightCommand, MaturityModulation, - NeuronInFlightCommand, SnsMetadata, UpgradeInProgress, Version, + self, + neuron_in_flight_command::{self, Command as InFlightCommand}, + MaturityModulation, NeuronInFlightCommand, SnsMetadata, UpgradeInProgress, Version, }, governance_error::ErrorType, manage_neuron::{ @@ -2907,6 +2908,12 @@ impl Governance { .expect("NervousSystemParameters must have max_followees_per_function") } + fn max_number_of_principals_per_neuron_or_panic(&self) -> u64 { + self.nervous_system_parameters_or_panic() + .max_number_of_principals_per_neuron + .expect("NervousSystemParameters must have max_followees_per_function") + } + /// Inserts a proposals that has already been validated in the state. /// /// This is a low-level function that makes no verification whatsoever. @@ -3873,73 +3880,165 @@ impl Governance { // Safe to do with the validation step above let neuron_minimum_stake_e8s = self.neuron_minimum_stake_e8s_or_panic(); let max_followees_per_function = self.max_followees_per_function_or_panic(); + let max_number_of_principals_per_neuron = + self.max_number_of_principals_per_neuron_or_panic(); let neuron_claimer_permissions = self.neuron_claimer_permissions_or_panic(); let mut swap_neurons = vec![]; - for neuron_parameter in &request.neuron_parameters { - match neuron_parameter.validate(neuron_minimum_stake_e8s, max_followees_per_function) { - Ok(_) => (), - Err(err) => { - log!(ERROR, "Failed to claim Sale Neuron due to {:?}", err); - swap_neurons.push(SwapNeuron::from_neuron_parameters( - neuron_parameter, - ClaimedSwapNeuronStatus::Invalid, - )); - continue; + // `request.neuron_parameters` is deprecated. For now, + // we will support both as long as only one is specified + #[allow(deprecated)] // TODO(NNS1-3198): Remove this once neuron_parameters is removed. + match (request.recipes, &request.neuron_parameters[..]) { + (Some(NeuronRecipes { recipes }), []) => { + for neuron_recipe in recipes { + match neuron_recipe.validate( + neuron_minimum_stake_e8s, + max_followees_per_function, + max_number_of_principals_per_neuron, + ) { + Ok(_) => (), + Err(err) => { + log!(ERROR, "Failed to claim Sale Neuron due to {:?}", err); + swap_neurons.push(SwapNeuron::from_neuron_recipe( + neuron_recipe, + ClaimedSwapNeuronStatus::Invalid, + )); + continue; + } + } + + // Its safe to get all fields in NeuronRecipe because of the previous validation. + let neuron_id = neuron_recipe.get_neuron_id_or_panic(); + + // This neuron was claimed previously. + if self.proto.neurons.contains_key(&neuron_id.to_string()) { + swap_neurons.push(SwapNeuron::from_neuron_recipe( + neuron_recipe, + ClaimedSwapNeuronStatus::AlreadyExists, + )); + continue; + } + + let neuron = Neuron { + id: Some(neuron_id.clone()), + permissions: neuron_recipe + .construct_permissions_or_panic(neuron_claimer_permissions.clone()), + cached_neuron_stake_e8s: neuron_recipe.get_stake_e8s_or_panic(), + neuron_fees_e8s: 0, + created_timestamp_seconds: now, + aging_since_timestamp_seconds: now, + followees: neuron_recipe.construct_followees(), + maturity_e8s_equivalent: 0, + dissolve_state: Some(DissolveState::DissolveDelaySeconds( + neuron_recipe.get_dissolve_delay_seconds_or_panic(), + )), + voting_power_percentage_multiplier: + DEFAULT_VOTING_POWER_PERCENTAGE_MULTIPLIER, + source_nns_neuron_id: neuron_recipe.source_nns_neuron_id(), + staked_maturity_e8s_equivalent: None, + auto_stake_maturity: neuron_recipe.construct_auto_staking_maturity(), + vesting_period_seconds: None, + disburse_maturity_in_progress: vec![], + }; + + // Add the neuron to the various data structures and indexes to support neurons. This + // method may fail if the memory limits of Governance have been reached, which is a + // recoverable error. The sale canister can retry claiming after GC or manual upgrades + // of SNS Governance. + match self.add_neuron(neuron) { + Ok(()) => swap_neurons.push(SwapNeuron::from_neuron_recipe( + neuron_recipe, + ClaimedSwapNeuronStatus::Success, + )), + Err(err) => { + log!(ERROR, "Failed to claim Sale Neuron due to {:?}", err); + swap_neurons.push(SwapNeuron::from_neuron_recipe( + neuron_recipe, + ClaimedSwapNeuronStatus::MemoryExhausted, + )) + } + } } } + // Handle the deprecated neuron_parameters. + // TODO(NNS1-3198): Remove this branch + (None, neuron_parameters) => { + for neuron_parameter in neuron_parameters { + match neuron_parameter + .validate(neuron_minimum_stake_e8s, max_followees_per_function) + { + Ok(_) => (), + Err(err) => { + log!(ERROR, "Failed to claim Sale Neuron due to {:?}", err); + swap_neurons.push(SwapNeuron::from_neuron_parameters( + neuron_parameter, + ClaimedSwapNeuronStatus::Invalid, + )); + continue; + } + } - // Its safe to get all fields in NeuronParameters because of the previous validation. - let neuron_id = neuron_parameter.get_neuron_id_or_panic(); + // Its safe to get all fields in NeuronParameters because of the previous validation. + let neuron_id = neuron_parameter.get_neuron_id_or_panic(); - // This neuron was claimed previously. - if self.proto.neurons.contains_key(&neuron_id.to_string()) { - swap_neurons.push(SwapNeuron::from_neuron_parameters( - neuron_parameter, - ClaimedSwapNeuronStatus::AlreadyExists, - )); - continue; - } + // This neuron was claimed previously. + if self.proto.neurons.contains_key(&neuron_id.to_string()) { + swap_neurons.push(SwapNeuron::from_neuron_parameters( + neuron_parameter, + ClaimedSwapNeuronStatus::AlreadyExists, + )); + continue; + } - let neuron = Neuron { - id: Some(neuron_id.clone()), - permissions: neuron_parameter - .construct_permissions_or_panic(neuron_claimer_permissions.clone()), - cached_neuron_stake_e8s: neuron_parameter.get_stake_e8s_or_panic(), - neuron_fees_e8s: 0, - created_timestamp_seconds: now, - aging_since_timestamp_seconds: now, - followees: neuron_parameter.construct_followees(), - maturity_e8s_equivalent: 0, - dissolve_state: Some(DissolveState::DissolveDelaySeconds( - neuron_parameter.get_dissolve_delay_seconds_or_panic(), - )), - voting_power_percentage_multiplier: DEFAULT_VOTING_POWER_PERCENTAGE_MULTIPLIER, - source_nns_neuron_id: neuron_parameter.source_nns_neuron_id, - staked_maturity_e8s_equivalent: None, - auto_stake_maturity: neuron_parameter.construct_auto_staking_maturity(), - vesting_period_seconds: None, - disburse_maturity_in_progress: vec![], - }; + let neuron = Neuron { + id: Some(neuron_id.clone()), + permissions: neuron_parameter + .construct_permissions_or_panic(neuron_claimer_permissions.clone()), + cached_neuron_stake_e8s: neuron_parameter.get_stake_e8s_or_panic(), + neuron_fees_e8s: 0, + created_timestamp_seconds: now, + aging_since_timestamp_seconds: now, + followees: neuron_parameter.construct_followees(), + maturity_e8s_equivalent: 0, + dissolve_state: Some(DissolveState::DissolveDelaySeconds( + neuron_parameter.get_dissolve_delay_seconds_or_panic(), + )), + voting_power_percentage_multiplier: + DEFAULT_VOTING_POWER_PERCENTAGE_MULTIPLIER, + source_nns_neuron_id: neuron_parameter.source_nns_neuron_id, + staked_maturity_e8s_equivalent: None, + auto_stake_maturity: neuron_parameter.construct_auto_staking_maturity(), + vesting_period_seconds: None, + disburse_maturity_in_progress: vec![], + }; - // Add the neuron to the various data structures and indexes to support neurons. This - // method may fail if the memory limits of Governance have been reached, which is a - // recoverable error. The sale canister can retry claiming after GC or manual upgrades - // of SNS Governance. - match self.add_neuron(neuron) { - Ok(()) => swap_neurons.push(SwapNeuron::from_neuron_parameters( - neuron_parameter, - ClaimedSwapNeuronStatus::Success, - )), - Err(err) => { - log!(ERROR, "Failed to claim Sale Neuron due to {:?}", err); - swap_neurons.push(SwapNeuron::from_neuron_parameters( - neuron_parameter, - ClaimedSwapNeuronStatus::MemoryExhausted, - )) + // Add the neuron to the various data structures and indexes to support neurons. This + // method may fail if the memory limits of Governance have been reached, which is a + // recoverable error. The sale canister can retry claiming after GC or manual upgrades + // of SNS Governance. + match self.add_neuron(neuron) { + Ok(()) => swap_neurons.push(SwapNeuron::from_neuron_parameters( + neuron_parameter, + ClaimedSwapNeuronStatus::Success, + )), + Err(err) => { + log!(ERROR, "Failed to claim Sale Neuron due to {:?}", err); + swap_neurons.push(SwapNeuron::from_neuron_parameters( + neuron_parameter, + ClaimedSwapNeuronStatus::MemoryExhausted, + )) + } + } } } + // If both are set, just fail + (Some(_), [..]) => { + log!(ERROR, "Failed to claim Sale Neuron due to both neuron_parameters and recipes being set in the ClaimSwapNeuronsRequest."); + return ClaimSwapNeuronsResponse { + claim_swap_neurons_result: None, + }; + } } ClaimSwapNeuronsResponse::new(swap_neurons) diff --git a/rs/sns/governance/src/types.rs b/rs/sns/governance/src/types.rs index 0c904b35d43a..f540ed3ee03c 100644 --- a/rs/sns/governance/src/types.rs +++ b/rs/sns/governance/src/types.rs @@ -7,7 +7,10 @@ use crate::{ RegisterDappCanistersRequest, SetDappControllersRequest, }, v1::{ - claim_swap_neurons_request::NeuronParameters, + claim_swap_neurons_request::{ + neuron_recipe::{self, Participant}, + NeuronParameters, NeuronRecipe, NeuronRecipes, + }, claim_swap_neurons_response::{ClaimSwapNeuronsResult, ClaimedSwapNeurons, SwapNeuron}, get_neuron_response, governance::{ @@ -27,10 +30,10 @@ use crate::{ DefaultFollowees, DeregisterDappCanisters, Empty, ExecuteGenericNervousSystemFunction, GovernanceError, ManageDappCanisterSettings, ManageLedgerParameters, ManageNeuronResponse, ManageSnsMetadata, MintSnsTokens, Motion, NervousSystemFunction, - NervousSystemParameters, Neuron, NeuronId, NeuronPermission, NeuronPermissionList, - NeuronPermissionType, ProposalId, RegisterDappCanisters, RewardEvent, - TransferSnsTreasuryFunds, UpgradeSnsControlledCanister, UpgradeSnsToNextVersion, Vote, - VotingRewardsParameters, + NervousSystemParameters, Neuron, NeuronId, NeuronIds, NeuronPermission, + NeuronPermissionList, NeuronPermissionType, ProposalId, RegisterDappCanisters, + RewardEvent, TransferSnsTreasuryFunds, UpgradeSnsControlledCanister, + UpgradeSnsToNextVersion, Vote, VotingRewardsParameters, }, }, proposal::ValidGenericNervousSystemFunction, @@ -2016,6 +2019,19 @@ impl From for ProposalId { } } +impl From> for NeuronIds { + fn from(ids: Vec) -> Self { + NeuronIds { ids } + } +} + +impl From for Vec { + fn from(neuron_ids: NeuronIds) -> Self { + neuron_ids.ids + } +} + +// TODO(NNS1-3198): All of this can be removed once NeuronParameters is no longer used. impl NeuronParameters { pub(crate) fn validate( &self, @@ -2160,6 +2176,250 @@ impl NeuronParameters { } } +impl NeuronRecipe { + pub(crate) fn validate( + &self, + neuron_minimum_stake_e8s: u64, + max_followees_per_function: u64, + max_number_of_principals_per_neuron: u64, + ) -> Result<(), String> { + let mut defects = vec![]; + + let Self { + controller, + neuron_id, + stake_e8s, + dissolve_delay_seconds, + followees, + participant, + } = self; + + if neuron_id.is_none() { + defects.push("Missing neuron_id".to_string()); + } + + if let Some(stake_e8s) = stake_e8s { + if *stake_e8s < neuron_minimum_stake_e8s { + defects.push(format!( + "Provided stake_e8s ({}) is less than the required neuron_minimum_stake_e8s({})", + stake_e8s, neuron_minimum_stake_e8s + )); + } + } else { + defects.push("Missing stake_e8s".to_string()); + } + + if dissolve_delay_seconds.is_none() { + defects.push("Missing dissolve_delay_seconds".to_string()); + } + + if let Some(followees) = followees { + let followees = &followees.ids; + if followees.len() as u64 > max_followees_per_function { + defects.push(format!( + "Provided number of followees ({}) exceeds the maximum \ + number of followees per function ({})", + followees.len(), + max_followees_per_function + )); + } + } else { + defects.push("Missing followees".to_string()); + } + + if controller.is_none() { + defects.push("Missing controller".to_string()); + } + + match participant { + Some(Participant::Direct(_)) => {} + Some(Participant::NeuronsFund(neuron_recipe::NeuronsFund { + nns_neuron_id, + nns_neuron_controller, + nns_neuron_hotkeys, + })) => { + if nns_neuron_id.is_none() { + defects.push("Missing nns_neuron_id for neurons fund participant".to_string()); + } + if nns_neuron_controller.is_none() { + defects.push( + "Missing nns_neuron_controller for neurons fund participant".to_string(), + ); + } + if nns_neuron_hotkeys.is_none() { + defects.push( + "Missing nns_neuron_hotkeys for neurons fund participant".to_string(), + ); + } + } + None => { + defects.push("Missing participant type (Direct or Neurons' Fund)".to_string()); + } + } + + match self.construct_permissions(NeuronPermissionList::default()) { + Ok(permissions) => { + if permissions.len() > max_number_of_principals_per_neuron as usize { + defects.push(format!( + "Number of permissions implied by neuron ({}) exceeds the maximum \ + number of permissions ({})", + permissions.len(), + max_number_of_principals_per_neuron + )); + } + } + Err(e) => { + defects.push(e); + } + } + + if !defects.is_empty() { + let participant_info = match participant { + Some(Participant::Direct(_)) => { + format!("direct participant {:?}", self.controller) + } + Some(Participant::NeuronsFund(nf)) => { + format!("neurons fund participant {:?}", nf.nns_neuron_id) + } + None => "unknown participant".to_string(), + }; + + Err(format!( + "Could not claim neuron for {} with NeuronId {:?} due to: {}", + participant_info, + neuron_id, + defects.join("\n"), + )) + } else { + Ok(()) + } + } + + pub(crate) fn is_community_fund_neuron(&self) -> bool { + matches!(self.participant, Some(Participant::NeuronsFund(_))) + } + + #[track_caller] + pub(crate) fn get_dissolve_delay_seconds_or_panic(&self) -> u64 { + self.dissolve_delay_seconds + .expect("Expected the dissolve_delay_seconds to be present in NeuronRecipe") + } + + #[track_caller] + pub(crate) fn get_stake_e8s_or_panic(&self) -> u64 { + self.stake_e8s + .expect("Expected the stake_e8s to be present in NeuronRecipe") + } + + #[track_caller] + pub(crate) fn get_neuron_id_or_panic(&self) -> &NeuronId { + self.neuron_id + .as_ref() + .expect("Expected NeuronId to be present in NeuronRecipe") + } + + pub(crate) fn source_nns_neuron_id(&self) -> Option { + match &self.participant { + Some(Participant::NeuronsFund(nf)) => nf.nns_neuron_id.as_ref().cloned(), + _ => None, + } + } + + #[track_caller] + pub(crate) fn construct_permissions_or_panic( + &self, + neuron_claimer_permissions: NeuronPermissionList, + ) -> Vec { + self.construct_permissions(neuron_claimer_permissions) + .expect("Failed to construct permissions for neuron") + } + + pub(crate) fn construct_permissions( + &self, + neuron_claimer_permissions: NeuronPermissionList, + ) -> Result, String> { + let mut permissions = vec![]; + + let controller = self + .controller + .as_ref() + .ok_or("Expected controller to be present in NeuronRecipe".to_string())?; + + permissions.push(NeuronPermission::new( + controller, + neuron_claimer_permissions.permissions, + )); + + match &self.participant { + // NeuronsFund participants have additional permissions. + Some(Participant::NeuronsFund(neurons_fund_participant)) => { + let nns_neuron_controller = neurons_fund_participant.nns_neuron_controller.ok_or( + "Expected the nns_neuron_controller to be present for NeuronsFundParticipant" + .to_string(), + )?; + permissions.push(NeuronPermission::new( + &nns_neuron_controller, + vec![ + NeuronPermissionType::ManageVotingPermission as i32, + NeuronPermissionType::SubmitProposal as i32, + NeuronPermissionType::Vote as i32, + ], + )); + + for hotkey in neurons_fund_participant + .nns_neuron_hotkeys + .as_ref() + .ok_or( + "Expected the nns_neuron_hotkeys to be present for NeuronsFundParticipant" + .to_string(), + )? + .principals + .iter() + { + permissions.push(NeuronPermission::new( + hotkey, + vec![ + NeuronPermissionType::SubmitProposal as i32, + NeuronPermissionType::Vote as i32, + ], + )); + } + } + Some(Participant::Direct(_)) => {} + None => return Err("Expected participant to be present in NeuronRecipe".to_string()), + } + + Ok(permissions) + } + + /// Adds `self.followees` entries in `base_followees` that are + /// keyed by `function_ids_to_follow`. + pub(crate) fn construct_followees(&self) -> BTreeMap { + let Some(followees) = &self.followees else { + return BTreeMap::new(); + }; + let followees = &followees.ids; + + if followees.is_empty() { + BTreeMap::new() + } else { + let catch_all = u64::from(&Action::Unspecified(Empty {})); + let followees = Followees { + followees: followees.clone(), + }; + btreemap! { catch_all => followees } + } + } + + pub(crate) fn construct_auto_staking_maturity(&self) -> Option { + if self.is_community_fund_neuron() { + Some(true) + } else { + None + } + } +} + impl ClaimSwapNeuronsResponse { pub(crate) fn new_with_error(error: ClaimSwapNeuronsError) -> Self { ClaimSwapNeuronsResponse { @@ -2186,6 +2446,16 @@ impl SwapNeuron { status: claimed_swap_neuron_status as i32, } } + + pub(crate) fn from_neuron_recipe( + neuron_recipe: NeuronRecipe, + claimed_swap_neuron_status: ClaimedSwapNeuronStatus, + ) -> Self { + SwapNeuron { + id: neuron_recipe.neuron_id.clone(), + status: claimed_swap_neuron_status as i32, + } + } } impl From> for NeuronPermissionList { @@ -2386,6 +2656,20 @@ impl UpgradeSnsControlledCanister { } } +impl From> for NeuronRecipes { + fn from(neuron_recipes: Vec) -> Self { + NeuronRecipes { + recipes: neuron_recipes, + } + } +} + +impl From for Vec { + fn from(neuron_recipes: NeuronRecipes) -> Self { + neuron_recipes.recipes + } +} + pub mod test_helpers { use super::*; use rand::{Rng, RngCore}; @@ -2611,13 +2895,15 @@ pub mod test_helpers { pub(crate) mod tests { use super::*; use crate::pb::v1::{ + claim_swap_neurons_request::neuron_recipe, governance::Mode::PreInitializationSwap, nervous_system_function::{FunctionType, GenericNervousSystemFunction}, neuron::Followees, ExecuteGenericNervousSystemFunction, Proposal, ProposalData, VotingRewardsParameters, }; use ic_base_types::PrincipalId; - use ic_nervous_system_common_test_keys::{TEST_USER1_PRINCIPAL, TEST_USER2_PRINCIPAL}; + use ic_nervous_system_common_test_keys::TEST_USER1_PRINCIPAL; + use ic_nervous_system_proto::pb::v1::Principals; use lazy_static::lazy_static; use maplit::{btreemap, hashset}; use std::convert::TryInto; @@ -3335,83 +3621,185 @@ pub(crate) mod tests { } } - impl NeuronParameters { - fn validate_default() -> Self { + impl NeuronRecipe { + fn validate_default_direct_participant() -> Self { Self { controller: Some(*TEST_USER1_PRINCIPAL), - hotkey: Some(*TEST_USER2_PRINCIPAL), + neuron_id: Some(NeuronId::new_test_neuron_id(0)), stake_e8s: Some(E8S_PER_TOKEN), dissolve_delay_seconds: Some(3 * ONE_MONTH_SECONDS), - source_nns_neuron_id: None, + followees: Some(NeuronIds::from(vec![NeuronId::new_test_neuron_id(1)])), + participant: Some(Participant::Direct(neuron_recipe::Direct {})), + } + } + + fn validate_default_neurons_fund() -> Self { + Self { + controller: Some(PrincipalId::from(CanisterId::from_u64(1185))), // NNS Governance neuron_id: Some(NeuronId::new_test_neuron_id(0)), - followees: vec![NeuronId::new_test_neuron_id(1)], + stake_e8s: Some(E8S_PER_TOKEN), + dissolve_delay_seconds: Some(3 * ONE_MONTH_SECONDS), + followees: Some(NeuronIds::from(vec![NeuronId::new_test_neuron_id(1)])), + participant: Some(Participant::NeuronsFund(neuron_recipe::NeuronsFund { + nns_neuron_id: Some(2), + nns_neuron_controller: Some(PrincipalId::new_user_test_id(13847)), + nns_neuron_hotkeys: Some(Principals::from(vec![ + PrincipalId::new_user_test_id(13848), + PrincipalId::new_user_test_id(13849), + ])), + })), } } } #[test] - fn test_neuron_parameters_validate() { + fn test_neuron_recipe_validate() { let neuron_minimum_stake_e8s = E8S_PER_TOKEN; let max_followees_per_function = 1; + let max_number_of_principals_per_neuron = 5; - // Assert that the default is valid - NeuronParameters::validate_default() - .validate(neuron_minimum_stake_e8s, max_followees_per_function) + // Assert that the defaults are valid + NeuronRecipe::validate_default_direct_participant() + .validate( + neuron_minimum_stake_e8s, + max_followees_per_function, + max_number_of_principals_per_neuron, + ) + .unwrap(); + NeuronRecipe::validate_default_neurons_fund() + .validate( + neuron_minimum_stake_e8s, + max_followees_per_function, + max_number_of_principals_per_neuron, + ) .unwrap(); - let invalid_neuron_parameters = vec![ - NeuronParameters { - controller: None, // No controller specified - ..NeuronParameters::validate_default() + let invalid_neuron_recipes = vec![ + // Common invalid cases + NeuronRecipe { + controller: None, + ..NeuronRecipe::validate_default_direct_participant() }, - NeuronParameters { - stake_e8s: None, // No stake specified - ..NeuronParameters::validate_default() + NeuronRecipe { + neuron_id: None, + ..NeuronRecipe::validate_default_direct_participant() }, - NeuronParameters { - stake_e8s: Some(0), // Stake is less than neuron_minimum_stake_e8s - ..NeuronParameters::validate_default() + NeuronRecipe { + stake_e8s: None, + ..NeuronRecipe::validate_default_direct_participant() }, - NeuronParameters { - neuron_id: None, // No memo specified - ..NeuronParameters::validate_default() + NeuronRecipe { + stake_e8s: Some(neuron_minimum_stake_e8s - 1), + ..NeuronRecipe::validate_default_direct_participant() }, - NeuronParameters { - dissolve_delay_seconds: None, // No dissolve_delay_seconds specified - ..NeuronParameters::validate_default() + NeuronRecipe { + dissolve_delay_seconds: None, + ..NeuronRecipe::validate_default_direct_participant() }, - NeuronParameters { - followees: vec![ + NeuronRecipe { + followees: None, + ..NeuronRecipe::validate_default_direct_participant() + }, + NeuronRecipe { + followees: Some(NeuronIds::from(vec![ NeuronId::new_test_neuron_id(1), NeuronId::new_test_neuron_id(2), - ], - ..NeuronParameters::validate_default() + ])), + ..NeuronRecipe::validate_default_direct_participant() + }, + NeuronRecipe { + participant: None, + ..NeuronRecipe::validate_default_direct_participant() + }, + // NeuronsFund specific invalid cases + NeuronRecipe { + participant: Some(Participant::NeuronsFund(neuron_recipe::NeuronsFund { + nns_neuron_id: None, + nns_neuron_controller: Some(PrincipalId::new_user_test_id(13847)), + nns_neuron_hotkeys: Some(Principals::from(vec![ + PrincipalId::new_user_test_id(13848), + ])), + })), + ..NeuronRecipe::validate_default_neurons_fund() + }, + NeuronRecipe { + participant: Some(Participant::NeuronsFund(neuron_recipe::NeuronsFund { + nns_neuron_id: Some(2), + nns_neuron_controller: None, + nns_neuron_hotkeys: Some(Principals::from(vec![ + PrincipalId::new_user_test_id(13848), + ])), + })), + ..NeuronRecipe::validate_default_neurons_fund() + }, + NeuronRecipe { + participant: Some(Participant::NeuronsFund(neuron_recipe::NeuronsFund { + nns_neuron_id: Some(2), + nns_neuron_controller: Some(PrincipalId::new_user_test_id(13847)), + nns_neuron_hotkeys: None, + })), + ..NeuronRecipe::validate_default_neurons_fund() + }, + NeuronRecipe { + participant: Some(Participant::NeuronsFund(neuron_recipe::NeuronsFund { + nns_neuron_id: Some(2), + nns_neuron_controller: Some(PrincipalId::new_user_test_id(13847)), + nns_neuron_hotkeys: Some(Principals::from(vec![ + PrincipalId::new_user_test_id(13848), + PrincipalId::new_user_test_id(13849), + PrincipalId::new_user_test_id(13810), + PrincipalId::new_user_test_id(13811), + PrincipalId::new_user_test_id(13812), + ])), + })), + ..NeuronRecipe::validate_default_neurons_fund() }, ]; - // Assert all invalid neuron parameters produce an error - for neuron_parameter in invalid_neuron_parameters { - assert!(neuron_parameter - .validate(neuron_minimum_stake_e8s, max_followees_per_function) - .is_err()); + // Assert all invalid neuron recipes produce an error + for (index, neuron_recipe) in invalid_neuron_recipes.iter().enumerate() { + assert!( + neuron_recipe + .validate( + neuron_minimum_stake_e8s, + max_followees_per_function, + max_number_of_principals_per_neuron + ) + .is_err(), + "Test case {} should have failed validation", + index + ); } - let valid_neuron_parameters = vec![ - NeuronParameters { - hotkey: None, // Hotkey can be unspecified - ..NeuronParameters::validate_default() + let valid_neuron_recipes = vec![ + // Valid Direct participant + NeuronRecipe::validate_default_direct_participant(), + // Valid NeuronsFund participant + NeuronRecipe::validate_default_neurons_fund(), + // Edge cases that should still be valid + NeuronRecipe { + dissolve_delay_seconds: Some(0), + ..NeuronRecipe::validate_default_direct_participant() }, - NeuronParameters { - dissolve_delay_seconds: Some(0), // Dissolve delay can be 0 - ..NeuronParameters::validate_default() + NeuronRecipe { + followees: Some(NeuronIds::from(vec![])), + ..NeuronRecipe::validate_default_direct_participant() + }, + NeuronRecipe { + stake_e8s: Some(neuron_minimum_stake_e8s), + ..NeuronRecipe::validate_default_neurons_fund() }, ]; - // Assert all valid neuron parameters produce valid results - for neuron_parameter in valid_neuron_parameters { - neuron_parameter - .validate(neuron_minimum_stake_e8s, max_followees_per_function) - .unwrap_or_else(|err| panic!("Validation failed for {neuron_parameter:#?}: {err}")); + // Assert all valid neuron recipes produce valid results + for (index, neuron_recipe) in valid_neuron_recipes.iter().enumerate() { + neuron_recipe + .validate( + neuron_minimum_stake_e8s, + max_followees_per_function, + max_number_of_principals_per_neuron, + ) + .unwrap_or_else(|err| panic!("Validation failed for test case {}: {}", index, err)); } } @@ -3494,51 +3882,103 @@ pub(crate) mod tests { format!("permissions: [Unspecified, ConfigureDissolveState, ManagePrincipals, SubmitProposal, Vote, Disburse, Split, MergeMaturity, DisburseMaturity, StakeMaturity, ManageVotingPermission, ]") ); } - #[test] - fn test_construct_followees() { + fn test_neuron_recipe_construct_followees() { + // Helper function to create a test signature + let test_signature = |nid: &str| format!("Test followees of {nid}"); + + // Create some test NeuronIds let b0 = NeuronId::new_test_neuron_id(10); - let p0 = NeuronParameters { - followees: vec![], - neuron_id: Some(b0.clone()), - ..NeuronParameters::validate_default() - }; let b1 = NeuronId::new_test_neuron_id(11); - let p1 = NeuronParameters { - followees: vec![b0.clone()], - neuron_id: Some(b1), - ..NeuronParameters::validate_default() - }; let b2 = NeuronId::new_test_neuron_id(12); - let p2 = NeuronParameters { - followees: vec![b0.clone()], - neuron_id: Some(b2), - ..NeuronParameters::validate_default() - }; + let b3 = NeuronId::new_test_neuron_id(13); + + // Action for followees let w = u64::from(&Action::Unspecified(Empty {})); - { - let test_signature = |nid: &str| format!("Test followees of {nid}"); - assert_eq!( - p0.construct_followees(), + + // Test cases + let test_cases = vec![ + // Case 1: Empty followees list (Direct participant) + ( + NeuronRecipe { + followees: Some(NeuronIds::from(vec![])), + neuron_id: Some(b0.clone()), + ..NeuronRecipe::validate_default_direct_participant() + }, btreemap! {}, - "{}", - test_signature("b0") - ); - assert_eq!( - p1.construct_followees(), + "b0 (Direct, empty followees)", + ), + // Case 2: Single followee (Direct participant) + ( + NeuronRecipe { + followees: Some(NeuronIds::from(vec![b0.clone()])), + neuron_id: Some(b1.clone()), + ..NeuronRecipe::validate_default_direct_participant() + }, btreemap! { w => Followees { followees: vec![b0.clone()] }, }, - "{}", - test_signature("b1") - ); - assert_eq!( - p2.construct_followees(), + "b1 (Direct, single followee)", + ), + // Case 3: Multiple followees (Direct participant) + ( + NeuronRecipe { + followees: Some(NeuronIds::from(vec![b0.clone(), b1.clone()])), + neuron_id: Some(b2.clone()), + ..NeuronRecipe::validate_default_direct_participant() + }, + btreemap! { + w => Followees { followees: vec![b0.clone(), b1.clone()] }, + }, + "b2 (Direct, multiple followees)", + ), + // Case 4: Empty followees list (NeuronsFund participant) + ( + NeuronRecipe { + followees: Some(NeuronIds::from(vec![])), + neuron_id: Some(b3.clone()), + ..NeuronRecipe::validate_default_neurons_fund() + }, + btreemap! {}, + "b3 (NeuronsFund, empty followees)", + ), + // Case 5: Single followee (NeuronsFund participant) + ( + NeuronRecipe { + followees: Some(NeuronIds::from(vec![b1.clone()])), + neuron_id: Some(b3.clone()), + ..NeuronRecipe::validate_default_neurons_fund() + }, + btreemap! { + w => Followees { followees: vec![b1.clone()] }, + }, + "b3 (NeuronsFund, single followee)", + ), + // Case 6: Multiple followees (NeuronsFund participant) + ( + NeuronRecipe { + followees: Some(NeuronIds::from(vec![b0.clone(), b1.clone(), b2.clone()])), + neuron_id: Some(b3.clone()), + ..NeuronRecipe::validate_default_neurons_fund() + }, btreemap! { - w => Followees { followees: vec![b0] }, + w => Followees { followees: vec![b0.clone(), b1.clone(), b2.clone()] }, }, - "{}", - test_signature("b2") + "b3 (NeuronsFund, multiple followees)", + ), + ]; + + // Run tests + for (index, (neuron_recipe, expected_followees, description)) in + test_cases.into_iter().enumerate() + { + assert_eq!( + neuron_recipe.construct_followees(), + expected_followees, + "{}\nTest case {} failed: {}", + test_signature(description), + index, + description ); } } diff --git a/rs/sns/governance/tests/governance.rs b/rs/sns/governance/tests/governance.rs index ae1527a6f22d..cd0a3b83b2ff 100644 --- a/rs/sns/governance/tests/governance.rs +++ b/rs/sns/governance/tests/governance.rs @@ -8,7 +8,7 @@ use ic_nervous_system_common::{E8, ONE_DAY_SECONDS, ONE_MONTH_SECONDS}; use ic_nervous_system_common_test_keys::{ TEST_NEURON_1_OWNER_PRINCIPAL, TEST_NEURON_2_OWNER_PRINCIPAL, }; -use ic_nervous_system_proto::pb::v1::Percentage; +use ic_nervous_system_proto::pb::v1::{Percentage, Principals}; use ic_sns_governance::{ governance::MATURITY_DISBURSEMENT_DELAY_SECONDS, neuron::NeuronState, @@ -18,7 +18,10 @@ use ic_sns_governance::{ SetDappControllersResponse, }, v1::{ - claim_swap_neurons_request::NeuronParameters, + claim_swap_neurons_request::{ + neuron_recipe::{self, Participant}, + NeuronRecipe, NeuronRecipes, + }, claim_swap_neurons_response::{ClaimSwapNeuronsResult, ClaimedSwapNeurons, SwapNeuron}, governance_error::ErrorType, manage_neuron::{ @@ -30,15 +33,14 @@ use ic_sns_governance::{ Command as CommandResponse, DisburseMaturityResponse, MergeMaturityResponse, RegisterVoteResponse, StakeMaturityResponse, }, - neuron, - neuron::{DissolveState, Followees}, + neuron::{self, DissolveState, Followees}, proposal::Action, Account as AccountProto, AddMaturityRequest, Ballot, ClaimSwapNeuronsError, ClaimSwapNeuronsRequest, ClaimSwapNeuronsResponse, ClaimedSwapNeuronStatus, DeregisterDappCanisters, Empty, GovernanceError, ManageNeuronResponse, MintTokensRequest, MintTokensResponse, Motion, NervousSystemParameters, Neuron, - NeuronId, NeuronPermission, NeuronPermissionList, NeuronPermissionType, Proposal, - ProposalData, ProposalId, RegisterDappCanisters, Vote, WaitForQuietState, + NeuronId, NeuronIds, NeuronPermission, NeuronPermissionList, NeuronPermissionType, + Proposal, ProposalData, ProposalId, RegisterDappCanisters, Vote, WaitForQuietState, }, }, types::native_action_ids, @@ -1713,6 +1715,7 @@ fn test_claim_swap_neurons_rejects_unauthorized_access() { // Build the request, but leave it empty as it is not relevant to the test let request = ClaimSwapNeuronsRequest { neuron_parameters: vec![], + recipes: None, }; // Generate a principal id that should not be authorized to call claim_swap_neurons @@ -1760,10 +1763,11 @@ fn test_claim_swap_neurons_reports_invalid_neuron_parameters() { // Create a request with an invalid NeuronParameter let request = ClaimSwapNeuronsRequest { - neuron_parameters: vec![NeuronParameters { + neuron_parameters: vec![], + recipes: Some(NeuronRecipes::from(vec![NeuronRecipe { neuron_id: Some(test_neuron_id.clone()), ..Default::default() // The rest of the fields are unset and will fail validation - }], + }])), }; // Call the method @@ -1804,13 +1808,15 @@ fn test_claim_swap_neurons_reports_already_existing_neurons() { // Create a request with a neuron id that should collide with the neuron already inserted into // Governance let request = ClaimSwapNeuronsRequest { - neuron_parameters: vec![NeuronParameters { + neuron_parameters: vec![], + recipes: Some(NeuronRecipes::from(vec![NeuronRecipe { neuron_id: Some(neuron_id.clone()), controller: Some(user_principal), + participant: Some(Participant::Direct(neuron_recipe::Direct {})), stake_e8s: Some(E8), dissolve_delay_seconds: Some(0), - ..Default::default() // The rest of the parameters are not required for this test - }], + followees: Some(NeuronIds::from(vec![])), // Empty vec since we're using ..Default::default() in the original + }])), }; let authorized_sale_principal = canister_fixture.get_sale_canister_id(); @@ -1851,22 +1857,25 @@ fn test_claim_swap_neurons_reports_failure_if_neuron_cannot_be_added() { // Create a request with a NeuronParameter should succeed let request = ClaimSwapNeuronsRequest { - neuron_parameters: vec![ - NeuronParameters { + neuron_parameters: vec![], + recipes: Some(NeuronRecipes::from(vec![ + NeuronRecipe { neuron_id: Some(test_neuron_id_success.clone()), controller: Some(PrincipalId::new_user_test_id(1000)), + participant: Some(Participant::Direct(neuron_recipe::Direct {})), stake_e8s: Some(E8), dissolve_delay_seconds: Some(0), - ..Default::default() // The rest of the parameters are not required for this test + followees: Some(NeuronIds::from(vec![])), // Empty vec since we're using ..Default::default() in the original }, - NeuronParameters { + NeuronRecipe { neuron_id: Some(test_neuron_id_failure.clone()), controller: Some(PrincipalId::new_user_test_id(1000)), + participant: Some(Participant::Direct(neuron_recipe::Direct {})), stake_e8s: Some(E8), dissolve_delay_seconds: Some(0), - ..Default::default() // The rest of the parameters are not required for this test + followees: Some(NeuronIds::from(vec![])), // Empty vec since we're using ..Default::default() in the original }, - ], + ])), }; // Call the method @@ -1900,31 +1909,36 @@ fn test_claim_swap_neurons_succeeds() { // Set up the test environment with default sale canister id. let mut canister_fixture = GovernanceCanisterFixtureBuilder::new().create(); - let direct_participant_neuron_params = NeuronParameters { + let direct_participant_neuron_recipe = NeuronRecipe { neuron_id: Some(NeuronId::new_test_neuron_id(1)), controller: Some(PrincipalId::new_user_test_id(1000)), - hotkey: None, + participant: Some(Participant::Direct(neuron_recipe::Direct {})), stake_e8s: Some(E8), dissolve_delay_seconds: Some(0), - source_nns_neuron_id: None, - followees: vec![NeuronId::new_test_neuron_id(10)], + followees: Some(NeuronIds::from(vec![NeuronId::new_test_neuron_id(10)])), }; - let cf_participant_neuron_params = NeuronParameters { + let nf_participant_nns_neuron_id = 2; + let nf_participant_nns_neuron_controller = PrincipalId::new_user_test_id(1002); + let nf_participant_neuron_recipe = NeuronRecipe { neuron_id: Some(NeuronId::new_test_neuron_id(2)), controller: Some(PrincipalId::new_user_test_id(1001)), - hotkey: Some(PrincipalId::new_user_test_id(1002)), + participant: Some(Participant::NeuronsFund(neuron_recipe::NeuronsFund { + nns_neuron_controller: Some(nf_participant_nns_neuron_controller), + nns_neuron_id: Some(nf_participant_nns_neuron_id), + nns_neuron_hotkeys: Some(Principals::from(vec![PrincipalId::new_user_test_id(1003)])), + })), stake_e8s: Some(2 * E8), dissolve_delay_seconds: Some(ONE_MONTH_SECONDS), - source_nns_neuron_id: Some(2), - followees: vec![NeuronId::new_test_neuron_id(20)], + followees: Some(NeuronIds::from(vec![NeuronId::new_test_neuron_id(20)])), }; let request = ClaimSwapNeuronsRequest { - neuron_parameters: vec![ - direct_participant_neuron_params.clone(), - cf_participant_neuron_params.clone(), - ], + neuron_parameters: vec![], + recipes: Some(NeuronRecipes::from(vec![ + direct_participant_neuron_recipe.clone(), + nf_participant_neuron_recipe.clone(), + ])), }; // Call the method @@ -1945,7 +1959,7 @@ fn test_claim_swap_neurons_succeeds() { // Assert that each NeuronParameter has a response and that it has the correct status let direct_participant_swap_neuron = swap_neurons .iter() - .find(|s| s.id == direct_participant_neuron_params.neuron_id) + .find(|s| s.id == direct_participant_neuron_recipe.neuron_id) .unwrap(); assert_eq!( direct_participant_swap_neuron.status, @@ -1954,7 +1968,7 @@ fn test_claim_swap_neurons_succeeds() { let cf_participant_swap_neuron = swap_neurons .iter() - .find(|s| s.id == cf_participant_neuron_params.neuron_id) + .find(|s| s.id == nf_participant_neuron_recipe.neuron_id) .unwrap(); assert_eq!( cf_participant_swap_neuron.status, @@ -1963,19 +1977,19 @@ fn test_claim_swap_neurons_succeeds() { // Asserts on Direct Participant let direct_participant_neuron = - canister_fixture.get_neuron(direct_participant_neuron_params.neuron_id.as_ref().unwrap()); + canister_fixture.get_neuron(direct_participant_neuron_recipe.neuron_id.as_ref().unwrap()); assert_eq!( direct_participant_neuron.id, - direct_participant_neuron_params.neuron_id + direct_participant_neuron_recipe.neuron_id ); assert_eq!( direct_participant_neuron.cached_neuron_stake_e8s, - direct_participant_neuron_params.stake_e8s() + direct_participant_neuron_recipe.stake_e8s() ); assert_eq!( direct_participant_neuron.dissolve_state, Some(DissolveState::DissolveDelaySeconds( - direct_participant_neuron_params.dissolve_delay_seconds() + direct_participant_neuron_recipe.dissolve_delay_seconds() )) ); assert_eq!(direct_participant_neuron.source_nns_neuron_id, None); @@ -1984,37 +1998,40 @@ fn test_claim_swap_neurons_succeeds() { assert_eq!(direct_participant_neuron.auto_stake_maturity, None); for followees in direct_participant_neuron.followees.values() { assert_eq!( - followees.followees, - direct_participant_neuron_params.followees + Some(NeuronIds::from(followees.followees.clone())), + direct_participant_neuron_recipe.followees ); } // Asserts on CF Participant let cf_participant_neuron = - canister_fixture.get_neuron(cf_participant_neuron_params.neuron_id.as_ref().unwrap()); + canister_fixture.get_neuron(nf_participant_neuron_recipe.neuron_id.as_ref().unwrap()); assert_eq!( cf_participant_neuron.id, - cf_participant_neuron_params.neuron_id + nf_participant_neuron_recipe.neuron_id ); assert_eq!( cf_participant_neuron.cached_neuron_stake_e8s, - cf_participant_neuron_params.stake_e8s() + nf_participant_neuron_recipe.stake_e8s() ); assert_eq!( cf_participant_neuron.dissolve_state, Some(DissolveState::DissolveDelaySeconds( - cf_participant_neuron_params.dissolve_delay_seconds() + nf_participant_neuron_recipe.dissolve_delay_seconds() )) ); assert_eq!( cf_participant_neuron.source_nns_neuron_id, - cf_participant_neuron_params.source_nns_neuron_id + Some(nf_participant_nns_neuron_id) ); assert_eq!(cf_participant_neuron.maturity_e8s_equivalent, 0); assert_eq!(cf_participant_neuron.neuron_fees_e8s, 0); assert_eq!(cf_participant_neuron.auto_stake_maturity, Some(true)); for followees in cf_participant_neuron.followees.values() { - assert_eq!(followees.followees, cf_participant_neuron_params.followees); + assert_eq!( + Some(NeuronIds::from(followees.followees.clone())), + nf_participant_neuron_recipe.followees + ); } } diff --git a/rs/sns/swap/src/swap.rs b/rs/sns/swap/src/swap.rs index d2bac9c75677..6e7af30ef1a3 100644 --- a/rs/sns/swap/src/swap.rs +++ b/rs/sns/swap/src/swap.rs @@ -1823,6 +1823,7 @@ impl Swap { let reply = sns_governance_client .claim_swap_neurons(ClaimSwapNeuronsRequest { neuron_parameters: batch, + recipes: None, }) .await; diff --git a/rs/sns/swap/tests/swap.rs b/rs/sns/swap/tests/swap.rs index 6a8db7ecff11..7827cbb9f45c 100644 --- a/rs/sns/swap/tests/swap.rs +++ b/rs/sns/swap/tests/swap.rs @@ -3528,6 +3528,7 @@ async fn test_claim_swap_neuron_correctly_creates_neuron_parameters() { followees: vec![NeuronId::new_test_neuron_id(20)], } ], + recipes: None, } )] )