Skip to content

Commit

Permalink
feat(nns): list_neurons supports querying by subaccount (#3592)
Browse files Browse the repository at this point in the history
# Why
This allows users who know the subaccounts of their neurons (i.e. they
know the nonce and the principal used when the neurons are created) to
specify which neurons they would like to list without needing to know
the neuron ids (which are generated at random).

# What 

This adds a new API field to `list_neurons` called `neuron_subaccounts`,
which works in the same way that `neuron_ids` works. By specifying which
subaccounts the user would like to list, they are able to retrieve
neurons in the same way that they also can provide neuron ids. Both of
these are compatible, and in the case of overlaps, no duplicates are
returned.
  • Loading branch information
max-dfinity authored Feb 5, 2025
1 parent 54b7f0e commit 413a393
Show file tree
Hide file tree
Showing 18 changed files with 363 additions and 49 deletions.
3 changes: 2 additions & 1 deletion rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,8 @@ pub mod nns {
include_empty_neurons_readable_by_caller: Some(true),
include_public_neurons_in_full_neurons: None,
page_number: None,
page_size: None
page_size: None,
neuron_subaccounts: None,
})
.unwrap(),
)
Expand Down
69 changes: 62 additions & 7 deletions rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3463,6 +3463,52 @@ pub struct ListProposalInfo {
pub struct ListProposalInfoResponse {
pub proposal_info: Vec<ProposalInfo>,
}

/// The same as ListNeurons, but only used in list_neurons_pb, which is deprecated.
/// This is temporarily split out so that the API changes to list_neurons do not have to
/// follow both candid and protobuf standards for changes, which simplifies the API design
/// considerably.
///
/// This type should be removed when list_neurons_pb is finally deprecated.
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
pub struct ListNeuronsProto {
/// 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: 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. The default value is false (i.e. `None` is treated as `Some(false)`). Here,
/// being "empty" means 0 stake, 0 maturity and 0 staked maturity.
#[prost(bool, optional, tag = "3")]
pub include_empty_neurons_readable_by_caller: 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: Option<bool>,
/// If this is set, we return the batch of neurons at a given page, using the `page_size` to
/// determine how many neurons are returned in each page.
#[prost(uint64, optional, tag = "5")]
pub page_number: Option<u64>,
/// If this is set, we use the page limit provided to determine how large pages will be.
/// This cannot be greater than MAX_LIST_NEURONS_RESULTS, which is set to 500.
/// If not set, this defaults to MAX_LIST_NEURONS_RESULTS.
#[prost(uint64, optional, tag = "6")]
pub page_size: Option<u64>,
}
/// 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,
Expand All @@ -3484,22 +3530,19 @@ pub struct ListProposalInfoResponse {
/// will be returned in the current page.
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
#[derive(Clone, Debug, Default, PartialEq)]
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: 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. The default value is false (i.e. `None` is treated as `Some(false)`). Here,
/// being "empty" means 0 stake, 0 maturity and 0 staked maturity.
#[prost(bool, optional, tag = "3")]
pub include_empty_neurons_readable_by_caller: 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
Expand All @@ -3509,18 +3552,30 @@ pub struct ListNeurons {
/// 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: Option<bool>,
/// If this is set, we return the batch of neurons at a given page, using the `page_size` to
/// determine how many neurons are returned in each page.
#[prost(uint64, optional, tag = "5")]
pub page_number: Option<u64>,
/// If this is set, we use the page limit provided to determine how large pages will be.
/// This cannot be greater than MAX_LIST_NEURONS_RESULTS, which is set to 500.
/// If not set, this defaults to MAX_LIST_NEURONS_RESULTS.
#[prost(uint64, optional, tag = "6")]
pub page_size: Option<u64>,
/// A list of neurons by subaccounts to return in the response. If the neurons are not
/// found by subaccount, no error is returned, but the page will still be returned.
pub neuron_subaccounts: Option<Vec<list_neurons::NeuronSubaccount>>,
}

pub mod list_neurons {
/// A type for the request to list neurons.
#[derive(candid::CandidType, candid::Deserialize, serde::Serialize, comparable::Comparable)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, Debug, PartialEq)]
pub struct NeuronSubaccount {
#[serde(with = "serde_bytes")]
pub subaccount: Vec<u8>,
}
}

/// A response to a `ListNeurons` request.
///
/// The "requested list" is described in `ListNeurons`.
Expand Down
23 changes: 20 additions & 3 deletions rs/nns/governance/api/src/pb.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::pb::v1::{
governance::migration::MigrationStatus, governance_error::ErrorType, neuron::DissolveState,
CreateServiceNervousSystem, GovernanceError, NetworkEconomics, Neuron, NeuronState,
NeuronsFundEconomics, NeuronsFundMatchedFundingCurveCoefficients, VotingPowerEconomics,
XdrConversionRate,
CreateServiceNervousSystem, GovernanceError, ListNeurons, ListNeuronsProto, NetworkEconomics,
Neuron, NeuronState, NeuronsFundEconomics, NeuronsFundMatchedFundingCurveCoefficients,
VotingPowerEconomics, XdrConversionRate,
};
use ic_nervous_system_common::{ONE_DAY_SECONDS, ONE_MONTH_SECONDS};
use ic_nervous_system_proto::pb::v1::{Decimal, Duration, GlobalTimeOfDay, Percentage};
Expand Down Expand Up @@ -264,3 +264,20 @@ impl CreateServiceNervousSystem {
Ok((swap_start_timestamp_seconds, swap_due_timestamp_seconds))
}
}

impl From<ListNeuronsProto> for ListNeurons {
fn from(list_neurons_proto: ListNeuronsProto) -> Self {
Self {
neuron_ids: list_neurons_proto.neuron_ids,
include_neurons_readable_by_caller: list_neurons_proto
.include_neurons_readable_by_caller,
include_empty_neurons_readable_by_caller: list_neurons_proto
.include_empty_neurons_readable_by_caller,
include_public_neurons_in_full_neurons: list_neurons_proto
.include_public_neurons_in_full_neurons,
page_number: list_neurons_proto.page_number,
page_size: list_neurons_proto.page_size,
neuron_subaccounts: None,
}
}
}
68 changes: 40 additions & 28 deletions rs/nns/governance/canbench/canbench_results.yml
Original file line number Diff line number Diff line change
@@ -1,169 +1,181 @@
benches:
add_neuron_active_maximum:
total:
instructions: 42752796
instructions: 42752810
heap_increase: 1
stable_memory_increase: 0
scopes: {}
add_neuron_active_typical:
total:
instructions: 2170658
instructions: 2170672
heap_increase: 0
stable_memory_increase: 0
scopes: {}
add_neuron_inactive_maximum:
total:
instructions: 112624375
instructions: 112624946
heap_increase: 1
stable_memory_increase: 0
scopes: {}
add_neuron_inactive_typical:
total:
instructions: 8497036
instructions: 8497607
heap_increase: 0
stable_memory_increase: 0
scopes: {}
cascading_vote_all_heap:
total:
instructions: 35676146
instructions: 35610228
heap_increase: 0
stable_memory_increase: 128
scopes: {}
cascading_vote_heap_neurons_stable_index:
total:
instructions: 61811185
instructions: 61744033
heap_increase: 0
stable_memory_increase: 128
scopes: {}
cascading_vote_stable_everything:
total:
instructions: 188611915
instructions: 189084050
heap_increase: 0
stable_memory_increase: 128
scopes: {}
cascading_vote_stable_neurons_with_heap_index:
total:
instructions: 162343480
instructions: 162816849
heap_increase: 0
stable_memory_increase: 128
scopes: {}
centralized_following_all_stable:
total:
instructions: 78265237
instructions: 78519736
heap_increase: 0
stable_memory_increase: 128
scopes: {}
compute_ballots_for_new_proposal_with_stable_neurons:
total:
instructions: 2230000
instructions: 2265911
heap_increase: 0
stable_memory_increase: 0
scopes: {}
draw_maturity_from_neurons_fund_heap:
total:
instructions: 7656798
instructions: 7607998
heap_increase: 0
stable_memory_increase: 0
scopes: {}
draw_maturity_from_neurons_fund_stable:
total:
instructions: 12339498
instructions: 12444384
heap_increase: 0
stable_memory_increase: 0
scopes: {}
list_active_neurons_fund_neurons_heap:
total:
instructions: 435463
instructions: 435492
heap_increase: 0
stable_memory_increase: 0
scopes: {}
list_active_neurons_fund_neurons_stable:
total:
instructions: 2820000
instructions: 2819667
heap_increase: 0
stable_memory_increase: 0
scopes: {}
list_neurons_by_subaccount_heap:
total:
instructions: 7551696
heap_increase: 9
stable_memory_increase: 0
scopes: {}
list_neurons_by_subaccount_stable:
total:
instructions: 111661652
heap_increase: 5
stable_memory_increase: 0
scopes: {}
list_neurons_heap:
total:
instructions: 4950000
instructions: 4963821
heap_increase: 9
stable_memory_increase: 0
scopes: {}
list_neurons_ready_to_unstake_maturity_heap:
total:
instructions: 158253
instructions: 158195
heap_increase: 0
stable_memory_increase: 0
scopes: {}
list_neurons_ready_to_unstake_maturity_stable:
total:
instructions: 43300000
instructions: 43332558
heap_increase: 0
stable_memory_increase: 0
scopes: {}
list_neurons_stable:
total:
instructions: 113606723
instructions: 113665560
heap_increase: 5
stable_memory_increase: 0
scopes: {}
list_proposals:
total:
instructions: 126040
instructions: 126041
heap_increase: 0
stable_memory_increase: 0
scopes: {}
list_ready_to_spawn_neuron_ids_heap:
total:
instructions: 132847
instructions: 132789
heap_increase: 0
stable_memory_increase: 0
scopes: {}
list_ready_to_spawn_neuron_ids_stable:
total:
instructions: 43270000
instructions: 43304554
heap_increase: 0
stable_memory_increase: 0
scopes: {}
neuron_data_validation_heap:
total:
instructions: 406853184
instructions: 407435747
heap_increase: 0
stable_memory_increase: 0
scopes: {}
neuron_data_validation_stable:
total:
instructions: 362648372
instructions: 363402784
heap_increase: 0
stable_memory_increase: 0
scopes: {}
neuron_metrics_calculation_heap:
total:
instructions: 1498869
instructions: 1485668
heap_increase: 0
stable_memory_increase: 0
scopes: {}
neuron_metrics_calculation_stable:
total:
instructions: 3027495
instructions: 3085347
heap_increase: 0
stable_memory_increase: 0
scopes: {}
range_neurons_performance:
total:
instructions: 56447340
instructions: 56514456
heap_increase: 0
stable_memory_increase: 0
scopes: {}
single_vote_all_stable:
total:
instructions: 2805871
instructions: 2806911
heap_increase: 0
stable_memory_increase: 128
scopes: {}
update_recent_ballots_stable_memory:
total:
instructions: 274000
instructions: 275035
heap_increase: 0
stable_memory_increase: 0
scopes: {}
Expand Down
16 changes: 9 additions & 7 deletions rs/nns/governance/canister/canister.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ use ic_nns_governance_api::{
manage_neuron_response, ClaimOrRefreshNeuronFromAccount,
ClaimOrRefreshNeuronFromAccountResponse, GetNeuronsFundAuditInfoRequest,
GetNeuronsFundAuditInfoResponse, Governance as ApiGovernanceProto, GovernanceError,
ListKnownNeuronsResponse, ListNeurons, ListNeuronsResponse, ListNodeProviderRewardsRequest,
ListNodeProviderRewardsResponse, ListNodeProvidersResponse, ListProposalInfo,
ListProposalInfoResponse, ManageNeuronCommandRequest, ManageNeuronRequest,
ManageNeuronResponse, MonthlyNodeProviderRewards, NetworkEconomics, Neuron, NeuronInfo,
NodeProvider, Proposal, ProposalInfo, RestoreAgingSummary, RewardEvent,
ListKnownNeuronsResponse, ListNeurons, ListNeuronsProto, ListNeuronsResponse,
ListNodeProviderRewardsRequest, ListNodeProviderRewardsResponse, ListNodeProvidersResponse,
ListProposalInfo, ListProposalInfoResponse, ManageNeuronCommandRequest,
ManageNeuronRequest, ManageNeuronResponse, MonthlyNodeProviderRewards, NetworkEconomics,
Neuron, NeuronInfo, NodeProvider, Proposal, ProposalInfo, RestoreAgingSummary, RewardEvent,
SettleCommunityFundParticipation, SettleNeuronsFundParticipationRequest,
SettleNeuronsFundParticipationResponse, UpdateNodeProvider, Vote,
},
Expand Down Expand Up @@ -907,8 +907,10 @@ fn list_neurons_pb() {
);

ic_cdk::setup();
let request = ListNeurons::decode(&arg_data_raw()[..]).expect("Could not decode ListNeurons");
let res: ListNeuronsResponse = list_neurons(request);
let request =
ListNeuronsProto::decode(&arg_data_raw()[..]).expect("Could not decode ListNeuronsProto");
let candid_request = ListNeurons::from(request);
let res: ListNeuronsResponse = list_neurons(candid_request);
let mut buf = Vec::with_capacity(res.encoded_len());
res.encode(&mut buf)
.map_err(|e| e.to_string())
Expand Down
Loading

0 comments on commit 413a393

Please sign in to comment.