diff --git a/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs b/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs index ee7f9705904..7420d724359 100644 --- a/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs +++ b/rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs @@ -1987,61 +1987,41 @@ pub struct WaitForQuietState { /// This is a view of the ProposalData returned by API queries and is NOT used /// for storage. The ballots are restricted to those of the caller's neurons and /// additionally it has the computed fields, topic, status, and reward_status. -#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)] -#[allow(clippy::derive_partial_eq_without_eq)] -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, Clone, Debug, PartialEq)] pub struct ProposalInfo { /// The unique id for this proposal. - #[prost(message, optional, tag = "1")] pub id: Option<::ic_nns_common::pb::v1::ProposalId>, /// The ID of the neuron that made this proposal. - #[prost(message, optional, tag = "2")] pub proposer: Option, /// The amount of ICP in E8s to be charged to the proposer if the proposal is /// rejected. - #[prost(uint64, tag = "3")] pub reject_cost_e8s: u64, /// The proposal originally submitted. - #[prost(message, optional, tag = "4")] pub proposal: Option, /// The timestamp, in seconds from the Unix epoch, when this proposal was made. - #[prost(uint64, tag = "5")] pub proposal_timestamp_seconds: u64, /// See \[ProposalData::ballots\]. - #[prost(map = "fixed64, message", tag = "6")] pub ballots: ::std::collections::HashMap, /// See \[ProposalData::latest_tally\]. - #[prost(message, optional, tag = "7")] pub latest_tally: Option, /// See \[ProposalData::decided_timestamp_seconds\]. - #[prost(uint64, tag = "8")] pub decided_timestamp_seconds: u64, /// See \[ProposalData::executed_timestamp_seconds\]. - #[prost(uint64, tag = "12")] pub executed_timestamp_seconds: u64, /// See \[ProposalData::failed_timestamp_seconds\]. - #[prost(uint64, tag = "13")] pub failed_timestamp_seconds: u64, /// See \[ProposalData::failure_reason\]. - #[prost(message, optional, tag = "18")] pub failure_reason: Option, /// See \[ProposalData::reward_event_round\]. - #[prost(uint64, tag = "14")] pub reward_event_round: u64, /// Derived - see \[Topic\] for more information - #[prost(enumeration = "Topic", tag = "15")] pub topic: i32, /// Derived - see \[ProposalStatus\] for more information - #[prost(enumeration = "ProposalStatus", tag = "16")] pub status: i32, /// Derived - see \[ProposalRewardStatus\] for more information - #[prost(enumeration = "ProposalRewardStatus", tag = "17")] pub reward_status: i32, - #[prost(uint64, optional, tag = "19")] pub deadline_timestamp_seconds: Option, - #[prost(message, optional, tag = "20")] pub derived_proposal_information: Option, - #[prost(uint64, optional, tag = "21")] pub total_potential_voting_power: ::core::option::Option, } @@ -3308,11 +3288,8 @@ pub struct ListProposalInfo { #[prost(bool, optional, tag = "7")] pub omit_large_fields: Option, } -#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)] -#[allow(clippy::derive_partial_eq_without_eq)] -#[derive(Clone, PartialEq, ::prost::Message)] +#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, Clone, Debug, PartialEq)] pub struct ListProposalInfoResponse { - #[prost(message, repeated, tag = "1")] pub proposal_info: Vec, } /// A request to list neurons. The "requested list", i.e., the list of diff --git a/rs/nns/governance/canbench/canbench_results.yml b/rs/nns/governance/canbench/canbench_results.yml index 7d8efc965bc..8ad8c187219 100644 --- a/rs/nns/governance/canbench/canbench_results.yml +++ b/rs/nns/governance/canbench/canbench_results.yml @@ -1,55 +1,55 @@ benches: add_neuron_active_maximum: total: - instructions: 42559874 + instructions: 42556627 heap_increase: 1 stable_memory_increase: 0 scopes: {} add_neuron_active_typical: total: - instructions: 2160611 + instructions: 2160472 heap_increase: 0 stable_memory_increase: 0 scopes: {} add_neuron_inactive_maximum: total: - instructions: 112126649 + instructions: 112123402 heap_increase: 1 stable_memory_increase: 0 scopes: {} add_neuron_inactive_typical: total: - instructions: 8465215 + instructions: 8465076 heap_increase: 0 stable_memory_increase: 0 scopes: {} cascading_vote_all_heap: total: - instructions: 35565672 + instructions: 34853041 heap_increase: 0 stable_memory_increase: 128 scopes: {} cascading_vote_heap_neurons_stable_index: total: - instructions: 61721390 + instructions: 61008759 heap_increase: 0 stable_memory_increase: 128 scopes: {} cascading_vote_stable_everything: total: - instructions: 188861073 + instructions: 188854175 heap_increase: 0 stable_memory_increase: 128 scopes: {} cascading_vote_stable_neurons_with_heap_index: total: - instructions: 162571959 + instructions: 162565061 heap_increase: 0 stable_memory_increase: 128 scopes: {} centralized_following_all_stable: total: - instructions: 78285442 + instructions: 78286335 heap_increase: 0 stable_memory_increase: 128 scopes: {} @@ -61,7 +61,7 @@ benches: scopes: {} draw_maturity_from_neurons_fund_heap: total: - instructions: 7549569 + instructions: 7473869 heap_increase: 0 stable_memory_increase: 0 scopes: {} @@ -73,13 +73,13 @@ benches: scopes: {} list_active_neurons_fund_neurons_heap: total: - instructions: 436248 + instructions: 425548 heap_increase: 0 stable_memory_increase: 0 scopes: {} list_active_neurons_fund_neurons_stable: total: - instructions: 2755320 + instructions: 2745520 heap_increase: 0 stable_memory_increase: 0 scopes: {} @@ -91,19 +91,19 @@ benches: scopes: {} list_neurons_ready_to_unstake_maturity_heap: total: - instructions: 158257 + instructions: 158253 heap_increase: 0 stable_memory_increase: 0 scopes: {} list_neurons_ready_to_unstake_maturity_stable: total: - instructions: 41326673 + instructions: 41326675 heap_increase: 0 stable_memory_increase: 0 scopes: {} list_neurons_stable: total: - instructions: 113166221 + instructions: 113168021 heap_increase: 5 stable_memory_increase: 0 scopes: {} @@ -115,19 +115,19 @@ benches: scopes: {} list_ready_to_spawn_neuron_ids_stable: total: - instructions: 41298569 + instructions: 41298578 heap_increase: 0 stable_memory_increase: 0 scopes: {} neuron_data_validation_heap: total: - instructions: 407871691 + instructions: 407849853 heap_increase: 0 stable_memory_increase: 0 scopes: {} neuron_data_validation_stable: total: - instructions: 363603468 + instructions: 363580728 heap_increase: 0 stable_memory_increase: 0 scopes: {} @@ -151,7 +151,7 @@ benches: scopes: {} single_vote_all_stable: total: - instructions: 2802680 + instructions: 2802518 heap_increase: 0 stable_memory_increase: 128 scopes: {} diff --git a/rs/nns/governance/canister/canister.rs b/rs/nns/governance/canister/canister.rs index 0d343ebe194..4e9eeb9d92b 100644 --- a/rs/nns/governance/canister/canister.rs +++ b/rs/nns/governance/canister/canister.rs @@ -782,9 +782,7 @@ fn get_neuron_info_by_id_or_subaccount( #[query] fn get_proposal_info(id: ProposalId) -> Option { debug_log("get_proposal_info"); - governance() - .get_proposal_info(&caller(), id) - .map(ProposalInfo::from) + GOVERNANCE.with_borrow(|governance| governance.get_proposal_info(&caller(), id)) } #[query] @@ -800,17 +798,13 @@ fn get_neurons_fund_audit_info( #[query] fn get_pending_proposals() -> Vec { debug_log("get_pending_proposals"); - governance() - .get_pending_proposals(&caller()) - .into_iter() - .map(ProposalInfo::from) - .collect() + GOVERNANCE.with_borrow(|governance| governance.get_pending_proposals(&caller())) } #[query] fn list_proposals(req: ListProposalInfo) -> ListProposalInfoResponse { debug_log("list_proposals"); - governance().list_proposals(&caller(), &(req.into())).into() + GOVERNANCE.with_borrow(|governance| governance.list_proposals(&caller(), &req.into())) } #[query] @@ -892,7 +886,10 @@ fn get_latest_reward_event() -> RewardEvent { #[query] fn get_neuron_ids() -> Vec { debug_log("get_neuron_ids"); - let votable = governance().get_neuron_ids_by_principal(&caller()); + let votable = governance() + .get_neuron_ids_by_principal(&caller()) + .into_iter() + .collect(); governance() .get_managed_neuron_ids_for(votable) 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 7db94271fc0..d7c5ed07f9e 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 @@ -1780,64 +1780,6 @@ message WaitForQuietState { uint64 current_deadline_timestamp_seconds = 1; } -// This is a view of the ProposalData returned by API queries and is NOT used -// for storage. The ballots are restricted to those of the caller's neurons and -// additionally it has the computed fields, topic, status, and reward_status. -message ProposalInfo { - // The unique id for this proposal. - ic_nns_common.pb.v1.ProposalId id = 1; - - // The ID of the neuron that made this proposal. - ic_nns_common.pb.v1.NeuronId proposer = 2; - - // The amount of ICP in E8s to be charged to the proposer if the proposal is - // rejected. - uint64 reject_cost_e8s = 3; - - // The proposal originally submitted. - Proposal proposal = 4; - - // The timestamp, in seconds from the Unix epoch, when this proposal was made. - uint64 proposal_timestamp_seconds = 5; - - // See [ProposalData::ballots]. - map ballots = 6; - - // See [ProposalData::latest_tally]. - Tally latest_tally = 7; - - // See [ProposalData::decided_timestamp_seconds]. - uint64 decided_timestamp_seconds = 8; - - // See [ProposalData::executed_timestamp_seconds]. - uint64 executed_timestamp_seconds = 12; - - // See [ProposalData::failed_timestamp_seconds]. - uint64 failed_timestamp_seconds = 13; - - // See [ProposalData::failure_reason]. - GovernanceError failure_reason = 18; - - // See [ProposalData::reward_event_round]. - uint64 reward_event_round = 14; - - // Derived - see [Topic] for more information - Topic topic = 15; - - // Derived - see [ProposalStatus] for more information - ProposalStatus status = 16; - - // Derived - see [ProposalRewardStatus] for more information - ProposalRewardStatus reward_status = 17; - - optional uint64 deadline_timestamp_seconds = 19; - - DerivedProposalInformation derived_proposal_information = 20; - - // See [ProposalData::total_potential_voting_power]. - optional uint64 total_potential_voting_power = 21; -} - // Network economics contains the parameters for several operations related // to the economy of the network. When submitting a NetworkEconomics proposal // default values (0) are considered unchanged, so a valid proposal only needs @@ -2626,10 +2568,6 @@ message ListProposalInfo { optional bool omit_large_fields = 7; } -message ListProposalInfoResponse { - repeated ProposalInfo proposal_info = 1; -} - // A request to list neurons. The "requested list", i.e., the list of // neuron IDs to retrieve information about, is the union of the list // of neurons listed in `neuron_ids` and, if `caller_neurons` is true, 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 bfc6f5d5de3..a067b64c6c9 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 @@ -2418,73 +2418,6 @@ pub struct WaitForQuietState { #[prost(uint64, tag = "1")] pub current_deadline_timestamp_seconds: u64, } -/// This is a view of the ProposalData returned by API queries and is NOT used -/// for storage. The ballots are restricted to those of the caller's neurons and -/// additionally it has the computed fields, topic, status, and reward_status. -#[derive( - candid::CandidType, - candid::Deserialize, - serde::Serialize, - comparable::Comparable, - Clone, - PartialEq, - ::prost::Message, -)] -pub struct ProposalInfo { - /// The unique id for this proposal. - #[prost(message, optional, tag = "1")] - pub id: ::core::option::Option<::ic_nns_common::pb::v1::ProposalId>, - /// The ID of the neuron that made this proposal. - #[prost(message, optional, tag = "2")] - pub proposer: ::core::option::Option<::ic_nns_common::pb::v1::NeuronId>, - /// The amount of ICP in E8s to be charged to the proposer if the proposal is - /// rejected. - #[prost(uint64, tag = "3")] - pub reject_cost_e8s: u64, - /// The proposal originally submitted. - #[prost(message, optional, tag = "4")] - pub proposal: ::core::option::Option, - /// The timestamp, in seconds from the Unix epoch, when this proposal was made. - #[prost(uint64, tag = "5")] - pub proposal_timestamp_seconds: u64, - /// See \[ProposalData::ballots\]. - #[prost(map = "fixed64, message", tag = "6")] - pub ballots: ::std::collections::HashMap, - /// See \[ProposalData::latest_tally\]. - #[prost(message, optional, tag = "7")] - pub latest_tally: ::core::option::Option, - /// See \[ProposalData::decided_timestamp_seconds\]. - #[prost(uint64, tag = "8")] - pub decided_timestamp_seconds: u64, - /// See \[ProposalData::executed_timestamp_seconds\]. - #[prost(uint64, tag = "12")] - pub executed_timestamp_seconds: u64, - /// See \[ProposalData::failed_timestamp_seconds\]. - #[prost(uint64, tag = "13")] - pub failed_timestamp_seconds: u64, - /// See \[ProposalData::failure_reason\]. - #[prost(message, optional, tag = "18")] - pub failure_reason: ::core::option::Option, - /// See \[ProposalData::reward_event_round\]. - #[prost(uint64, tag = "14")] - pub reward_event_round: u64, - /// Derived - see \[Topic\] for more information - #[prost(enumeration = "Topic", tag = "15")] - pub topic: i32, - /// Derived - see \[ProposalStatus\] for more information - #[prost(enumeration = "ProposalStatus", tag = "16")] - pub status: i32, - /// Derived - see \[ProposalRewardStatus\] for more information - #[prost(enumeration = "ProposalRewardStatus", tag = "17")] - pub reward_status: i32, - #[prost(uint64, optional, tag = "19")] - pub deadline_timestamp_seconds: ::core::option::Option, - #[prost(message, optional, tag = "20")] - pub derived_proposal_information: ::core::option::Option, - /// See \[ProposalData::total_potential_voting_power\]. - #[prost(uint64, optional, tag = "21")] - pub total_potential_voting_power: ::core::option::Option, -} /// Network economics contains the parameters for several operations related /// to the economy of the network. When submitting a NetworkEconomics proposal /// default values (0) are considered unchanged, so a valid proposal only needs @@ -3914,19 +3847,6 @@ pub struct ListProposalInfo { #[prost(bool, optional, tag = "7")] pub omit_large_fields: ::core::option::Option, } -#[derive( - candid::CandidType, - candid::Deserialize, - serde::Serialize, - comparable::Comparable, - Clone, - PartialEq, - ::prost::Message, -)] -pub struct ListProposalInfoResponse { - #[prost(message, repeated, tag = "1")] - pub proposal_info: ::prost::alloc::vec::Vec, -} /// A request to list neurons. The "requested list", i.e., the list of /// neuron IDs to retrieve information about, is the union of the list /// of neurons listed in `neuron_ids` and, if `caller_neurons` is true, diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index d5829c24be5..5c77614afeb 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -25,46 +25,49 @@ use crate::{ latest_node_provider_rewards, list_node_provider_rewards, record_node_provider_rewards, DateRangeFilter, }, - pb::v1::{ - add_or_remove_node_provider::Change, - archived_monthly_node_provider_rewards, - create_service_nervous_system::LedgerParameters, - get_neurons_fund_audit_info_response, - governance::{ - governance_cached_metrics::NeuronSubsetMetrics as NeuronSubsetMetricsPb, - neuron_in_flight_command::{Command as InFlightCommand, SyncCommand}, - GovernanceCachedMetrics, NeuronInFlightCommand, - }, - governance_error::ErrorType, - manage_neuron, - manage_neuron::{ - claim_or_refresh::{By, MemoAndController}, - ClaimOrRefresh, Command, NeuronIdOrSubaccount, + pb::{ + proposal_conversions::{convert_proposal, proposal_data_to_info}, + v1::{ + add_or_remove_node_provider::Change, + archived_monthly_node_provider_rewards, + create_service_nervous_system::LedgerParameters, + get_neurons_fund_audit_info_response, + governance::{ + governance_cached_metrics::NeuronSubsetMetrics as NeuronSubsetMetricsPb, + neuron_in_flight_command::{Command as InFlightCommand, SyncCommand}, + GovernanceCachedMetrics, NeuronInFlightCommand, + }, + governance_error::ErrorType, + manage_neuron::{ + self, + claim_or_refresh::{By, MemoAndController}, + ClaimOrRefresh, Command, NeuronIdOrSubaccount, + }, + manage_neuron_response::{self, MergeMaturityResponse, StakeMaturityResponse}, + neuron::Followees, + neurons_fund_snapshot::NeuronsFundNeuronPortion as NeuronsFundNeuronPortionPb, + proposal::Action, + reward_node_provider::{RewardMode, RewardToAccount}, + settle_neurons_fund_participation_request, + settle_neurons_fund_participation_response::{ + self, NeuronsFundNeuron as NeuronsFundNeuronPb, + }, + swap_background_information, ArchivedMonthlyNodeProviderRewards, Ballot, + CreateServiceNervousSystem, ExecuteNnsFunction, GetNeuronsFundAuditInfoRequest, + GetNeuronsFundAuditInfoResponse, Governance as GovernanceProto, GovernanceError, + InstallCode, KnownNeuron, ListKnownNeuronsResponse, ListNeurons, ListNeuronsResponse, + ListProposalInfo, ManageNeuron, ManageNeuronResponse, MonthlyNodeProviderRewards, + Motion, NetworkEconomics, Neuron as NeuronProto, NeuronInfo, NeuronState, + NeuronsFundAuditInfo, NeuronsFundData, + NeuronsFundEconomics as NeuronsFundNetworkEconomicsPb, + NeuronsFundParticipation as NeuronsFundParticipationPb, + NeuronsFundSnapshot as NeuronsFundSnapshotPb, NnsFunction, NodeProvider, Proposal, + ProposalData, ProposalRewardStatus, ProposalStatus, RestoreAgingSummary, RewardEvent, + RewardNodeProvider, RewardNodeProviders, SettleNeuronsFundParticipationRequest, + SettleNeuronsFundParticipationResponse, StopOrStartCanister, Tally, Topic, + UpdateCanisterSettings, UpdateNodeProvider, Visibility, Vote, VotingPowerEconomics, + WaitForQuietState, XdrConversionRate as XdrConversionRatePb, }, - manage_neuron_response, - manage_neuron_response::{MergeMaturityResponse, StakeMaturityResponse}, - neuron::Followees, - neurons_fund_snapshot::NeuronsFundNeuronPortion as NeuronsFundNeuronPortionPb, - proposal, - proposal::Action, - reward_node_provider::{RewardMode, RewardToAccount}, - settle_neurons_fund_participation_request, settle_neurons_fund_participation_response, - settle_neurons_fund_participation_response::NeuronsFundNeuron as NeuronsFundNeuronPb, - swap_background_information, ArchivedMonthlyNodeProviderRewards, Ballot, - CreateServiceNervousSystem, ExecuteNnsFunction, GetNeuronsFundAuditInfoRequest, - GetNeuronsFundAuditInfoResponse, Governance as GovernanceProto, GovernanceError, - InstallCode, KnownNeuron, ListKnownNeuronsResponse, ListNeurons, ListNeuronsResponse, - ListProposalInfo, ListProposalInfoResponse, ManageNeuron, ManageNeuronResponse, - MonthlyNodeProviderRewards, Motion, NetworkEconomics, Neuron as NeuronProto, NeuronInfo, - NeuronState, NeuronsFundAuditInfo, NeuronsFundData, - NeuronsFundEconomics as NeuronsFundNetworkEconomicsPb, - NeuronsFundParticipation as NeuronsFundParticipationPb, - NeuronsFundSnapshot as NeuronsFundSnapshotPb, NnsFunction, NodeProvider, Proposal, - ProposalData, ProposalInfo, ProposalRewardStatus, ProposalStatus, RestoreAgingSummary, - RewardEvent, RewardNodeProvider, RewardNodeProviders, - SettleNeuronsFundParticipationRequest, SettleNeuronsFundParticipationResponse, - StopOrStartCanister, Tally, Topic, UpdateCanisterSettings, UpdateNodeProvider, Visibility, - Vote, VotingPowerEconomics, WaitForQuietState, XdrConversionRate as XdrConversionRatePb, }, proposals::{call_canister::CallCanister, sum_weighted_voting_power}, }; @@ -94,7 +97,11 @@ use ic_nns_constants::{ SUBNET_RENTAL_CANISTER_ID, }; use ic_nns_governance_api::{ - pb::v1::CreateServiceNervousSystem as ApiCreateServiceNervousSystem, proposal_validation, + pb::v1::{ + CreateServiceNervousSystem as ApiCreateServiceNervousSystem, ListProposalInfoResponse, + ProposalInfo, + }, + proposal_validation, subnet_rental::SubnetRentalRequest, }; use ic_protobuf::registry::dc::v1::AddOrRemoveDataCentersProposalPayload; @@ -1055,13 +1062,6 @@ impl Proposal { .as_ref() .map_or(false, |a| a.allowed_when_resources_are_low()) } - - fn omit_large_fields(self) -> Self { - Proposal { - action: self.action.map(|action| action.omit_large_fields()), - ..self - } - } } impl Action { @@ -1109,32 +1109,6 @@ impl Action { _ => false, } } - - fn omit_large_fields(self) -> Self { - match self { - Action::CreateServiceNervousSystem(create_service_nervous_system) => { - Action::CreateServiceNervousSystem(CreateServiceNervousSystem { - ledger_parameters: create_service_nervous_system.ledger_parameters.map( - |ledger_parameters| LedgerParameters { - token_logo: None, - ..ledger_parameters - }, - ), - logo: None, - ..create_service_nervous_system - }) - } - Action::ExecuteNnsFunction(mut execute_nns_function) => { - if execute_nns_function.payload.len() - > EXECUTE_NNS_FUNCTION_PAYLOAD_LISTING_BYTES_MAX - { - execute_nns_function.payload.clear(); - } - Action::ExecuteNnsFunction(execute_nns_function) - } - action => action, - } - } } impl ProposalData { @@ -1459,15 +1433,6 @@ impl ProposalData { } } -impl ProposalInfo { - fn omit_large_fields(self) -> Self { - ProposalInfo { - proposal: self.proposal.map(|proposal| proposal.omit_large_fields()), - ..self - } - } -} - #[cfg(test)] mod test_wait_for_quiet { use crate::pb::v1::{ProposalData, Tally, WaitForQuietState}; @@ -2383,11 +2348,9 @@ impl Governance { /// TODO(NNS1-2499): inline this. /// Return the Neuron IDs of all Neurons that have `principal` as their /// controller or as one of their hot keys. - pub fn get_neuron_ids_by_principal(&self, principal_id: &PrincipalId) -> Vec { + pub fn get_neuron_ids_by_principal(&self, principal_id: &PrincipalId) -> HashSet { self.neuron_store .get_neuron_ids_readable_by_caller(*principal_id) - .into_iter() - .collect() } /// Return the union of `followees` with the set of Neuron IDs of all @@ -2442,6 +2405,8 @@ impl Governance { let mut implicitly_requested_neuron_ids = if *include_neurons_readable_by_caller { if include_empty_neurons_readable_by_caller { self.get_neuron_ids_by_principal(&caller) + .into_iter() + .collect() } else { self.neuron_store .get_non_empty_neuron_ids_readable_by_caller(caller) @@ -3865,16 +3830,22 @@ impl Governance { caller: &PrincipalId, pid: impl Into, ) -> Option { - let proposal_data = self.heap_data.proposals.get(&pid.into().id); - match proposal_data { - None => None, - Some(pd) => { - let caller_neurons: HashSet = - self.neuron_store.get_neuron_ids_readable_by_caller(*caller); - let now = self.env.now(); - Some(self.proposal_data_to_info(pd, &caller_neurons, now, false)) - } - } + let now_seconds = self.env.now(); + let caller_neurons = self.get_neuron_ids_by_principal(caller); + let voting_period_seconds = self.voting_period_seconds(); + self.heap_data + .proposals + .get(&pid.into().id) + .map(|proposal_data| { + proposal_data_to_info( + proposal_data, + false, + false, + &caller_neurons, + now_seconds, + voting_period_seconds, + ) + }) } /// Tries to get the Neurons' Fund participation data for an SNS Swap created via given proposal. @@ -3944,31 +3915,25 @@ impl Governance { }) } - /// Gets all open proposals - /// - /// - The proposals' ballots only show votes from neurons that the - /// caller either controls or is a registered hot key for. - /// - /// - Proposals with `ExecuteNnsFunction` as action have their - /// `payload` cleared if larger than - /// EXECUTE_NNS_FUNCTION_PAYLOAD_LISTING_BYTES_MAX. The caller can - /// retrieve dropped payloads by calling `get_proposal_info` for - /// each proposal of interest. + /// Iterator over proposals info of pending proposals. pub fn get_pending_proposals(&self, caller: &PrincipalId) -> Vec { - let caller_neurons: HashSet = - self.neuron_store.get_neuron_ids_readable_by_caller(*caller); let now = self.env.now(); - self.get_pending_proposals_data() - .map(|data| self.proposal_data_to_info(data, &caller_neurons, now, true)) - .collect() - } - - /// Iterator over proposals info of pending proposals. - pub fn get_pending_proposals_data(&self) -> impl Iterator { + let caller_neurons = self.get_neuron_ids_by_principal(caller); self.heap_data .proposals .values() .filter(|data| data.status() == ProposalStatus::Open) + .map(|data| { + proposal_data_to_info( + data, + true, + false, + &caller_neurons, + now, + self.voting_period_seconds(), + ) + }) + .collect() } // Gets the raw proposal data @@ -3990,76 +3955,6 @@ impl Governance { ) } - fn proposal_data_to_info( - &self, - data: &ProposalData, - caller_neurons: &HashSet, - now_seconds: u64, - multi_query: bool, - ) -> ProposalInfo { - // Calculate derived fields - let topic = data.topic(); - let status = data.status(); - let voting_period_seconds = self.voting_period_seconds()(topic); - let reward_status = data.reward_status(now_seconds, voting_period_seconds); - - // For multi-queries, large fields such as WASM blobs need to be omitted. Otherwise the - // message limit will be exceeded. - let proposal = if multi_query { - if let Some( - proposal @ Proposal { - action: Some(proposal::Action::ExecuteNnsFunction(_)), - .. - }, - ) = data.proposal.clone() - { - Some(proposal.omit_large_fields()) - } else { - data.proposal.clone() - } - } else { - data.proposal.clone() - }; - - /// Remove all ballots except the ballots belonging to a neuron present - /// in `except_from`. - fn remove_ballots_not_cast_by( - all_ballots: &HashMap, - except_from: &HashSet, - ) -> HashMap { - let mut ballots = HashMap::new(); - for neuron_id in except_from.iter() { - if let Some(v) = all_ballots.get(&neuron_id.id) { - ballots.insert(neuron_id.id, *v); - } - } - ballots - } - - ProposalInfo { - id: data.id, - proposer: data.proposer, - reject_cost_e8s: data.reject_cost_e8s, - proposal, - proposal_timestamp_seconds: data.proposal_timestamp_seconds, - ballots: remove_ballots_not_cast_by(&data.ballots, caller_neurons), - latest_tally: data.latest_tally, - decided_timestamp_seconds: data.decided_timestamp_seconds, - executed_timestamp_seconds: data.executed_timestamp_seconds, - failed_timestamp_seconds: data.failed_timestamp_seconds, - failure_reason: data.failure_reason.clone(), - reward_event_round: data.reward_event_round, - topic: topic as i32, - status: status as i32, - reward_status: reward_status as i32, - deadline_timestamp_seconds: Some( - data.get_deadline_timestamp_seconds(voting_period_seconds), - ), - derived_proposal_information: data.derived_proposal_information.clone(), - total_potential_voting_power: data.total_potential_voting_power, - } - } - /// Return true if the 'info' proposal is visible to some of the neurons in /// 'caller_neurons'. fn proposal_is_visible_to_neurons( @@ -4135,12 +4030,11 @@ impl Governance { caller: &PrincipalId, req: &ListProposalInfo, ) -> ListProposalInfoResponse { - let caller_neurons: HashSet = - self.neuron_store.get_neuron_ids_readable_by_caller(*caller); let exclude_topic: HashSet = req.exclude_topic.iter().cloned().collect(); let include_reward_status: HashSet = req.include_reward_status.iter().cloned().collect(); let include_status: HashSet = req.include_status.iter().cloned().collect(); + let caller_neurons = self.get_neuron_ids_by_principal(caller); let now = self.env.now(); let proposal_matches_request = |data: &ProposalData| -> bool { let topic = data.topic(); @@ -4185,23 +4079,21 @@ impl Governance { let proposals = proposals .rev() .filter(|(_, x)| proposal_matches_request(x)) - .take(limit); - - let proposal_info = proposals - .map(|(_, y)| y) - .map(|pd| self.proposal_data_to_info(pd, &caller_neurons, now, true)) - .collect::>(); - - let proposal_info = if req.omit_large_fields() { - proposal_info - .into_iter() - .map(|data| data.omit_large_fields()) - .collect() - } else { - proposal_info - }; - - ListProposalInfoResponse { proposal_info } + .take(limit) + .map(|(_, proposal_data)| { + proposal_data_to_info( + proposal_data, + true, + req.omit_large_fields(), + &caller_neurons, + now, + self.voting_period_seconds(), + ) + }) + .collect(); + ListProposalInfoResponse { + proposal_info: proposals, + } } // This is slow, because it scans all proposals. @@ -5289,9 +5181,9 @@ impl Governance { Err(format!("Topic not specified. proposal: {:#?}", proposal))?; } - proposal_validation::validate_user_submitted_proposal_fields( - &ic_nns_governance_api::pb::v1::Proposal::from(proposal.clone()), - )?; + proposal_validation::validate_user_submitted_proposal_fields(&convert_proposal( + &proposal, true, + ))?; if !proposal.allowed_when_resources_are_low() { self.check_heap_can_grow()?; diff --git a/rs/nns/governance/src/pb/conversions.rs b/rs/nns/governance/src/pb/conversions.rs index e25e507ab52..e1bba131bc2 100644 --- a/rs/nns/governance/src/pb/conversions.rs +++ b/rs/nns/governance/src/pb/conversions.rs @@ -1,6 +1,8 @@ use crate::pb::v1 as pb; use ic_nns_governance_api::pb::v1 as pb_api; +use crate::pb::proposal_conversions::convert_proposal; + impl From for pb_api::NodeProvider { fn from(item: pb::NodeProvider) -> Self { Self { @@ -479,16 +481,6 @@ impl From for pb::SetSnsTokenSwapOpenTime } } -impl From for pb_api::Proposal { - fn from(item: pb::Proposal) -> Self { - Self { - title: item.title, - summary: item.summary, - url: item.url, - action: item.action.map(|x| x.into()), - } - } -} impl From for pb::Proposal { fn from(item: pb_api::Proposal) -> Self { Self { @@ -510,56 +502,6 @@ impl From for pb::Proposal { } } -impl From for pb_api::proposal::Action { - fn from(item: pb::proposal::Action) -> Self { - match item { - pb::proposal::Action::ManageNeuron(v) => { - pb_api::proposal::Action::ManageNeuron(Box::new((*v).into())) - } - pb::proposal::Action::ManageNetworkEconomics(v) => { - pb_api::proposal::Action::ManageNetworkEconomics(v.into()) - } - pb::proposal::Action::Motion(v) => pb_api::proposal::Action::Motion(v.into()), - pb::proposal::Action::ExecuteNnsFunction(v) => { - pb_api::proposal::Action::ExecuteNnsFunction(v.into()) - } - pb::proposal::Action::ApproveGenesisKyc(v) => { - pb_api::proposal::Action::ApproveGenesisKyc(v.into()) - } - pb::proposal::Action::AddOrRemoveNodeProvider(v) => { - pb_api::proposal::Action::AddOrRemoveNodeProvider(v.into()) - } - pb::proposal::Action::RewardNodeProvider(v) => { - pb_api::proposal::Action::RewardNodeProvider(v.into()) - } - pb::proposal::Action::SetDefaultFollowees(v) => { - pb_api::proposal::Action::SetDefaultFollowees(v.into()) - } - pb::proposal::Action::RewardNodeProviders(v) => { - pb_api::proposal::Action::RewardNodeProviders(v.into()) - } - pb::proposal::Action::RegisterKnownNeuron(v) => { - pb_api::proposal::Action::RegisterKnownNeuron(v.into()) - } - pb::proposal::Action::SetSnsTokenSwapOpenTimeWindow(v) => { - pb_api::proposal::Action::SetSnsTokenSwapOpenTimeWindow(v.into()) - } - pb::proposal::Action::OpenSnsTokenSwap(v) => { - pb_api::proposal::Action::OpenSnsTokenSwap(v.into()) - } - pb::proposal::Action::CreateServiceNervousSystem(v) => { - pb_api::proposal::Action::CreateServiceNervousSystem(v.into()) - } - pb::proposal::Action::InstallCode(v) => pb_api::proposal::Action::InstallCode(v.into()), - pb::proposal::Action::StopOrStartCanister(v) => { - pb_api::proposal::Action::StopOrStartCanister(v.into()) - } - pb::proposal::Action::UpdateCanisterSettings(v) => { - pb_api::proposal::Action::UpdateCanisterSettings(v.into()) - } - } - } -} impl From for pb::proposal::Action { fn from(item: pb_api::proposal::Action) -> Self { match item { @@ -1206,7 +1148,7 @@ impl From for pb_api::manage_neuron::Command { pb_api::manage_neuron::Command::Follow(v.into()) } pb::manage_neuron::Command::MakeProposal(v) => { - pb_api::manage_neuron::Command::MakeProposal(Box::new((*v).into())) + pb_api::manage_neuron::Command::MakeProposal(Box::new(convert_proposal(&v, false))) } pb::manage_neuron::Command::RegisterVote(v) => { pb_api::manage_neuron::Command::RegisterVote(v.into()) @@ -1857,7 +1799,7 @@ impl From for pb_api::ProposalData { id: item.id, proposer: item.proposer, reject_cost_e8s: item.reject_cost_e8s, - proposal: item.proposal.map(|x| x.into()), + proposal: item.proposal.map(|x| convert_proposal(&x, false)), proposal_timestamp_seconds: item.proposal_timestamp_seconds, ballots: item .ballots @@ -2338,64 +2280,6 @@ impl From for pb::WaitForQuietState { } } -impl From for pb_api::ProposalInfo { - fn from(item: pb::ProposalInfo) -> Self { - Self { - id: item.id, - proposer: item.proposer, - reject_cost_e8s: item.reject_cost_e8s, - proposal: item.proposal.map(|x| x.into()), - proposal_timestamp_seconds: item.proposal_timestamp_seconds, - ballots: item - .ballots - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(), - latest_tally: item.latest_tally.map(|x| x.into()), - decided_timestamp_seconds: item.decided_timestamp_seconds, - executed_timestamp_seconds: item.executed_timestamp_seconds, - failed_timestamp_seconds: item.failed_timestamp_seconds, - failure_reason: item.failure_reason.map(|x| x.into()), - reward_event_round: item.reward_event_round, - topic: item.topic, - status: item.status, - reward_status: item.reward_status, - deadline_timestamp_seconds: item.deadline_timestamp_seconds, - derived_proposal_information: item.derived_proposal_information.map(|x| x.into()), - total_potential_voting_power: item.total_potential_voting_power, - } - } -} - -impl From for pb::ProposalInfo { - fn from(item: pb_api::ProposalInfo) -> Self { - Self { - id: item.id, - proposer: item.proposer, - reject_cost_e8s: item.reject_cost_e8s, - proposal: item.proposal.map(|x| x.into()), - proposal_timestamp_seconds: item.proposal_timestamp_seconds, - ballots: item - .ballots - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(), - latest_tally: item.latest_tally.map(|x| x.into()), - decided_timestamp_seconds: item.decided_timestamp_seconds, - executed_timestamp_seconds: item.executed_timestamp_seconds, - failed_timestamp_seconds: item.failed_timestamp_seconds, - failure_reason: item.failure_reason.map(|x| x.into()), - reward_event_round: item.reward_event_round, - topic: item.topic, - status: item.status, - reward_status: item.reward_status, - deadline_timestamp_seconds: item.deadline_timestamp_seconds, - derived_proposal_information: item.derived_proposal_information.map(|x| x.into()), - total_potential_voting_power: item.total_potential_voting_power, - } - } -} - impl From for pb_api::NetworkEconomics { fn from(item: pb::NetworkEconomics) -> Self { Self { @@ -2894,27 +2778,6 @@ impl From for pb_api::InstallCode { - fn from(item: pb::InstallCode) -> Self { - let wasm_module_hash = item - .wasm_module - .map(|wasm_module| super::calculate_hash(&wasm_module).to_vec()); - let arg = item.arg.unwrap_or_default(); - let arg_hash = if arg.is_empty() { - Some(vec![]) - } else { - Some(super::calculate_hash(&arg).to_vec()) - }; - - Self { - canister_id: item.canister_id, - install_mode: item.install_mode, - skip_stopping_before_installing: item.skip_stopping_before_installing, - wasm_module_hash, - arg_hash, - } - } -} impl From for pb::InstallCode { fn from(item: pb_api::InstallCode) -> Self { Self { @@ -3137,62 +3000,6 @@ impl From } } -impl From for pb_api::Governance { - fn from(item: pb::Governance) -> Self { - Self { - neurons: item - .neurons - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(), - proposals: item - .proposals - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(), - to_claim_transfers: item - .to_claim_transfers - .into_iter() - .map(|x| x.into()) - .collect(), - wait_for_quiet_threshold_seconds: item.wait_for_quiet_threshold_seconds, - economics: item.economics.map(|x| x.into()), - latest_reward_event: item.latest_reward_event.map(|x| x.into()), - in_flight_commands: item - .in_flight_commands - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(), - genesis_timestamp_seconds: item.genesis_timestamp_seconds, - node_providers: item.node_providers.into_iter().map(|x| x.into()).collect(), - default_followees: item - .default_followees - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(), - short_voting_period_seconds: item.short_voting_period_seconds, - neuron_management_voting_period_seconds: item.neuron_management_voting_period_seconds, - metrics: item.metrics.map(|x| x.into()), - most_recent_monthly_node_provider_rewards: item - .most_recent_monthly_node_provider_rewards - .map(|x| x.into()), - cached_daily_maturity_modulation_basis_points: item - .cached_daily_maturity_modulation_basis_points, - maturity_modulation_last_updated_at_timestamp_seconds: item - .maturity_modulation_last_updated_at_timestamp_seconds, - spawning_neurons: item.spawning_neurons, - making_sns_proposal: item.making_sns_proposal.map(|x| x.into()), - migrations: item.migrations.map(|x| x.into()), - topic_followee_index: item - .topic_followee_index - .into_iter() - .map(|(k, v)| (k, v.into())) - .collect(), - xdr_conversion_rate: item.xdr_conversion_rate.map(|x| x.into()), - restore_aging_summary: item.restore_aging_summary.map(|x| x.into()), - } - } -} impl From for pb::Governance { fn from(item: pb_api::Governance) -> Self { Self { @@ -3539,15 +3346,6 @@ impl From } } -impl From for pb_api::governance::MakingSnsProposal { - fn from(item: pb::governance::MakingSnsProposal) -> Self { - Self { - proposer_id: item.proposer_id, - caller: item.caller, - proposal: item.proposal.map(|x| x.into()), - } - } -} impl From for pb::governance::MakingSnsProposal { fn from(item: pb_api::governance::MakingSnsProposal) -> Self { Self { @@ -3744,21 +3542,6 @@ impl From for pb::ListProposalInfo { } } -impl From for pb_api::ListProposalInfoResponse { - fn from(item: pb::ListProposalInfoResponse) -> Self { - Self { - proposal_info: item.proposal_info.into_iter().map(|x| x.into()).collect(), - } - } -} -impl From for pb::ListProposalInfoResponse { - fn from(item: pb_api::ListProposalInfoResponse) -> Self { - Self { - proposal_info: item.proposal_info.into_iter().map(|x| x.into()).collect(), - } - } -} - impl From for pb_api::ListNeurons { fn from(item: pb::ListNeurons) -> Self { Self { diff --git a/rs/nns/governance/src/pb/mod.rs b/rs/nns/governance/src/pb/mod.rs index 849abfed4be..8aee644076d 100644 --- a/rs/nns/governance/src/pb/mod.rs +++ b/rs/nns/governance/src/pb/mod.rs @@ -1,5 +1,4 @@ use crate::pb::v1::ArchivedMonthlyNodeProviderRewards; -use ic_crypto_sha2::Sha256; use ic_stable_structures::{storable::Bound, Storable}; use prost::Message; use std::borrow::Cow; @@ -10,6 +9,7 @@ pub mod v1; mod conversions; mod convert_struct_to_enum; +pub mod proposal_conversions; impl Storable for ArchivedMonthlyNodeProviderRewards { fn to_bytes(&self) -> Cow<'_, [u8]> { @@ -25,13 +25,3 @@ impl Storable for ArchivedMonthlyNodeProviderRewards { const BOUND: Bound = Bound::Unbounded; } - -/// Calculates the SHA256 hash of the given bytes. -fn calculate_hash(bytes: &[u8]) -> [u8; 32] { - let mut wasm_sha = Sha256::new(); - wasm_sha.write(bytes); - wasm_sha.finish() -} - -#[cfg(test)] -mod tests; diff --git a/rs/nns/governance/src/pb/proposal_conversions.rs b/rs/nns/governance/src/pb/proposal_conversions.rs new file mode 100644 index 00000000000..0b565d09142 --- /dev/null +++ b/rs/nns/governance/src/pb/proposal_conversions.rs @@ -0,0 +1,379 @@ +use crate::{governance::EXECUTE_NNS_FUNCTION_PAYLOAD_LISTING_BYTES_MAX, pb::v1 as pb}; + +use ic_crypto_sha2::Sha256; +use ic_nns_common::pb::v1::NeuronId; +use ic_nns_governance_api::pb::v1 as pb_api; +use std::collections::{HashMap, HashSet}; + +/// Calculates the SHA256 hash of the given bytes. +fn calculate_hash(bytes: &[u8]) -> [u8; 32] { + let mut wasm_sha = Sha256::new(); + wasm_sha.write(bytes); + wasm_sha.finish() +} + +fn convert_execute_nns_function( + item: &pb::ExecuteNnsFunction, + omit_large_fields: bool, +) -> pb_api::ExecuteNnsFunction { + let pb::ExecuteNnsFunction { + nns_function, + payload, + } = item; + + let nns_function = *nns_function; + let payload = + if omit_large_fields && payload.len() > EXECUTE_NNS_FUNCTION_PAYLOAD_LISTING_BYTES_MAX { + vec![] + } else { + payload.clone() + }; + + pb_api::ExecuteNnsFunction { + nns_function, + payload, + } +} + +fn convert_install_code(item: &pb::InstallCode) -> pb_api::InstallCode { + let pb::InstallCode { + canister_id, + install_mode, + wasm_module, + arg, + skip_stopping_before_installing, + } = item; + + // Convert small fields. + let canister_id = canister_id.clone(); + let install_mode = *install_mode; + let skip_stopping_before_installing = *skip_stopping_before_installing; + + // Convert (potentially) large fields. + let wasm_module_hash = wasm_module + .as_ref() + .map(|wasm_module| calculate_hash(&wasm_module).to_vec()); + let arg_hash = match arg { + Some(arg) => { + // We could calculate the hash of an empty arg, but it would be confusing for the + // proposal reviewers, since the arg_hash is the only thing they can see, and it would + // not be obvious that the arg is empty. + if arg.is_empty() { + Some(vec![]) + } else { + Some(calculate_hash(&arg).to_vec()) + } + } + None => Some(vec![]), + }; + + pb_api::InstallCode { + canister_id, + install_mode, + skip_stopping_before_installing, + wasm_module_hash, + arg_hash, + } +} + +fn convert_ledger_parameters( + item: &pb::create_service_nervous_system::LedgerParameters, + omit_large_fields: bool, +) -> pb_api::create_service_nervous_system::LedgerParameters { + let pb::create_service_nervous_system::LedgerParameters { + transaction_fee, + token_name, + token_symbol, + token_logo, + } = item; + + let transaction_fee = *transaction_fee; + let token_name = token_name.clone(); + let token_symbol = token_symbol.clone(); + + let token_logo = if omit_large_fields { + None + } else { + token_logo.clone() + }; + + pb_api::create_service_nervous_system::LedgerParameters { + transaction_fee, + token_name, + token_symbol, + token_logo, + } +} + +fn convert_create_service_nervous_system( + item: &pb::CreateServiceNervousSystem, + omit_large_fields: bool, +) -> pb_api::CreateServiceNervousSystem { + let pb::CreateServiceNervousSystem { + name, + description, + url, + logo, + fallback_controller_principal_ids, + dapp_canisters, + initial_token_distribution, + swap_parameters, + ledger_parameters, + governance_parameters, + } = item; + + let name = name.clone(); + let description = description.clone(); + let url = url.clone(); + let fallback_controller_principal_ids = fallback_controller_principal_ids.clone(); + let dapp_canisters = dapp_canisters.clone(); + let initial_token_distribution = initial_token_distribution.clone().map(|x| x.into()); + let swap_parameters = swap_parameters.clone().map(|x| x.into()); + let governance_parameters = governance_parameters.clone().map(|x| x.into()); + + let logo = if omit_large_fields { + None + } else { + logo.clone() + }; + let ledger_parameters = ledger_parameters + .as_ref() + .map(|ledger_parameters| convert_ledger_parameters(ledger_parameters, omit_large_fields)); + + pb_api::CreateServiceNervousSystem { + name, + description, + url, + logo, + fallback_controller_principal_ids, + dapp_canisters, + initial_token_distribution, + swap_parameters, + ledger_parameters, + governance_parameters, + } +} + +fn convert_action( + item: &pb::proposal::Action, + omit_large_fields: bool, +) -> pb_api::proposal::Action { + match item { + // Trivial conversions + pb::proposal::Action::ManageNeuron(v) => { + pb_api::proposal::Action::ManageNeuron(Box::new(v.as_ref().clone().into())) + } + pb::proposal::Action::ManageNetworkEconomics(v) => { + pb_api::proposal::Action::ManageNetworkEconomics(v.clone().into()) + } + pb::proposal::Action::Motion(v) => pb_api::proposal::Action::Motion(v.clone().into()), + pb::proposal::Action::ApproveGenesisKyc(v) => { + pb_api::proposal::Action::ApproveGenesisKyc(v.clone().into()) + } + pb::proposal::Action::AddOrRemoveNodeProvider(v) => { + pb_api::proposal::Action::AddOrRemoveNodeProvider(v.clone().into()) + } + pb::proposal::Action::RewardNodeProvider(v) => { + pb_api::proposal::Action::RewardNodeProvider(v.clone().into()) + } + pb::proposal::Action::SetDefaultFollowees(v) => { + pb_api::proposal::Action::SetDefaultFollowees(v.clone().into()) + } + pb::proposal::Action::RewardNodeProviders(v) => { + pb_api::proposal::Action::RewardNodeProviders(v.clone().into()) + } + pb::proposal::Action::RegisterKnownNeuron(v) => { + pb_api::proposal::Action::RegisterKnownNeuron(v.clone().into()) + } + pb::proposal::Action::SetSnsTokenSwapOpenTimeWindow(v) => { + pb_api::proposal::Action::SetSnsTokenSwapOpenTimeWindow(v.clone().into()) + } + pb::proposal::Action::OpenSnsTokenSwap(v) => { + pb_api::proposal::Action::OpenSnsTokenSwap(v.clone().into()) + } + pb::proposal::Action::StopOrStartCanister(v) => { + pb_api::proposal::Action::StopOrStartCanister(v.clone().into()) + } + pb::proposal::Action::UpdateCanisterSettings(v) => { + pb_api::proposal::Action::UpdateCanisterSettings(v.clone().into()) + } + + // The action types with potentially large fields need to be converted in a way that avoids + // cloning the action first. + pb::proposal::Action::InstallCode(v) => { + pb_api::proposal::Action::InstallCode(convert_install_code(v)) + } + pb::proposal::Action::ExecuteNnsFunction(v) => { + pb_api::proposal::Action::ExecuteNnsFunction(convert_execute_nns_function( + v, + omit_large_fields, + )) + } + pb::proposal::Action::CreateServiceNervousSystem(v) => { + pb_api::proposal::Action::CreateServiceNervousSystem( + convert_create_service_nervous_system(v, omit_large_fields), + ) + } + } +} + +pub fn convert_proposal(item: &pb::Proposal, omit_large_fields: bool) -> pb_api::Proposal { + let pb::Proposal { + title, + summary, + url, + action, + } = item; + + // Convert (relatively) small fields + let title = title.clone(); + let summary = summary.clone(); + let url = url.clone(); + + // Convert action which is potentially large. + let action = action + .as_ref() + .map(|x| convert_action(x, omit_large_fields)); + + pb_api::Proposal { + title, + summary, + url, + action, + } +} + +fn convert_ballots( + all_ballots: &HashMap, + caller_neurons: &HashSet, +) -> HashMap { + let mut ballots = HashMap::new(); + for neuron_id in caller_neurons.iter() { + if let Some(v) = all_ballots.get(&neuron_id.id) { + ballots.insert(neuron_id.id, v.clone().into()); + } + } + ballots +} + +pub fn proposal_data_to_info( + data: &pb::ProposalData, + multi_query: bool, + omit_large_fields_requested: bool, + caller_neurons: &HashSet, + now_seconds: u64, + voting_period_seconds: impl Fn(pb::Topic) -> u64, +) -> pb_api::ProposalInfo { + // Calculate derived fields + let topic = data.topic() as i32; + let status = data.status() as i32; + let reward_status = data.reward_status(now_seconds, voting_period_seconds(data.topic())) as i32; + let deadline_timestamp_seconds = + Some(data.get_deadline_timestamp_seconds(voting_period_seconds(data.topic()))); + + // Trivially convert fields + let id = data.id; + let proposer = data.proposer; + let reject_cost_e8s = data.reject_cost_e8s; + let proposal_timestamp_seconds = data.proposal_timestamp_seconds; + let latest_tally = data.latest_tally.map(|x| x.into()); + let decided_timestamp_seconds = data.decided_timestamp_seconds; + let executed_timestamp_seconds = data.executed_timestamp_seconds; + let failed_timestamp_seconds = data.failed_timestamp_seconds; + let failure_reason = data.failure_reason.clone().map(|x| x.into()); + let reward_event_round = data.reward_event_round; + let derived_proposal_information = data.derived_proposal_information.clone().map(|x| x.into()); + let total_potential_voting_power = data.total_potential_voting_power; + + // Convert proposal which is potentially large. + let proposal_action = data + .proposal + .as_ref() + .and_then(|proposal| proposal.action.as_ref()); + let omit_large_fields = match (multi_query, proposal_action) { + // When not doing a multi-query (i.e. querying a single proposal), we never omit large + // fields. + (false, _) => false, + // Proposals which might contain WASM code are always omitted in multi-queries. + (true, Some(pb::proposal::Action::ExecuteNnsFunction(_))) => true, + // Otherwise, we respect the request for omitting large fields. + (true, _) => omit_large_fields_requested, + }; + let proposal = data + .proposal + .as_ref() + .map(|x| convert_proposal(x, omit_large_fields)); + + // Convert ballots which are potentially large. + let ballots = convert_ballots(&data.ballots, caller_neurons); + + pb_api::ProposalInfo { + id, + proposer, + reject_cost_e8s, + proposal, + proposal_timestamp_seconds, + ballots, + latest_tally, + decided_timestamp_seconds, + executed_timestamp_seconds, + failed_timestamp_seconds, + failure_reason, + reward_event_round, + topic, + status, + reward_status, + deadline_timestamp_seconds, + derived_proposal_information, + total_potential_voting_power, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use ic_base_types::PrincipalId; + + #[test] + fn install_code_internal_to_api() { + let test_cases = vec![ + ( + pb::InstallCode { + canister_id: Some(PrincipalId::new_user_test_id(1)), + install_mode: Some(pb::install_code::CanisterInstallMode::Install as i32), + skip_stopping_before_installing: None, + wasm_module: Some(vec![1, 2, 3]), + arg: Some(vec![]), + }, + pb_api::InstallCode { + canister_id: Some(PrincipalId::new_user_test_id(1)), + install_mode: Some(pb_api::install_code::CanisterInstallMode::Install as i32), + skip_stopping_before_installing: None, + wasm_module_hash: Some(Sha256::hash(&[1, 2, 3]).to_vec()), + arg_hash: Some(vec![]), + }, + ), + ( + pb::InstallCode { + canister_id: Some(PrincipalId::new_user_test_id(1)), + install_mode: Some(pb::install_code::CanisterInstallMode::Upgrade as i32), + skip_stopping_before_installing: Some(true), + wasm_module: Some(vec![1, 2, 3]), + arg: Some(vec![4, 5, 6]), + }, + pb_api::InstallCode { + canister_id: Some(PrincipalId::new_user_test_id(1)), + install_mode: Some(pb_api::install_code::CanisterInstallMode::Upgrade as i32), + skip_stopping_before_installing: Some(true), + wasm_module_hash: Some(Sha256::hash(&[1, 2, 3]).to_vec()), + arg_hash: Some(Sha256::hash(&[4, 5, 6]).to_vec()), + }, + ), + ]; + + for (internal, api) in test_cases { + assert_eq!(convert_install_code(&internal), api); + } + } +} diff --git a/rs/nns/governance/src/pb/tests.rs b/rs/nns/governance/src/pb/tests.rs index 9d69b19028d..8b137891791 100644 --- a/rs/nns/governance/src/pb/tests.rs +++ b/rs/nns/governance/src/pb/tests.rs @@ -1,47 +1 @@ -use super::*; -use crate::pb::v1 as pb; -use ic_base_types::PrincipalId; -use ic_nns_governance_api::pb::v1 as pb_api; - -#[test] -fn install_code_internal_to_api() { - let test_cases = vec![ - ( - pb::InstallCode { - canister_id: Some(PrincipalId::new_user_test_id(1)), - install_mode: Some(pb::install_code::CanisterInstallMode::Install as i32), - skip_stopping_before_installing: None, - wasm_module: Some(vec![1, 2, 3]), - arg: Some(vec![]), - }, - pb_api::InstallCode { - canister_id: Some(PrincipalId::new_user_test_id(1)), - install_mode: Some(pb_api::install_code::CanisterInstallMode::Install as i32), - skip_stopping_before_installing: None, - wasm_module_hash: Some(Sha256::hash(&[1, 2, 3]).to_vec()), - arg_hash: Some(vec![]), - }, - ), - ( - pb::InstallCode { - canister_id: Some(PrincipalId::new_user_test_id(1)), - install_mode: Some(pb::install_code::CanisterInstallMode::Upgrade as i32), - skip_stopping_before_installing: Some(true), - wasm_module: Some(vec![1, 2, 3]), - arg: Some(vec![4, 5, 6]), - }, - pb_api::InstallCode { - canister_id: Some(PrincipalId::new_user_test_id(1)), - install_mode: Some(pb_api::install_code::CanisterInstallMode::Upgrade as i32), - skip_stopping_before_installing: Some(true), - wasm_module_hash: Some(Sha256::hash(&[1, 2, 3]).to_vec()), - arg_hash: Some(Sha256::hash(&[4, 5, 6]).to_vec()), - }, - ), - ]; - - for (internal, api) in test_cases { - assert_eq!(pb_api::InstallCode::from(internal), api); - } -} diff --git a/rs/nns/governance/tests/governance.rs b/rs/nns/governance/tests/governance.rs index e62a2d89de3..ecc90ac18f0 100644 --- a/rs/nns/governance/tests/governance.rs +++ b/rs/nns/governance/tests/governance.rs @@ -85,10 +85,10 @@ use ic_nns_governance::{ CreateServiceNervousSystem, Empty, ExecuteNnsFunction, Governance as GovernanceProto, GovernanceChange, GovernanceError, IdealMatchedParticipationFunction, InstallCode, KnownNeuron, KnownNeuronData, ListNeurons, ListNeuronsResponse, ListProposalInfo, - ListProposalInfoResponse, ManageNeuron, ManageNeuronResponse, MonthlyNodeProviderRewards, - Motion, NetworkEconomics, Neuron, NeuronChange, NeuronState, NeuronType, NeuronsFundData, - NeuronsFundParticipation, NeuronsFundSnapshot, NnsFunction, NodeProvider, Proposal, - ProposalChange, ProposalData, ProposalDataChange, + ManageNeuron, ManageNeuronResponse, MonthlyNodeProviderRewards, Motion, NetworkEconomics, + Neuron, NeuronChange, NeuronState, NeuronType, NeuronsFundData, NeuronsFundParticipation, + NeuronsFundSnapshot, NnsFunction, NodeProvider, Proposal, ProposalChange, ProposalData, + ProposalDataChange, ProposalRewardStatus::{self, AcceptVotes, ReadyToSettle}, ProposalStatus::{self, Rejected}, RewardEvent, RewardNodeProvider, RewardNodeProviders, @@ -101,7 +101,10 @@ use ic_nns_governance::{ DEFAULT_VOTING_POWER_REFRESHED_TIMESTAMP_SECONDS, }; use ic_nns_governance_api::{ - pb::v1::CreateServiceNervousSystem as ApiCreateServiceNervousSystem, + pb::v1::{ + proposal::Action as ApiAction, Ballot as ApiBallot, + CreateServiceNervousSystem as ApiCreateServiceNervousSystem, ListProposalInfoResponse, + }, proposal_validation::validate_proposal_title, }; use ic_nns_governance_init::GovernanceCanisterInitPayloadBuilder; @@ -118,7 +121,7 @@ use ic_sns_wasm::pb::v1::{ }; use icp_ledger::{protobuf, AccountIdentifier, Memo, Subaccount, Tokens}; use lazy_static::lazy_static; -use maplit::{btreemap, hashmap}; +use maplit::{btreemap, hashmap, hashset}; use pretty_assertions::{assert_eq, assert_ne}; use proptest::prelude::{proptest, ProptestConfig}; use rand::{prelude::IteratorRandom, rngs::StdRng, Rng, SeedableRng}; @@ -4465,24 +4468,18 @@ fn test_get_neuron_ids_by_principal() { driver.get_fake_cmc(), ); - let mut principal2_neuron_ids = gov.get_neuron_ids_by_principal(&principal2); - principal2_neuron_ids.sort_unstable(); - assert_eq!( gov.get_neuron_ids_by_principal(&principal1), - vec![NeuronId { id: 1 }] - ); - assert_eq!( - principal2_neuron_ids, - vec![NeuronId { id: 2 }, NeuronId { id: 3 }, NeuronId { id: 4 }] + hashset! { NeuronId { id: 1 } } ); assert_eq!( - gov.get_neuron_ids_by_principal(&principal3), - Vec::::new() + gov.get_neuron_ids_by_principal(&principal2), + hashset! { NeuronId { id: 2 }, NeuronId { id: 3 }, NeuronId { id: 4 } } ); + assert_eq!(gov.get_neuron_ids_by_principal(&principal3), hashset! {}); assert_eq!( gov.get_neuron_ids_by_principal(&principal4), - vec![NeuronId { id: 4 }] + hashset! { NeuronId { id: 4 } } ); } @@ -4644,7 +4641,7 @@ fn create_mature_neuron(dissolved: bool) -> (fake::FakeDriver, Governance, Neuro ..Default::default() } ); - assert_eq!(gov.get_neuron_ids_by_principal(&from), vec![id]); + assert_eq!(gov.get_neuron_ids_by_principal(&from), hashset! {id}); // Dissolve the neuron if `dissolved` is true if dissolved { @@ -5783,11 +5780,8 @@ fn test_neuron_split() { parent_neuron.voting_power_refreshed_timestamp_seconds, ); - let mut neuron_ids = governance.get_neuron_ids_by_principal(&from); - neuron_ids.sort_unstable(); - let mut expected_neuron_ids = vec![id, child_nid]; - expected_neuron_ids.sort_unstable(); - assert_eq!(neuron_ids, expected_neuron_ids); + let neuron_ids = governance.get_neuron_ids_by_principal(&from); + assert_eq!(neuron_ids, hashset! {id, child_nid}); } #[test] @@ -7217,7 +7211,12 @@ fn test_manage_and_reward_node_providers() { let info = gov .get_proposal_info(&PrincipalId::new_anonymous(), pid) .unwrap(); - assert_eq!(info.status(), ProposalStatus::Failed, "info: {:?}", info); + assert_eq!( + info.status, + ProposalStatus::Failed as i32, + "info: {:?}", + info + ); assert_eq!( info.failure_reason.as_ref().unwrap().error_type, ErrorType::NotFound as i32, @@ -7568,7 +7567,12 @@ fn test_manage_and_reward_multiple_node_providers() { let info = gov .get_proposal_info(&PrincipalId::new_anonymous(), pid) .unwrap(); - assert_eq!(info.status(), ProposalStatus::Failed, "info: {:?}", info); + assert_eq!( + info.status, + ProposalStatus::Failed as i32, + "info: {:?}", + info + ); assert_eq!( info.failure_reason.as_ref().unwrap().error_type, ErrorType::NotFound as i32, @@ -8030,7 +8034,7 @@ fn test_get_proposal_info() { let action = result.proposal.unwrap().action.unwrap(); assert_matches!( action, - proposal::Action::ExecuteNnsFunction(eu) if eu.payload == [1, 2, 3] + ApiAction::ExecuteNnsFunction(eu) if eu.payload == [1, 2, 3] ); } @@ -8068,7 +8072,7 @@ fn test_list_proposals_removes_execute_nns_function_payload() { .unwrap(); assert_matches!( action, - proposal::Action::ExecuteNnsFunction(eu) if eu.payload.is_empty() + ApiAction::ExecuteNnsFunction(eu) if eu.payload.is_empty() ); } @@ -8106,7 +8110,7 @@ fn test_list_proposals_retains_execute_nns_function_payload() { .unwrap(); assert_matches!( action, - proposal::Action::ExecuteNnsFunction(eu) + ApiAction::ExecuteNnsFunction(eu) if eu.payload.len() == EXECUTE_NNS_FUNCTION_PAYLOAD_LISTING_BYTES_MAX ); @@ -8141,7 +8145,7 @@ fn test_get_pending_proposals_removes_execute_nns_function_payload() { .unwrap(); assert_matches!( action, - proposal::Action::ExecuteNnsFunction(eu) if eu.payload.is_empty() + ApiAction::ExecuteNnsFunction(eu) if eu.payload.is_empty() ); } @@ -8800,7 +8804,7 @@ fn test_filter_proposal_ballots() { .proposal_info[0] .ballots, hashmap! { - 1 => Ballot { + 1 => ApiBallot { vote: Vote::Yes as i32, voting_power: 1, }, @@ -8812,7 +8816,7 @@ fn test_filter_proposal_ballots() { .proposal_info[0] .ballots, hashmap! { - 2 => Ballot { + 2 => ApiBallot { vote: Vote::Yes as i32, voting_power: 2, }, @@ -8824,7 +8828,7 @@ fn test_filter_proposal_ballots() { .proposal_info[0] .ballots, hashmap! { - 1 => Ballot { + 1 => ApiBallot { vote: Vote::Yes as i32, voting_power: 1, }, @@ -8974,7 +8978,7 @@ fn test_omit_large_fields() { ); fn get_logo(list_proposals_response: ListProposalInfoResponse) -> Option { - let Action::CreateServiceNervousSystem(create_service_nervous_system) = + let ApiAction::CreateServiceNervousSystem(create_service_nervous_system) = list_proposals_response.proposal_info[0] .proposal .clone() @@ -9078,8 +9082,9 @@ async fn test_max_number_of_proposals_with_ballots() { .unwrap(); } assert_eq!( + gov.get_pending_proposals(&PrincipalId::new_anonymous()) + .len(), MAX_NUMBER_OF_PROPOSALS_WITH_BALLOTS, - gov.get_pending_proposals_data().count() ); // Let's try one more. It should be rejected. assert_matches!(gov.make_proposal( @@ -11881,8 +11886,8 @@ async fn test_known_neurons() { assert_eq!( gov.get_proposal_info(&principal(3), failed_proposal_id) .unwrap() - .status(), - ProposalStatus::Failed + .status, + ProposalStatus::Failed as i32 ); // Check that the state is the same as before the last proposal.