Deleted ListNeurons (and ListNeuronsResponse) from NNS governance.pro…

…to. We should be able to delete most if not all request and response types from governance.proto, because we do not store those in stable memory. This shows how we would delete other request and response types. This also shows that such deletions are fairly straightforward.

rs/nns/governance/canbench/canbench_results.yml

@@ -1,55 +1,55 @@
 benches:
   add_neuron_active_maximum:
     total:
-      instructions: 42470580
+      instructions: 42473815
       heap_increase: 1
       stable_memory_increase: 0
     scopes: {}
   add_neuron_active_typical:
     total:
-      instructions: 2156095
+      instructions: 2156222
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
   add_neuron_inactive_maximum:
     total:
-      instructions: 111969854
+      instructions: 111973099
       heap_increase: 1
       stable_memory_increase: 0
     scopes: {}
   add_neuron_inactive_typical:
     total:
-      instructions: 8450397
+      instructions: 8450534
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
   cascading_vote_all_heap:
     total:
-      instructions: 34764213
+      instructions: 34785713
       heap_increase: 0
       stable_memory_increase: 128
     scopes: {}
   cascading_vote_heap_neurons_stable_index:
     total:
-      instructions: 60897882
+      instructions: 60919382
       heap_increase: 0
       stable_memory_increase: 128
     scopes: {}
   cascading_vote_stable_everything:
     total:
-      instructions: 188426120
+      instructions: 188428219
       heap_increase: 0
       stable_memory_increase: 128
     scopes: {}
   cascading_vote_stable_neurons_with_heap_index:
     total:
-      instructions: 162159055
+      instructions: 162161154
       heap_increase: 0
       stable_memory_increase: 128
     scopes: {}
   centralized_following_all_stable:
     total:
-      instructions: 78085489
+      instructions: 78085643
       heap_increase: 0
       stable_memory_increase: 128
     scopes: {}
@@ -61,7 +61,7 @@ benches:
     scopes: {}
   draw_maturity_from_neurons_fund_heap:
     total:
-      instructions: 7455504
+      instructions: 7466404
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
@@ -79,13 +79,13 @@ benches:
     scopes: {}
   list_active_neurons_fund_neurons_stable:
     total:
-      instructions: 2742494
+      instructions: 2741594
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
   list_neurons_heap:
     total:
-      instructions: 4717287
+      instructions: 4872808
       heap_increase: 9
       stable_memory_increase: 0
     scopes: {}
@@ -97,13 +97,13 @@ benches:
     scopes: {}
   list_neurons_ready_to_unstake_maturity_stable:
     total:
-      instructions: 41328019
+      instructions: 41328010
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
   list_neurons_stable:
     total:
-      instructions: 113419930
+      instructions: 113505838
       heap_increase: 5
       stable_memory_increase: 0
     scopes: {}
@@ -115,19 +115,19 @@ benches:
     scopes: {}
   list_ready_to_spawn_neuron_ids_stable:
     total:
-      instructions: 41299922
+      instructions: 41299913
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
   neuron_data_validation_heap:
     total:
-      instructions: 406681837
+      instructions: 406689244
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
   neuron_data_validation_stable:
     total:
-      instructions: 362505461
+      instructions: 362506975
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
@@ -145,13 +145,13 @@ benches:
     scopes: {}
   range_neurons_performance:
     total:
-      instructions: 56426715
+      instructions: 56428115
       heap_increase: 0
       stable_memory_increase: 0
     scopes: {}
   single_vote_all_stable:
     total:
-      instructions: 2801395
+      instructions: 2801398
       heap_increase: 0
       stable_memory_increase: 128
     scopes: {}

rs/nns/governance/canister/canister.rs

@@ -812,7 +812,7 @@ fn list_proposals(req: ListProposalInfo) -> ListProposalInfoResponse {
 #[query]
 fn list_neurons(req: ListNeurons) -> ListNeuronsResponse {
     debug_log("list_neurons");
-    governance().list_neurons(&(req.into()), caller()).into()
+    governance().list_neurons(&req, caller())
 }
 
 #[query]

rs/nns/governance/proto/ic_nns_governance/pb/v1/governance.proto

@@ -2630,51 +2630,6 @@ 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,
-// the list of neuron IDs of neurons for which the caller is the
-// controller or one of the hot keys.
-message ListNeurons {
-  option (ic_base_types.pb.v1.tui_signed_message) = true;
-  // The neurons to get information about. The "requested list"
-  // contains all of these neuron IDs.
-  repeated fixed64 neuron_ids = 1 [(ic_base_types.pb.v1.tui_signed_display_q2_2021) = true];
-  // If true, the "requested list" also contains the neuron ID of the
-  // neurons that the calling principal is authorized to read.
-  bool include_neurons_readable_by_caller = 2 [(ic_base_types.pb.v1.tui_signed_display_q2_2021) = true];
-  // Whether to also include empty neurons readable by the caller. This field only has an effect
-  // when `include_neurons_readable_by_caller` is true. If a neuron's id already exists in the
-  // `neuron_ids` field, then the neuron will be included in the response regardless of the value of
-  // this field. Since the previous behavior was to always include empty neurons readable by caller,
-  // if this field is not provided, it defaults to true, in order to maintain backwards
-  // compatibility. Here, being "empty" means 0 stake, 0 maturity and 0 staked maturity.
-  optional bool include_empty_neurons_readable_by_caller = 3;
-  // If this is set to true, and a neuron in the "requested list" has its
-  // visibility set to public, then, it will (also) be included in the
-  // full_neurons field in the response (which is of type ListNeuronsResponse).
-  // Note that this has no effect on which neurons are in the "requested list".
-  // In particular, this does not cause all public neurons to become part of the
-  // requested list. In general, you probably want to set this to true, but
-  // since this feature was added later, it is opt in to avoid confusing
-  // existing (unmigrated) callers.
-  optional bool include_public_neurons_in_full_neurons = 4;
-}
-
-// A response to a `ListNeurons` request.
-//
-// The "requested list" is described in `ListNeurons`.
-message ListNeuronsResponse {
-  // For each neuron ID in the "requested list", if this neuron exists,
-  // its `NeuronInfo` at the time of the call will be in this map.
-  map<fixed64, NeuronInfo> neuron_infos = 1;
-  // For each neuron ID in the "requested list", if the neuron exists,
-  // and the caller is authorized to read the full neuron (controller,
-  // hot key, or controller or hot key of some followee on the
-  // `ManageNeuron` topic).
-  repeated Neuron full_neurons = 2;
-}
-
 // A response to "ListKnownNeurons"
 message ListKnownNeuronsResponse {
   // List of known neurons.

rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs

@@ -3927,72 +3927,6 @@ pub struct ListProposalInfoResponse {
     #[prost(message, repeated, tag = "1")]
     pub proposal_info: ::prost::alloc::vec::Vec<ProposalInfo>,
 }
-/// 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,
-/// the list of neuron IDs of neurons for which the caller is the
-/// controller or one of the hot keys.
-#[derive(
-    candid::CandidType,
-    candid::Deserialize,
-    serde::Serialize,
-    comparable::Comparable,
-    Clone,
-    PartialEq,
-    ::prost::Message,
-)]
-pub struct ListNeurons {
-    /// The neurons to get information about. The "requested list"
-    /// contains all of these neuron IDs.
-    #[prost(fixed64, repeated, packed = "false", tag = "1")]
-    pub neuron_ids: ::prost::alloc::vec::Vec<u64>,
-    /// If true, the "requested list" also contains the neuron ID of the
-    /// neurons that the calling principal is authorized to read.
-    #[prost(bool, tag = "2")]
-    pub include_neurons_readable_by_caller: bool,
-    /// Whether to also include empty neurons readable by the caller. This field only has an effect
-    /// when `include_neurons_readable_by_caller` is true. If a neuron's id already exists in the
-    /// `neuron_ids` field, then the neuron will be included in the response regardless of the value of
-    /// this field. Since the previous behavior was to always include empty neurons readable by caller,
-    /// if this field is not provided, it defaults to true, in order to maintain backwards
-    /// compatibility. Here, being "empty" means 0 stake, 0 maturity and 0 staked maturity.
-    #[prost(bool, optional, tag = "3")]
-    pub include_empty_neurons_readable_by_caller: ::core::option::Option<bool>,
-    /// If this is set to true, and a neuron in the "requested list" has its
-    /// visibility set to public, then, it will (also) be included in the
-    /// full_neurons field in the response (which is of type ListNeuronsResponse).
-    /// Note that this has no effect on which neurons are in the "requested list".
-    /// In particular, this does not cause all public neurons to become part of the
-    /// requested list. In general, you probably want to set this to true, but
-    /// since this feature was added later, it is opt in to avoid confusing
-    /// existing (unmigrated) callers.
-    #[prost(bool, optional, tag = "4")]
-    pub include_public_neurons_in_full_neurons: ::core::option::Option<bool>,
-}
-/// A response to a `ListNeurons` request.
-///
-/// The "requested list" is described in `ListNeurons`.
-#[derive(
-    candid::CandidType,
-    candid::Deserialize,
-    serde::Serialize,
-    comparable::Comparable,
-    Clone,
-    PartialEq,
-    ::prost::Message,
-)]
-pub struct ListNeuronsResponse {
-    /// For each neuron ID in the "requested list", if this neuron exists,
-    /// its `NeuronInfo` at the time of the call will be in this map.
-    #[prost(map = "fixed64, message", tag = "1")]
-    pub neuron_infos: ::std::collections::HashMap<u64, NeuronInfo>,
-    /// For each neuron ID in the "requested list", if the neuron exists,
-    /// and the caller is authorized to read the full neuron (controller,
-    /// hot key, or controller or hot key of some followee on the
-    /// `ManageNeuron` topic).
-    #[prost(message, repeated, tag = "2")]
-    pub full_neurons: ::prost::alloc::vec::Vec<Neuron>,
-}
 /// A response to "ListKnownNeurons"
 #[derive(
     candid::CandidType,

rs/nns/governance/src/governance.rs

@@ -53,10 +53,10 @@ use crate::{
         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,
+        InstallCode, KnownNeuron, ListKnownNeuronsResponse, 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,
@@ -94,7 +94,11 @@ use ic_nns_constants::{
     SUBNET_RENTAL_CANISTER_ID,
 };
 use ic_nns_governance_api::{
-    pb::v1::CreateServiceNervousSystem as ApiCreateServiceNervousSystem, proposal_validation,
+    pb::v1::{
+        self as api, CreateServiceNervousSystem as ApiCreateServiceNervousSystem, ListNeurons,
+        ListNeuronsResponse,
+    },
+    proposal_validation,
     subnet_rental::SubnetRentalRequest,
 };
 use ic_protobuf::registry::dc::v1::AddOrRemoveDataCentersProposalPayload;
@@ -2470,7 +2474,14 @@ impl Governance {
             // requested_neuron_ids are supplied by the caller.
             let _ignore_when_neuron_not_found = self.with_neuron(&neuron_id, |neuron| {
                 // Populate neuron_infos.
-                neuron_infos.insert(neuron_id.id, neuron.get_neuron_info(self.voting_power_economics(), now, caller));
+                let neuron_info = neuron.get_neuron_info(self.voting_power_economics(), now, caller);
+
+                // We will be able to get rid of this conversion once we delete
+                // NeuronInfo from governance.proto. That should be possible,
+                // since we do not store NeuronInfo in stable memory.
+                let neuron_info = api::NeuronInfo::from(neuron_info);
+
+                neuron_infos.insert(neuron_id.id, neuron_info);
 
                 // Populate full_neurons.
                 let let_caller_read_full_neuron =
@@ -2489,7 +2500,7 @@ impl Governance {
                     // we need to do a larger refactoring to use the correct API types instead of the internal
                     // governance proto at this level.
                     proto.recent_ballots = neuron.sorted_recent_ballots();
-                    full_neurons.push(proto);
+                    full_neurons.push(api::Neuron::from(proto));
                 }
             });
         }

rs/nns/governance/src/governance/benches.rs

@@ -4,8 +4,7 @@ use crate::{
     neuron_store::NeuronStore,
     pb::v1::{
         neuron::Followees, proposal::Action, Ballot, BallotInfo, Governance as GovernanceProto,
-        KnownNeuron, ListNeurons, Neuron as NeuronProto, ProposalData, Topic, Vote,
-        VotingPowerEconomics,
+        KnownNeuron, Neuron as NeuronProto, ProposalData, Topic, Vote, VotingPowerEconomics,
     },
     temporarily_disable_allow_active_neurons_in_stable_memory,
     temporarily_disable_migrate_active_neurons_to_stable_memory,
@@ -22,6 +21,7 @@ use ic_nns_common::{
     pb::v1::{NeuronId as NeuronIdProto, ProposalId},
     types::NeuronId,
 };
+use ic_nns_governance_api::pb::v1::ListNeurons;
 use icp_ledger::Subaccount;
 use maplit::hashmap;
 use rand::{Rng, SeedableRng};