diff --git a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs index c75a6354ab3..c5e308a0ca3 100644 --- a/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs +++ b/rs/nervous_system/integration_tests/src/pocket_ic_helpers.rs @@ -1,12 +1,9 @@ use candid::{Decode, Encode, Nat, Principal}; use canister_test::Wasm; -use futures::stream; -use futures::StreamExt; +use futures::{stream, StreamExt}; use ic_base_types::{CanisterId, PrincipalId, SubnetId}; use ic_ledger_core::Tokens; -use ic_nervous_system_agent::pocketic_impl::PocketIcCallError; -use ic_nervous_system_agent::sns::Sns; -use ic_nervous_system_agent::CallCanisters; +use ic_nervous_system_agent::{pocketic_impl::PocketIcCallError, sns::Sns, CallCanisters}; use ic_nervous_system_common::{E8, ONE_DAY_SECONDS}; use ic_nervous_system_common_test_keys::{TEST_NEURON_1_ID, TEST_NEURON_1_OWNER_PRINCIPAL}; use ic_nns_common::pb::v1::{NeuronId, ProposalId}; @@ -60,8 +57,7 @@ use icrc_ledger_types::icrc1::{ account::Account, transfer::{TransferArg, TransferError}, }; -use itertools::EitherOrBoth; -use itertools::Itertools; +use itertools::{EitherOrBoth, Itertools}; use maplit::btreemap; use pocket_ic::{ management_canister::CanisterSettings, nonblocking::PocketIc, ErrorCode, PocketIcBuilder, @@ -69,8 +65,7 @@ use pocket_ic::{ }; use prost::Message; use rust_decimal::prelude::ToPrimitive; -use std::ops::Range; -use std::{collections::BTreeMap, fmt::Write, time::Duration}; +use std::{collections::BTreeMap, fmt::Write, ops::Range, time::Duration}; pub const STARTING_CYCLES_PER_CANISTER: u128 = 2_000_000_000_000_000; @@ -806,6 +801,7 @@ pub mod nns { include_neurons_readable_by_caller: true, include_empty_neurons_readable_by_caller: None, include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None }) .unwrap(), ) @@ -1402,8 +1398,9 @@ pub mod sns { use assert_matches::assert_matches; use ic_crypto_sha2::Sha256; use ic_nervous_system_agent::sns::governance::GovernanceCanister; - use ic_sns_governance::governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS; - use ic_sns_governance::pb::v1::get_neuron_response; + use ic_sns_governance::{ + governance::UPGRADE_STEPS_INTERVAL_REFRESH_BACKOFF_SECONDS, pb::v1::get_neuron_response, + }; use pocket_ic::ErrorCode; pub const EXPECTED_UPGRADE_DURATION_MAX_SECONDS: u64 = 1000; diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index 3ed80ddb56a..68ab4092720 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -2467,11 +2467,9 @@ impl Governance { const MAX_LIST_NEURONS_COUNT: usize = 500; - let mut requested_neuron_ids = requested_neuron_ids.into_iter(); - let mut count = 0; let mut next_start_from_neuron_id = None; // Populate the above two neuron collections. - for neuron_id in &mut requested_neuron_ids { + for (count, neuron_id) in &mut requested_neuron_ids.into_iter().enumerate() { if count >= MAX_LIST_NEURONS_COUNT { next_start_from_neuron_id = Some(neuron_id.id); break; @@ -2503,8 +2501,6 @@ impl Governance { full_neurons.push(proto); } }); - // See if we should exit - count += 1; } // Assemble final result. diff --git a/rs/nns/governance/src/governance/tests/list_neurons.rs b/rs/nns/governance/src/governance/tests/list_neurons.rs index 8bc9c4d102e..503f7e4009a 100644 --- a/rs/nns/governance/src/governance/tests/list_neurons.rs +++ b/rs/nns/governance/src/governance/tests/list_neurons.rs @@ -1,6 +1,6 @@ use crate::{ governance::Governance, - pb::v1::{neuron::DissolveState, ListNeurons, NetworkEconomics, Neuron, VotingPowerEconomics}, + pb::v1::{neuron::DissolveState, ListNeurons, NetworkEconomics, Neuron}, test_utils::{MockEnvironment, StubCMC, StubIcpLedger}, }; use ic_base_types::PrincipalId; @@ -10,15 +10,16 @@ use ic_nns_common::pb::v1::NeuronId; fn test_list_neurons_with_paging() { let user_id = PrincipalId::new_user_test_id(100); - let neurons = (1..1000) + let neurons = (1..1000u64) .map(|id| { let dissolve_state = DissolveState::DissolveDelaySeconds(100); + let account = crate::test_utils::test_subaccount_for_neuron_id(id); ( id, Neuron { id: Some(NeuronId::from_u64(id)), controller: Some(user_id), - account: vec![1; 32], + account, dissolve_state: Some(dissolve_state), // Fill in the rest as needed (stake, maturity, etc.) ..Default::default() diff --git a/rs/nns/governance/src/test_utils.rs b/rs/nns/governance/src/test_utils.rs index 7764f856b3a..8dbc0f33ebf 100644 --- a/rs/nns/governance/src/test_utils.rs +++ b/rs/nns/governance/src/test_utils.rs @@ -314,3 +314,9 @@ impl Environment for MockEnvironment { result } } + +/// Useful for avoiding errors related to index corruption that happens when neurons +/// share subaccounts. +pub fn test_subaccount_for_neuron_id(neuron_id: u64) -> Vec { + [vec![0; 24], neuron_id.to_be_bytes().to_vec()].concat() +} diff --git a/rs/nns/integration_tests/src/governance_neurons.rs b/rs/nns/integration_tests/src/governance_neurons.rs index 6d6fb4cbc7d..dd3f198ea3f 100644 --- a/rs/nns/integration_tests/src/governance_neurons.rs +++ b/rs/nns/integration_tests/src/governance_neurons.rs @@ -551,6 +551,7 @@ fn test_list_neurons() { include_neurons_readable_by_caller: false, include_empty_neurons_readable_by_caller: Some(false), include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None, }, ); assert_eq!(list_neurons_response.neuron_infos.len(), 3); @@ -565,6 +566,7 @@ fn test_list_neurons() { include_neurons_readable_by_caller: true, include_empty_neurons_readable_by_caller: Some(true), include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None, }, ); assert_eq!(list_neurons_response.neuron_infos.len(), 2); @@ -579,6 +581,7 @@ fn test_list_neurons() { include_neurons_readable_by_caller: true, include_empty_neurons_readable_by_caller: Some(false), include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None, }, ); assert_eq!(list_neurons_response.neuron_infos.len(), 1); @@ -594,6 +597,7 @@ fn test_list_neurons() { include_neurons_readable_by_caller: true, include_empty_neurons_readable_by_caller: None, include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None, }, ); assert_eq!(list_neurons_response.neuron_infos.len(), 3); diff --git a/rs/nns/integration_tests/src/neuron_voting.rs b/rs/nns/integration_tests/src/neuron_voting.rs index d5b6fb3f37e..c24fcdf00c9 100644 --- a/rs/nns/integration_tests/src/neuron_voting.rs +++ b/rs/nns/integration_tests/src/neuron_voting.rs @@ -402,6 +402,7 @@ fn test_voting_can_span_multiple_rounds() { include_neurons_readable_by_caller: false, include_empty_neurons_readable_by_caller: None, include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None, }, ); @@ -427,6 +428,7 @@ fn test_voting_can_span_multiple_rounds() { include_neurons_readable_by_caller: false, include_empty_neurons_readable_by_caller: None, include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None, }, ); diff --git a/rs/tests/driver/src/canister_api.rs b/rs/tests/driver/src/canister_api.rs index 1dd732f65ea..862af37a6a4 100644 --- a/rs/tests/driver/src/canister_api.rs +++ b/rs/tests/driver/src/canister_api.rs @@ -738,6 +738,7 @@ impl ListNnsNeuronsRequest { include_neurons_readable_by_caller, include_empty_neurons_readable_by_caller: None, include_public_neurons_in_full_neurons: None, + start_from_neuron_id: None, }, } }