Skip to content

Commit

Permalink
Merge branch 'jason/NNS1-3133-1' into 'master'
Browse files Browse the repository at this point in the history
feat(nns): Add an option in list_neurons to include empty neurons readable by caller

Add an option in list_neurons to include empty neurons readable by caller, without implementation yet.

The following implementation should be backward compatible, treating `null` as `opt true`.

An alternative is to add the option as `exclude_...` so that `null` is equivalent to `opt false` (which is more intuitive). The reason for the current approach is that in the long term we'd like to encourage clients to not include empty neurons, which lowers the load on the system querying empty neurons. We will propose treating `null` as `opt false` after clients have time to set their preference based on their use case. 

See merge request dfinity-lab/public/ic!19968
  • Loading branch information
jasonz-dfinity committed Jun 24, 2024
2 parents 5ad19d1 + f16b6a7 commit d7d4555
Show file tree
Hide file tree
Showing 10 changed files with 32 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -656,6 +656,7 @@ pub mod nns {
Encode!(&ListNeurons {
neuron_ids: vec![],
include_neurons_readable_by_caller: true,
include_empty_neurons_readable_by_caller: None,
})
.unwrap(),
)
Expand Down
8 changes: 8 additions & 0 deletions rs/nns/governance/api/src/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2766,6 +2766,14 @@ pub struct ListNeurons {
/// 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>,
}
/// A response to a `ListNeurons` request.
///
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/canister/governance.did
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ type LedgerParameters = record {
type ListKnownNeuronsResponse = record { known_neurons : vec KnownNeuron };
type ListNeurons = record {
neuron_ids : vec nat64;
include_empty_neurons_readable_by_caller : opt bool;
include_neurons_readable_by_caller : bool;
};
type ListNeuronsResponse = record {
Expand Down
1 change: 1 addition & 0 deletions rs/nns/governance/canister/governance_test.did
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ type LedgerParameters = record {
type ListKnownNeuronsResponse = record { known_neurons : vec KnownNeuron };
type ListNeurons = record {
neuron_ids : vec nat64;
include_empty_neurons_readable_by_caller : opt bool;
include_neurons_readable_by_caller : bool;
};
type ListNeuronsResponse = record {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2340,6 +2340,13 @@ message ListNeurons {
// 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;
}

// A response to a `ListNeurons` request.
Expand Down
8 changes: 8 additions & 0 deletions rs/nns/governance/src/gen/ic_nns_governance.pb.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2766,6 +2766,14 @@ pub struct ListNeurons {
/// 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>,
}
/// A response to a `ListNeurons` request.
///
Expand Down
3 changes: 3 additions & 0 deletions rs/nns/governance/tests/governance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8428,6 +8428,7 @@ fn test_list_neurons() {
let p1_listing = gov.list_neurons_by_principal(
&ListNeurons {
include_neurons_readable_by_caller: true,
include_empty_neurons_readable_by_caller: Some(true),
neuron_ids: vec![],
},
&p1,
Expand All @@ -8449,6 +8450,7 @@ fn test_list_neurons() {
let p4_listing = gov.list_neurons_by_principal(
&ListNeurons {
include_neurons_readable_by_caller: true,
include_empty_neurons_readable_by_caller: Some(true),
neuron_ids: vec![42, 99],
},
&p4,
Expand All @@ -8474,6 +8476,7 @@ fn test_list_neurons() {
let list_neurons_response = gov.list_neurons_by_principal(
&ListNeurons {
include_neurons_readable_by_caller: true,
include_empty_neurons_readable_by_caller: Some(true),
neuron_ids: vec![200],
},
&principal_with_no_neurons,
Expand Down
1 change: 1 addition & 0 deletions rs/nns/test_utils/src/state_test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,7 @@ pub fn list_neurons(state_machine: &StateMachine, sender: PrincipalId) -> ListNe
Encode!(&ListNeurons {
neuron_ids: vec![],
include_neurons_readable_by_caller: true,
include_empty_neurons_readable_by_caller: None,
})
.unwrap(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ fn handle_list_neurons(
let args = ic_nns_governance::pb::v1::ListNeurons {
neuron_ids: vec![],
include_neurons_readable_by_caller: true,
include_empty_neurons_readable_by_caller: None,
};
let update = HttpCanisterUpdate {
canister_id: Blob(ic_nns_constants::GOVERNANCE_CANISTER_ID.get().to_vec()),
Expand Down
1 change: 1 addition & 0 deletions rs/tests/src/canister_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ impl ListNnsNeuronsRequest {
payload: ListNnsNeuronsReq {
neuron_ids,
include_neurons_readable_by_caller,
include_empty_neurons_readable_by_caller: None,
},
}
}
Expand Down

0 comments on commit d7d4555

Please sign in to comment.