diff --git a/rs/nervous_system/canisters/src/cmc.rs b/rs/nervous_system/canisters/src/cmc.rs index 1f8e2adeeea..10a3614d140 100644 --- a/rs/nervous_system/canisters/src/cmc.rs +++ b/rs/nervous_system/canisters/src/cmc.rs @@ -28,7 +28,7 @@ impl Default for CMCCanister { #[async_trait] impl CMC for CMCCanister { /// Returns the maturity_modulation from the CMC in basis points. - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { let result: Result<(Result,), (i32, String)> = Rt::call_with_cleanup(self.canister_id, "neuron_maturity_modulation", ()).await; match result { diff --git a/rs/nervous_system/common/src/cmc.rs b/rs/nervous_system/common/src/cmc.rs index d0112b888f5..ba71691586d 100644 --- a/rs/nervous_system/common/src/cmc.rs +++ b/rs/nervous_system/common/src/cmc.rs @@ -6,7 +6,7 @@ use mockall::automock; #[async_trait] pub trait CMC: Send + Sync { /// Returns the current neuron maturity modulation. - async fn neuron_maturity_modulation(&mut self) -> Result; + async fn neuron_maturity_modulation(&self) -> Result; } pub struct FakeCmc {} @@ -25,7 +25,7 @@ impl Default for FakeCmc { #[async_trait] impl CMC for FakeCmc { - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { Ok(0) } } diff --git a/rs/nns/governance/benches/scale.rs b/rs/nns/governance/benches/scale.rs index 80948f53ba2..a15470ead05 100644 --- a/rs/nns/governance/benches/scale.rs +++ b/rs/nns/governance/benches/scale.rs @@ -19,6 +19,7 @@ use futures::future::FutureExt; use ic_base_types::{CanisterId, PrincipalId}; use ic_nervous_system_common::{cmc::FakeCmc, ledger::IcpLedger, NervousSystemError}; use ic_nns_common::pb::v1::NeuronId; +use ic_nns_governance::governance::RandomnessGenerator; use ic_nns_governance::{ governance::{Environment, Governance, HeapGrowthPotential, RngError}, pb::v1::{ @@ -28,6 +29,7 @@ use ic_nns_governance::{ }; use icp_ledger::{AccountIdentifier, Subaccount, Tokens}; use std::convert::TryFrom; +use std::sync::Arc; criterion_group! { name = benches; @@ -37,17 +39,15 @@ criterion_group! { criterion_main!(benches); -/// Mock of the required interface of `Governance`. -struct MockEnvironment { - secs: u64, -} +struct FakeRandomness {} -#[async_trait] -impl Environment for MockEnvironment { - fn now(&self) -> u64 { - self.secs +impl FakeRandomness { + fn new() -> Self { + Self {} } +} +impl RandomnessGenerator for FakeRandomness { fn random_u64(&mut self) -> Result { todo!() } @@ -63,6 +63,18 @@ impl Environment for MockEnvironment { fn get_rng_seed(&self) -> Option<[u8; 32]> { todo!() } +} + +/// Mock of the required interface of `Governance`. +struct MockEnvironment { + secs: u64, +} + +#[async_trait] +impl Environment for MockEnvironment { + fn now(&self) -> u64 { + self.secs + } fn execute_nns_function( &self, @@ -145,9 +157,10 @@ fn linear_20k(c: &mut Criterion) { let secs = 1; let mut gov = Governance::new( fixture_for_scale(20_000, true), - Box::new(MockEnvironment { secs }), - Box::new(MockLedger {}), - Box::new(FakeCmc::new()), + Arc::new(MockEnvironment { secs }), + Arc::new(MockLedger {}), + Arc::new(FakeCmc::new()), + Box::new(FakeRandomness::new()), ); c.bench_function("linear 20k", |b| { b.iter(|| make_and_process_proposal(&mut gov)) @@ -158,9 +171,10 @@ fn tree_20k(c: &mut Criterion) { let secs = 1; let mut gov = Governance::new( fixture_for_scale(20_000, false), - Box::new(MockEnvironment { secs }), - Box::new(MockLedger {}), - Box::new(FakeCmc::new()), + Arc::new(MockEnvironment { secs }), + Arc::new(MockLedger {}), + Arc::new(FakeCmc::new()), + Box::new(FakeRandomness::new()), ); c.bench_function("tree 20k", |b| { b.iter(|| make_and_process_proposal(&mut gov)) @@ -171,9 +185,10 @@ fn linear_200k(c: &mut Criterion) { let secs = 1; let mut gov = Governance::new( fixture_for_scale(200_000, true), - Box::new(MockEnvironment { secs }), - Box::new(MockLedger {}), - Box::new(FakeCmc::new()), + Arc::new(MockEnvironment { secs }), + Arc::new(MockLedger {}), + Arc::new(FakeCmc::new()), + Box::new(FakeRandomness::new()), ); c.bench_function("linear 200k", |b| { b.iter(|| make_and_process_proposal(&mut gov)) @@ -184,9 +199,10 @@ fn tree_200k(c: &mut Criterion) { let secs = 1; let mut gov = Governance::new( fixture_for_scale(200_000, false), - Box::new(MockEnvironment { secs }), - Box::new(MockLedger {}), - Box::new(FakeCmc::new()), + Arc::new(MockEnvironment { secs }), + Arc::new(MockLedger {}), + Arc::new(FakeCmc::new()), + Box::new(FakeRandomness::new()), ); c.bench_function("tree 200k", |b| { b.iter(|| make_and_process_proposal(&mut gov)) diff --git a/rs/nns/governance/canister/canister.rs b/rs/nns/governance/canister/canister.rs index e5e40727d5d..e3cf60624d9 100644 --- a/rs/nns/governance/canister/canister.rs +++ b/rs/nns/governance/canister/canister.rs @@ -55,11 +55,12 @@ use ic_nns_governance_api::test_api::TimeWarp; use prost::Message; use rand::{RngCore, SeedableRng}; use rand_chacha::ChaCha20Rng; +use std::sync::Arc; use std::{boxed::Box, ops::Bound, time::Duration}; #[cfg(not(feature = "tla"))] use ic_nervous_system_canisters::ledger::IcpLedgerCanister; -use ic_nns_governance::canister_state::with_governance; +use ic_nns_governance::canister_state::{with_governance, CanisterRandomnessGenerator}; #[cfg(feature = "tla")] mod tla_ledger; @@ -230,9 +231,10 @@ fn canister_init_(init_payload: ApiGovernanceProto) { let governance_proto = InternalGovernanceProto::from(init_payload); set_governance(Governance::new( governance_proto, - Box::new(CanisterEnv::new()), - Box::new(IcpLedgerCanister::::new(LEDGER_CANISTER_ID)), - Box::new(CMCCanister::::new()), + Arc::new(CanisterEnv::new()), + Arc::new(IcpLedgerCanister::::new(LEDGER_CANISTER_ID)), + Arc::new(CMCCanister::::new()), + Box::new(CanisterRandomnessGenerator::new()), )); } @@ -274,9 +276,10 @@ fn canister_post_upgrade() { schedule_timers(); set_governance(Governance::new_restored( restored_state, - Box::new(CanisterEnv::new()), - Box::new(IcpLedgerCanister::::new(LEDGER_CANISTER_ID)), - Box::new(CMCCanister::::new()), + Arc::new(CanisterEnv::new()), + Arc::new(IcpLedgerCanister::::new(LEDGER_CANISTER_ID)), + Arc::new(CMCCanister::::new()), + Box::new(CanisterRandomnessGenerator::new()), )); validate_stable_storage(); diff --git a/rs/nns/governance/src/canister_state.rs b/rs/nns/governance/src/canister_state.rs index 83dc4c879f2..2bf61a63850 100644 --- a/rs/nns/governance/src/canister_state.rs +++ b/rs/nns/governance/src/canister_state.rs @@ -1,5 +1,7 @@ use crate::decoder_config; -use crate::governance::{Environment, Governance, HeapGrowthPotential, RngError}; +use crate::governance::{ + Environment, Governance, HeapGrowthPotential, RandomnessGenerator, RngError, +}; use async_trait::async_trait; use candid::{Decode, Encode}; use ic_base_types::CanisterId; @@ -21,12 +23,16 @@ use rand::{RngCore, SeedableRng}; use rand_chacha::ChaCha20Rng; use std::cell::RefCell; use std::str::FromStr; +use std::sync::Arc; +#[cfg(any(test, feature = "test"))] +use std::sync::RwLock; thread_local! { pub(crate) static GOVERNANCE: RefCell = RefCell::new(Governance::new_uninitialized( - Box::new(CanisterEnv::new()), - Box::new(IcpLedgerCanister::::new(LEDGER_CANISTER_ID)), - Box::new(CMCCanister::::new()), + Arc::new(CanisterEnv::new()), + Arc::new(IcpLedgerCanister::::new(LEDGER_CANISTER_ID)), + Arc::new(CMCCanister::::new()), + Box::new(CanisterRandomnessGenerator::new()), )); } /* @@ -106,29 +112,31 @@ pub fn set_governance(gov: Governance) { #[derive(Default)] pub struct CanisterEnv { - rng: Option, - time_warp: crate::governance::TimeWarp, + #[cfg(any(test, feature = "test"))] + time_warp: RwLock, } impl CanisterEnv { pub fn new() -> Self { CanisterEnv { - rng: None, - time_warp: crate::governance::TimeWarp { delta_s: 0 }, + #[cfg(any(test, feature = "test"))] + time_warp: RwLock::new(crate::governance::TimeWarp { delta_s: 0 }), } } } -#[async_trait] -impl Environment for CanisterEnv { - fn now(&self) -> u64 { - self.time_warp.apply(now_seconds()) - } +#[derive(Default)] +pub struct CanisterRandomnessGenerator { + rng: Option, +} - fn set_time_warp(&mut self, new_time_warp: crate::governance::TimeWarp) { - self.time_warp = new_time_warp; +impl CanisterRandomnessGenerator { + pub fn new() -> Self { + CanisterRandomnessGenerator { rng: None } } +} +impl RandomnessGenerator for CanisterRandomnessGenerator { fn random_u64(&mut self) -> Result { match self.rng.as_mut() { Some(rand) => Ok(rand.next_u64()), @@ -154,6 +162,24 @@ impl Environment for CanisterEnv { fn get_rng_seed(&self) -> Option<[u8; 32]> { self.rng.as_ref().map(|rng| rng.get_seed()) } +} + +#[async_trait] +impl Environment for CanisterEnv { + #[cfg(any(test, feature = "test"))] + fn now(&self) -> u64 { + self.time_warp.read().unwrap().apply(now_seconds()) + } + + #[cfg(not(any(test, feature = "test")))] + fn now(&self) -> u64 { + now_seconds() + } + + #[cfg(any(test, feature = "test"))] + fn set_time_warp(&self, new_time_warp: crate::governance::TimeWarp) { + *self.time_warp.write().unwrap() = new_time_warp; + } fn execute_nns_function( &self, @@ -420,7 +446,7 @@ mod tests { use candid::{Decode, Encode}; #[test] fn test_set_time_warp() { - let mut environment = CanisterEnv::new(); + let environment = CanisterEnv::new(); let start = environment.now(); environment.set_time_warp(crate::governance::TimeWarp { delta_s: 1_000 }); diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index 322dbf51ada..b4c393f175f 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -117,6 +117,7 @@ use registry_canister::{ }; use rust_decimal::Decimal; use rust_decimal_macros::dec; +use std::sync::Arc; use std::{ borrow::Cow, cmp::{max, Ordering}, @@ -1291,16 +1292,7 @@ impl From for GovernanceError { } } -/// A general trait for the environment in which governance is running. -#[async_trait] -pub trait Environment: Send + Sync { - /// Returns the current time, in seconds since the epoch. - fn now(&self) -> u64; - - fn set_time_warp(&mut self, _new_time_warp: TimeWarp) { - panic!("Not implemented."); - } - +pub trait RandomnessGenerator: Send + Sync { /// Returns a random number. /// /// This number is the same in all replicas. @@ -1316,6 +1308,18 @@ pub trait Environment: Send + Sync { // Get the current RNG seed (used in pre-upgrade) fn get_rng_seed(&self) -> Option<[u8; 32]>; +} + +/// A general trait for the environment in which governance is running. +#[async_trait] +pub trait Environment: Send + Sync { + /// Returns the current time, in seconds since the epoch. + fn now(&self) -> u64; + + #[cfg(any(test, feature = "test"))] + fn set_time_warp(&self, _new_time_warp: TimeWarp) { + panic!("Not implemented."); + } /// Executes a `ExecuteNnsFunction`. The standard implementation is /// expected to call out to another canister and eventually report the @@ -1420,13 +1424,16 @@ pub struct Governance { pub neuron_store: NeuronStore, /// Implementation of Environment to make unit testing easier. - pub env: Box, + pub env: Arc, /// Implementation of the interface with the Ledger canister. - ledger: Box, + ledger: Arc, /// Implementation of the interface with the CMC canister. - cmc: Box, + cmc: Arc, + + /// Implementation of a randomness generator + randomness: Box, /// Timestamp, in seconds since the unix epoch, until which no proposal /// needs to be processed. @@ -1599,9 +1606,10 @@ impl Governance { /// initialized in init. In any other case, the `Governance` object should be initialized with /// either `new` or `new_restored`. pub fn new_uninitialized( - env: Box, - ledger: Box, - cmc: Box, + env: Arc, + ledger: Arc, + cmc: Arc, + randomness: Box, ) -> Self { Self { heap_data: HeapGovernanceData::default(), @@ -1609,6 +1617,7 @@ impl Governance { env, ledger, cmc, + randomness, closest_proposal_deadline_timestamp_seconds: 0, latest_gc_timestamp_seconds: 0, latest_gc_num_proposals: 0, @@ -1622,9 +1631,10 @@ impl Governance { /// with its persisted state, `Governance::new_restored` should be called instead. pub fn new( mut governance_proto: GovernanceProto, - env: Box, - ledger: Box, - cmc: Box, + env: Arc, + ledger: Arc, + cmc: Arc, + randomness: Box, ) -> Self { // Step 1: Populate some fields governance_proto if they are blank. @@ -1681,6 +1691,7 @@ impl Governance { env, ledger, cmc, + randomness, closest_proposal_deadline_timestamp_seconds: 0, latest_gc_timestamp_seconds: 0, latest_gc_num_proposals: 0, @@ -1693,9 +1704,10 @@ impl Governance { /// Restores Governance after an upgrade from its persisted state. pub fn new_restored( governance_proto: GovernanceProto, - mut env: Box, - ledger: Box, - cmc: Box, + env: Arc, + ledger: Arc, + cmc: Arc, + mut randomness: Box, ) -> Self { let (heap_neurons, topic_followee_map, heap_governance_proto, maybe_rng_seed) = split_governance_proto(governance_proto); @@ -1703,7 +1715,7 @@ impl Governance { // Carry over the previous rng seed to avoid race conditions in handling queued ingress // messages that may require a functioning RNG. if let Some(rng_seed) = maybe_rng_seed { - env.seed_rng(rng_seed); + randomness.seed_rng(rng_seed); } Self { @@ -1712,6 +1724,7 @@ impl Governance { env, ledger, cmc, + randomness, closest_proposal_deadline_timestamp_seconds: 0, latest_gc_timestamp_seconds: 0, latest_gc_num_proposals: 0, @@ -1727,7 +1740,7 @@ impl Governance { let neuron_store = std::mem::take(&mut self.neuron_store); let (neurons, heap_topic_followee_index) = neuron_store.take(); let heap_governance_proto = std::mem::take(&mut self.heap_data); - let rng_seed = self.env.get_rng_seed(); + let rng_seed = self.randomness.get_rng_seed(); reassemble_governance_proto( neurons, heap_topic_followee_index, @@ -1740,7 +1753,7 @@ impl Governance { let neurons = self.neuron_store.__get_neurons_for_tests(); let heap_topic_followee_index = self.neuron_store.clone_topic_followee_index(); let heap_governance_proto = self.heap_data.clone(); - let rng_seed = self.env.get_rng_seed(); + let rng_seed = self.randomness.get_rng_seed(); reassemble_governance_proto( neurons, heap_topic_followee_index, @@ -1749,6 +1762,10 @@ impl Governance { ) } + pub fn seed_rng(&mut self, seed: [u8; 32]) { + self.randomness.seed_rng(seed); + } + /// Validates that the underlying protobuf is well formed. pub fn validate(&self) -> Result<(), GovernanceError> { if self.heap_data.economics.is_none() { @@ -1783,6 +1800,7 @@ impl Governance { Ok(()) } + #[cfg(any(test, feature = "test"))] pub fn set_time_warp(&mut self, new_time_warp: TimeWarp) { // This is not very DRY, because we have to keep a couple copies of TimeWarp in sync. // However, trying to share one copy of TimeWarp seems difficult. Seems like you would have @@ -2598,11 +2616,11 @@ impl Governance { } let created_timestamp_seconds = self.env.now(); - let child_nid = self.neuron_store.new_neuron_id(&mut *self.env)?; + let child_nid = self.neuron_store.new_neuron_id(&mut *self.randomness)?; let from_subaccount = parent_neuron.subaccount(); - let to_subaccount = Subaccount(self.env.random_byte_array()?); + let to_subaccount = Subaccount(self.randomness.random_byte_array()?); // Make sure there isn't already a neuron with the same sub-account. if self.neuron_store.has_neuron_with_subaccount(to_subaccount) { @@ -3036,11 +3054,11 @@ impl Governance { )); } - let child_nid = self.neuron_store.new_neuron_id(&mut *self.env)?; + let child_nid = self.neuron_store.new_neuron_id(&mut *self.randomness)?; // use provided sub-account if any, otherwise generate a random one. let to_subaccount = match spawn.nonce { - None => Subaccount(self.env.random_byte_array()?), + None => Subaccount(self.randomness.random_byte_array()?), Some(nonce_val) => { ledger::compute_neuron_staking_subaccount(child_controller, nonce_val) } @@ -3318,7 +3336,7 @@ impl Governance { ) })?; - let child_nid = self.neuron_store.new_neuron_id(&mut *self.env)?; + let child_nid = self.neuron_store.new_neuron_id(&mut *self.randomness)?; let from_subaccount = parent_neuron.subaccount(); // The account is derived from the new owner's principal so it can be found by @@ -4050,7 +4068,7 @@ impl Governance { "Reward node provider proposal must have a reward mode.", )), Some(RewardMode::RewardToNeuron(reward_to_neuron)) => { - let to_subaccount = Subaccount(self.env.random_byte_array()?); + let to_subaccount = Subaccount(self.randomness.random_byte_array()?); let _block_height = self .ledger .transfer_funds( @@ -4061,7 +4079,7 @@ impl Governance { now, ) .await?; - let nid = self.neuron_store.new_neuron_id(&mut *self.env)?; + let nid = self.neuron_store.new_neuron_id(&mut *self.randomness)?; let dissolve_delay_seconds = std::cmp::min( reward_to_neuron.dissolve_delay_seconds, MAX_DISSOLVE_DELAY_SECONDS, @@ -4798,7 +4816,7 @@ impl Governance { // Do NOT return Err right away using the ? operator, because we must refund maturity to the // Neurons' Fund before returning. let mut deploy_new_sns_response: Result = - call_deploy_new_sns(&mut self.env, sns_init_payload).await; + call_deploy_new_sns(&self.env, sns_init_payload).await; // Step 3: React to response from deploy_new_sns (Ok or Err). @@ -6060,7 +6078,7 @@ impl Governance { controller: PrincipalId, claim_or_refresh: &ClaimOrRefresh, ) -> Result { - let nid = self.neuron_store.new_neuron_id(&mut *self.env)?; + let nid = self.neuron_store.new_neuron_id(&mut *self.randomness)?; let now = self.env.now(); let neuron = NeuronBuilder::new( nid, @@ -7154,7 +7172,7 @@ impl Governance { ) })?; if let Err(err_msg) = - is_canister_id_valid_swap_canister_id(target_canister_id, &mut *self.env).await + is_canister_id_valid_swap_canister_id(target_canister_id, &*self.env).await { return Err(GovernanceError::new_with_message( ErrorType::NotAuthorized, @@ -7818,7 +7836,7 @@ impl Governance { pub fn randomly_pick_swap_start(&mut self) -> GlobalTimeOfDay { // It's not critical that we have perfect randomness here, so we can default to a fixed value // for the edge case where the RNG is not initialized (which should never happen in practice). - let time_of_day_seconds = self.env.random_u64().unwrap_or(10_000) % ONE_DAY_SECONDS; + let time_of_day_seconds = self.randomness.random_u64().unwrap_or(10_000) % ONE_DAY_SECONDS; // Round down to nearest multiple of 15 min. let remainder_seconds = time_of_day_seconds % (15 * 60); @@ -8027,7 +8045,7 @@ impl From for NeuronSubsetMetricsPb { /// /// If Ok is returned, it can be assumed that the error field is not populated. async fn call_deploy_new_sns( - env: &mut Box, + env: &Arc, sns_init_payload: SnsInitPayload, ) -> Result { // Step 2.1: Construct request @@ -8097,7 +8115,7 @@ fn validate_motion(motion: &Motion) -> Result<(), GovernanceError> { /// the SNS-W canister. async fn is_canister_id_valid_swap_canister_id( target_canister_id: CanisterId, - env: &mut dyn Environment, + env: &dyn Environment, ) -> Result<(), String> { let list_deployed_snses_response = env .call_canister_method( diff --git a/rs/nns/governance/src/governance/benches.rs b/rs/nns/governance/src/governance/benches.rs index cb460366a27..341a5eb80a8 100644 --- a/rs/nns/governance/src/governance/benches.rs +++ b/rs/nns/governance/src/governance/benches.rs @@ -1,3 +1,4 @@ +use crate::test_utils::MockRandomness; use crate::{ governance::{ test_data::CREATE_SERVICE_NERVOUS_SYSTEM_WITH_MATCHED_FUNDING, Governance, @@ -36,6 +37,7 @@ use maplit::hashmap; use rand::{Rng, SeedableRng}; use rand_chacha::ChaCha20Rng; use std::collections::{BTreeMap, HashMap}; +use std::sync::Arc; enum SetUpStrategy { // Every neuron follows a single neuron. @@ -329,9 +331,10 @@ fn cast_vote_cascade_helper(strategy: SetUpStrategy, topic: Topic) -> BenchResul }; let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let neuron_id = set_up(strategy, &mut rng, &mut governance, topic); @@ -512,9 +515,10 @@ fn compute_ballots_for_new_proposal_with_stable_neurons() -> BenchResult { let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(vec![], now_seconds)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(vec![], now_seconds)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); bench_fn(|| { @@ -562,9 +566,10 @@ fn list_neurons_by_subaccount_benchmark() -> BenchResult { let governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let request = ListNeurons { @@ -605,9 +610,10 @@ fn list_neurons_benchmark() -> BenchResult { let governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let request = ListNeurons { @@ -693,9 +699,10 @@ fn list_proposals_benchmark() -> BenchResult { let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let request = ListProposalInfo { diff --git a/rs/nns/governance/src/governance/tests/list_neurons.rs b/rs/nns/governance/src/governance/tests/list_neurons.rs index 7ccb825dce1..d3216b2cc8a 100644 --- a/rs/nns/governance/src/governance/tests/list_neurons.rs +++ b/rs/nns/governance/src/governance/tests/list_neurons.rs @@ -1,3 +1,4 @@ +use crate::test_utils::MockRandomness; use crate::{ governance::Governance, pb::v1::{neuron::DissolveState, NetworkEconomics, Neuron}, @@ -7,6 +8,7 @@ use ic_base_types::PrincipalId; use ic_nns_common::pb::v1::NeuronId; use ic_nns_governance_api::pb::v1::list_neurons::NeuronSubaccount; use ic_nns_governance_api::pb::v1::ListNeurons; +use std::sync::Arc; #[test] fn test_list_neurons_with_paging() { @@ -39,9 +41,10 @@ fn test_list_neurons_with_paging() { }), ..crate::pb::v1::Governance::default() }, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let mut request = ListNeurons { @@ -127,9 +130,10 @@ fn test_list_neurons_by_subaccounts_and_ids() { }), ..crate::pb::v1::Governance::default() }, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let request = ListNeurons { diff --git a/rs/nns/governance/src/governance/tests/mod.rs b/rs/nns/governance/src/governance/tests/mod.rs index f90393a853a..a6dadd9b37b 100644 --- a/rs/nns/governance/src/governance/tests/mod.rs +++ b/rs/nns/governance/src/governance/tests/mod.rs @@ -1,4 +1,5 @@ use super::*; +use crate::test_utils::MockRandomness; use crate::{ neuron::{DissolveStateAndAge, NeuronBuilder}, pb::v1::{ @@ -844,10 +845,11 @@ mod convert_create_service_nervous_system_proposal_to_sns_init_payload_tests_wit } mod metrics_tests { - use ic_nns_common::pb::v1::ProposalId; use maplit::btreemap; + use std::sync::Arc; + use crate::test_utils::MockRandomness; use crate::{ encode_metrics, governance::Governance, @@ -893,7 +895,6 @@ mod metrics_tests { }), ..ProposalData::default() }; - let governance = Governance::new( GovernanceProto { proposals: btreemap! { @@ -902,9 +903,10 @@ mod metrics_tests { }, ..GovernanceProto::default() }, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let mut writer = ic_metrics_encoder::MetricsEncoder::new(vec![], 1000); @@ -966,9 +968,10 @@ mod metrics_tests { }, ..GovernanceProto::default() }, - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let mut writer = ic_metrics_encoder::MetricsEncoder::new(vec![], 10); @@ -1171,11 +1174,15 @@ fn test_pre_and_post_upgrade_first_time() { // Then Governance is instantiated during upgrade with proto let mut governance = Governance::new( governance_proto, - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); + // Simulate seeding the randomness in a running governance canister. + governance.randomness.seed_rng([12; 32]); + assert_eq!(governance.neuron_store.len(), 1); // On next pre-upgrade, we get the heap proto and store it in stable memory let mut extracted_proto = governance.take_heap_proto(); @@ -1196,15 +1203,17 @@ fn test_pre_and_post_upgrade_first_time() { // We now simulate the post_upgrade let mut governance = Governance::new_restored( extracted_proto, - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); assert_eq!(governance.neuron_store.len(), 1); // It should not rebuild during post_upgrade so it should still be mis-matched with neurons. let extracted_proto = governance.take_heap_proto(); assert_eq!(extracted_proto.topic_followee_index.len(), 2); + assert_eq!(extracted_proto.rng_seed, Some(vec![12; 32])); } #[test] @@ -1217,9 +1226,10 @@ fn can_spawn_neurons_only_true_when_not_spawning_and_neurons_ready_to_spawn() { let mut governance = Governance::new( proto, - Box::new(mock_env), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(mock_env), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); // No neurons to spawn... assert!(!governance.can_spawn_neurons()); @@ -1264,9 +1274,10 @@ fn test_validate_execute_nns_function() { }], ..Default::default() }, - Box::new(MockEnvironment::new(vec![], 100)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(vec![], 100)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let test_execute_nns_function_error = @@ -1584,9 +1595,10 @@ fn test_update_neuron_errors_out_expectedly() { }; let mut governance = Governance::new( governance_proto, - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); assert_eq!( @@ -1648,9 +1660,10 @@ fn test_compute_ballots_for_new_proposal() { let mut governance = Governance::new( governance_proto, - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let manage_neuron_action = Action::ManageNeuron(Box::new(ManageNeuron { id: Some(NeuronId { id: 10 }), diff --git a/rs/nns/governance/src/governance/tests/neurons_fund.rs b/rs/nns/governance/src/governance/tests/neurons_fund.rs index 755ec9ad658..8a1d1d446d9 100644 --- a/rs/nns/governance/src/governance/tests/neurons_fund.rs +++ b/rs/nns/governance/src/governance/tests/neurons_fund.rs @@ -26,9 +26,10 @@ fn proposal_passes_if_not_too_many_nf_neurons_can_occur() { }; let mut governance = Governance::new( governance_proto.into(), - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); // Run code under test governance @@ -92,9 +93,10 @@ fn proposal_fails_if_too_many_nf_neurons_can_occur() { }; let mut governance = Governance::new( governance_proto.into(), - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); // Run code under test let err = governance @@ -133,9 +135,10 @@ fn proposal_fails_if_no_nf_neurons_exist() { }; let mut governance = Governance::new( governance_proto.into(), - Box::::default(), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::::default(), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); // Run code under test let err = governance diff --git a/rs/nns/governance/src/governance/tests/node_provider_rewards.rs b/rs/nns/governance/src/governance/tests/node_provider_rewards.rs index c569296a334..76856fac5c5 100644 --- a/rs/nns/governance/src/governance/tests/node_provider_rewards.rs +++ b/rs/nns/governance/src/governance/tests/node_provider_rewards.rs @@ -1,9 +1,11 @@ +use crate::test_utils::MockRandomness; use crate::{ governance::Governance, node_provider_rewards::DateRangeFilter, pb::v1::{Governance as GovernanceProto, MonthlyNodeProviderRewards}, test_utils::{MockEnvironment, StubCMC, StubIcpLedger}, }; +use std::sync::Arc; #[test] fn test_node_provider_rewards_read_from_correct_sources() { @@ -26,15 +28,15 @@ fn test_node_provider_rewards_read_from_correct_sources() { registry_version: None, node_providers: vec![], }; - let mut governance = Governance::new( GovernanceProto { most_recent_monthly_node_provider_rewards: Some(rewards_1.clone()), ..Default::default() }, - Box::new(MockEnvironment::new(vec![], 100)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(vec![], 100)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let result_1 = governance.get_most_recent_monthly_node_provider_rewards(); @@ -80,9 +82,10 @@ fn test_list_node_provider_rewards_api() { GovernanceProto { ..Default::default() }, - Box::new(MockEnvironment::new(vec![], 100)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(vec![], 100)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); governance.update_most_recent_monthly_node_provider_rewards(rewards_1.clone()); @@ -100,9 +103,10 @@ fn test_list_node_provider_rewards_api_with_paging_and_filters() { GovernanceProto { ..Default::default() }, - Box::new(MockEnvironment::new(vec![], 100)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(vec![], 100)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let mut rewards_minted = vec![]; diff --git a/rs/nns/governance/src/governance/tests/stake_maturity.rs b/rs/nns/governance/src/governance/tests/stake_maturity.rs index b990d67ed0f..b14a128f653 100644 --- a/rs/nns/governance/src/governance/tests/stake_maturity.rs +++ b/rs/nns/governance/src/governance/tests/stake_maturity.rs @@ -1,3 +1,4 @@ +use crate::test_utils::MockRandomness; use crate::{ governance::{ tests::{MockEnvironment, StubCMC, StubIcpLedger}, @@ -11,6 +12,7 @@ use ic_nns_governance_api::pb::v1::manage_neuron_response::{ MergeMaturityResponse, StakeMaturityResponse, }; use maplit::btreemap; +use std::sync::Arc; #[test] fn test_stake_maturity() { @@ -26,7 +28,6 @@ fn test_stake_maturity() { staked_maturity_e8s_equivalent: Some(100), ..Default::default() }; - let mut governance = Governance::new( GovernanceProto { neurons: btreemap! { @@ -34,9 +35,10 @@ fn test_stake_maturity() { }, ..GovernanceProto::default() }, - Box::new(MockEnvironment::new(vec![], 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(vec![], 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let request = StakeMaturity { diff --git a/rs/nns/governance/src/neuron_store.rs b/rs/nns/governance/src/neuron_store.rs index d5ae19dfd3e..6d9907277e6 100644 --- a/rs/nns/governance/src/neuron_store.rs +++ b/rs/nns/governance/src/neuron_store.rs @@ -1,8 +1,6 @@ use crate::{ allow_active_neurons_in_stable_memory, - governance::{ - Environment, TimeWarp, LOG_PREFIX, MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS, - }, + governance::{TimeWarp, LOG_PREFIX, MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS}, migrate_active_neurons_to_stable_memory, neuron::types::Neuron, neurons_fund::neurons_fund_neuron::pick_most_important_hotkeys, @@ -37,6 +35,7 @@ use std::{ }; pub mod metrics; +use crate::governance::RandomnessGenerator; use crate::pb::v1::{Ballot, Vote}; pub(crate) use metrics::NeuronMetrics; @@ -434,9 +433,12 @@ impl NeuronStore { self.clock.set_time_warp(new_time_warp); } - pub fn new_neuron_id(&self, env: &mut dyn Environment) -> Result { + pub fn new_neuron_id( + &self, + random: &mut dyn RandomnessGenerator, + ) -> Result { loop { - let id = env + let id = random .random_u64() .map_err(|_| NeuronStoreError::NeuronIdGenerationUnavailable)? // Let there be no question that id was chosen diff --git a/rs/nns/governance/src/seed_accounts/seed_accounts_tests.rs b/rs/nns/governance/src/seed_accounts/seed_accounts_tests.rs index 8e6dc9239dc..64d07124bab 100644 --- a/rs/nns/governance/src/seed_accounts/seed_accounts_tests.rs +++ b/rs/nns/governance/src/seed_accounts/seed_accounts_tests.rs @@ -45,9 +45,10 @@ fn test_can_tag_seed_neurons_handles_corrupted_state() { // Setup let mut governance = Governance::new( GovernanceProto::default(), - Box::new(create_mock_environment(None)), - Box::::default(), - Box::::default(), + Arc::new(create_mock_environment(None)), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); // Corrupt the state @@ -107,9 +108,10 @@ fn test_can_tag_seed_neurons_transitions() { // Setup let mut governance = Governance::new( GovernanceProto::default(), - Box::new(create_mock_environment(None)), - Box::::default(), - Box::::default(), + Arc::new(create_mock_environment(None)), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); // Execute / verify @@ -137,9 +139,10 @@ fn test_can_tag_seed_neurons_exits_early_if_data_is_processed() { // Setup let mut governance = Governance::new( GovernanceProto::default(), - Box::new(create_mock_environment(None)), - Box::::default(), - Box::::default(), + Arc::new(create_mock_environment(None)), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); // Execute / Verify @@ -195,9 +198,10 @@ async fn test_tag_seed_neurons_happy() { genesis_timestamp_seconds: now_timestamp_seconds - ONE_MONTH_SECONDS, ..Default::default() }, - Box::new(environment), - Box::::default(), - Box::::default(), + Arc::new(environment), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); let expected_seed_account = SeedAccount { @@ -281,9 +285,10 @@ async fn test_tag_neuron_sad() { genesis_timestamp_seconds: now_timestamp_seconds - ONE_MONTH_SECONDS, ..Default::default() }, - Box::new(environment), - Box::::default(), - Box::::default(), + Arc::new(environment), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); let expected_seed_account = SeedAccount { @@ -374,9 +379,10 @@ async fn test_tag_seed_neurons_handles_neuron_splits() { genesis_timestamp_seconds: now_timestamp_seconds, ..Default::default() }, - Box::new(environment), - Box::::default(), - Box::::default(), + Arc::new(environment), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); // Execute @@ -439,9 +445,10 @@ async fn test_tag_seed_neurons_doesnt_over_tag_seed_neurons() { genesis_timestamp_seconds, ..Default::default() }, - Box::new(environment), - Box::::default(), - Box::::default(), + Arc::new(environment), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); // Execute @@ -484,9 +491,10 @@ fn test_calculate_genesis_account_expected_stake_e8s() { genesis_timestamp_seconds, ..Default::default() }, - Box::new(create_mock_environment(None)), - Box::::default(), - Box::::default(), + Arc::new(create_mock_environment(None)), + Arc::::default(), + Arc::::default(), + Box::new(MockRandomness::new()), ); // At genesis_timestamp_seconds, expected stake e8s should diff --git a/rs/nns/governance/src/test_utils.rs b/rs/nns/governance/src/test_utils.rs index 8dbc0f33ebf..a1537efcb19 100644 --- a/rs/nns/governance/src/test_utils.rs +++ b/rs/nns/governance/src/test_utils.rs @@ -1,6 +1,7 @@ // This allow(dead_code) is necessary because some parts of this file // are not used in canbench-rs, but are used elsewhere. Otherwise we get annoying clippy warnings. #![allow(dead_code)] +use crate::governance::RandomnessGenerator; use crate::{ governance::{Environment, HeapGrowthPotential, RngError}, pb::v1::{ExecuteNnsFunction, GovernanceError, OpenSnsTokenSwap}, @@ -108,7 +109,7 @@ impl IcpLedger for StubIcpLedger { pub(crate) struct StubCMC {} #[async_trait] impl CMC for StubCMC { - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { unimplemented!() } } @@ -134,6 +135,33 @@ impl ExpectedCallCanisterMethodCallArguments { } } +pub(crate) struct MockRandomness { + seed: Option<[u8; 32]>, +} +impl MockRandomness { + pub fn new() -> MockRandomness { + MockRandomness { seed: None } + } +} + +impl RandomnessGenerator for MockRandomness { + fn random_u64(&mut self) -> Result { + todo!() + } + + fn random_byte_array(&mut self) -> Result<[u8; 32], RngError> { + todo!() + } + + fn seed_rng(&mut self, seed: [u8; 32]) { + self.seed = Some(seed); + } + + fn get_rng_seed(&self) -> Option<[u8; 32]> { + self.seed + } +} + #[allow(clippy::type_complexity)] pub(crate) struct MockEnvironment { expected_call_canister_method_calls: Arc< @@ -204,20 +232,6 @@ impl Environment for MockEnvironment { *self.now.lock().unwrap() } - fn random_u64(&mut self) -> Result { - unimplemented!(); - } - - fn random_byte_array(&mut self) -> Result<[u8; 32], RngError> { - unimplemented!(); - } - - fn seed_rng(&mut self, _seed: [u8; 32]) {} - - fn get_rng_seed(&self) -> Option<[u8; 32]> { - Some([0; 32]) - } - fn execute_nns_function( &self, _proposal_id: u64, diff --git a/rs/nns/governance/src/timer_tasks/seeding.rs b/rs/nns/governance/src/timer_tasks/seeding.rs index c7f68abae97..4b3f8f44418 100644 --- a/rs/nns/governance/src/timer_tasks/seeding.rs +++ b/rs/nns/governance/src/timer_tasks/seeding.rs @@ -1,11 +1,10 @@ +use crate::governance::{Governance, LOG_PREFIX}; use async_trait::async_trait; +use candid::{Decode, Encode}; use ic_management_canister_types_private::IC_00; -use ic_nervous_system_runtime::{CdkRuntime, Runtime}; use ic_nervous_system_timer_task::RecurringAsyncTask; use std::{cell::RefCell, thread::LocalKey, time::Duration}; -use crate::governance::{Governance, LOG_PREFIX}; - pub(super) struct SeedingTask { governance: &'static LocalKey>, } @@ -24,19 +23,25 @@ const RETRY_SEEDING_INTERVAL: Duration = Duration::from_secs(30); #[async_trait] impl RecurringAsyncTask for SeedingTask { async fn execute(self) -> (Duration, Self) { - let result: Result<([u8; 32],), (i32, String)> = - CdkRuntime::call_with_cleanup(IC_00, "raw_rand", ()).await; + let env = self + .governance + .with_borrow(|governance| governance.env.clone()); + + let result: Result, (Option, String)> = env + .call_canister_method(IC_00, "raw_rand", Encode!().unwrap()) + .await; let next_delay = match result { - Ok((seed,)) => { + Ok(bytes) => { + let seed = Decode!(&bytes, [u8; 32]).unwrap(); self.governance.with_borrow_mut(|governance| { - governance.env.seed_rng(seed); + governance.seed_rng(seed); }); SEEDING_INTERVAL } Err((code, msg)) => { println!( - "{}Error seeding RNG. Error Code: {}. Error Message: {}", + "{}Error seeding RNG. Error Code: {:?}. Error Message: {}", LOG_PREFIX, code, msg ); RETRY_SEEDING_INTERVAL diff --git a/rs/nns/governance/src/voting.rs b/rs/nns/governance/src/voting.rs index b98c96a886e..c1704ffdcbc 100644 --- a/rs/nns/governance/src/voting.rs +++ b/rs/nns/governance/src/voting.rs @@ -586,6 +586,7 @@ fn retain_neurons_with_castable_ballots( #[cfg(test)] mod test { + use crate::test_utils::MockRandomness; use crate::{ governance::{ Governance, MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS, @@ -618,6 +619,7 @@ mod test { use icp_ledger::Subaccount; use maplit::{btreemap, hashmap}; use std::collections::{BTreeMap, BTreeSet, HashMap}; + use std::sync::Arc; fn make_ballot(voting_power: u64, vote: Vote) -> Ballot { Ballot { @@ -730,9 +732,10 @@ mod test { }; let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); governance @@ -829,9 +832,10 @@ mod test { }; let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 234)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 234)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); governance @@ -1120,9 +1124,10 @@ mod test { }; let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); // In our test configuration, we always return "true" for is_over_instructions_limit() @@ -1195,9 +1200,10 @@ mod test { }; let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); let _e = temporarily_set_over_soft_message_limit(true); @@ -1255,9 +1261,10 @@ mod test { }; let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 0)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 0)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); // In test mode, we are always saying we're over the soft-message limit, so we know that @@ -1363,9 +1370,10 @@ mod test { }; let mut governance = Governance::new( governance_proto, - Box::new(MockEnvironment::new(Default::default(), 1234)), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(MockEnvironment::new(Default::default(), 1234)), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); governance.record_neuron_vote(ProposalId { id: 1 }, NeuronId { id: 1 }, Vote::Yes, topic); @@ -1494,9 +1502,10 @@ mod test { let mut governance = Governance::new( governance_proto, - Box::new(environment), - Box::new(StubIcpLedger {}), - Box::new(StubCMC {}), + Arc::new(environment), + Arc::new(StubIcpLedger {}), + Arc::new(StubCMC {}), + Box::new(MockRandomness::new()), ); assert_eq!( diff --git a/rs/nns/governance/tests/degraded_mode.rs b/rs/nns/governance/tests/degraded_mode.rs index 3eb3ac98302..bb82bda6637 100644 --- a/rs/nns/governance/tests/degraded_mode.rs +++ b/rs/nns/governance/tests/degraded_mode.rs @@ -8,10 +8,10 @@ use ic_crypto_sha2::Sha256; use ic_nervous_system_common::{cmc::CMC, ledger::IcpLedger, NervousSystemError}; use ic_nns_common::pb::v1::NeuronId; use ic_nns_constants::GOVERNANCE_CANISTER_ID; +use ic_nns_governance::canister_state::CanisterRandomnessGenerator; use ic_nns_governance::{ governance::{ - Environment, Governance, HeapGrowthPotential, RngError, - HEAP_SIZE_SOFT_LIMIT_IN_WASM32_PAGES, + Environment, Governance, HeapGrowthPotential, HEAP_SIZE_SOFT_LIMIT_IN_WASM32_PAGES, }, pb::v1::{ governance_error::ErrorType, @@ -30,6 +30,7 @@ use ic_nns_governance_api::pb::v1::{ use icp_ledger::{AccountIdentifier, Subaccount, Tokens}; use maplit::btreemap; use std::convert::TryFrom; +use std::sync::Arc; struct DegradedEnv {} #[async_trait] @@ -38,22 +39,6 @@ impl Environment for DegradedEnv { 111000222 } - fn random_u64(&mut self) -> Result { - Ok(4) // https://xkcd.com/221 - } - - fn random_byte_array(&mut self) -> Result<[u8; 32], RngError> { - unimplemented!() - } - - fn seed_rng(&mut self, _seed: [u8; 32]) { - todo!() - } - - fn get_rng_seed(&self) -> Option<[u8; 32]> { - todo!() - } - fn execute_nns_function(&self, _: u64, _: &ExecuteNnsFunction) -> Result<(), GovernanceError> { unimplemented!() } @@ -100,7 +85,7 @@ impl IcpLedger for DegradedEnv { #[async_trait] impl CMC for DegradedEnv { - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { unimplemented!() } } @@ -143,9 +128,10 @@ fn fixture_two_neurons_second_is_bigger() -> GovernanceProto { fn degraded_governance() -> Governance { Governance::new( fixture_two_neurons_second_is_bigger(), - Box::new(DegradedEnv {}), - Box::new(DegradedEnv {}), - Box::new(DegradedEnv {}), + Arc::new(DegradedEnv {}), + Arc::new(DegradedEnv {}), + Arc::new(DegradedEnv {}), + Box::new(CanisterRandomnessGenerator::new()), ) } diff --git a/rs/nns/governance/tests/fake.rs b/rs/nns/governance/tests/fake.rs index f900aa184fc..b832e430b74 100644 --- a/rs/nns/governance/tests/fake.rs +++ b/rs/nns/governance/tests/fake.rs @@ -45,6 +45,7 @@ pub const NODE_PROVIDER_REWARD: u64 = 10_000; use ic_nns_governance::governance::tla::{ self, account_to_tla, tla_function, Destination, ToTla, TLA_INSTRUMENTATION_STATE, }; +use ic_nns_governance::governance::RandomnessGenerator; use ic_nns_governance::{tla_log_request, tla_log_response}; lazy_static! { @@ -181,22 +182,29 @@ impl FakeDriver { } /// Constructs an `Environment` that interacts with this driver. - pub fn get_fake_env(&self) -> Box { - Box::new(FakeDriver { + pub fn get_fake_env(&self) -> Arc { + Arc::new(FakeDriver { state: Arc::clone(&self.state), error_on_next_ledger_call: Arc::clone(&self.error_on_next_ledger_call), }) } /// Constructs a `Ledger` that interacts with this driver. - pub fn get_fake_ledger(&self) -> Box { - Box::new(FakeDriver { + pub fn get_fake_ledger(&self) -> Arc { + Arc::new(FakeDriver { + state: Arc::clone(&self.state), + error_on_next_ledger_call: Arc::clone(&self.error_on_next_ledger_call), + }) + } + + pub fn get_fake_cmc(&self) -> Arc { + Arc::new(FakeDriver { state: Arc::clone(&self.state), error_on_next_ledger_call: Arc::clone(&self.error_on_next_ledger_call), }) } - pub fn get_fake_cmc(&self) -> Box { + pub fn get_fake_randomness_generator(&self) -> Box { Box::new(FakeDriver { state: Arc::clone(&self.state), error_on_next_ledger_call: Arc::clone(&self.error_on_next_ledger_call), @@ -392,17 +400,12 @@ impl IcpLedger for FakeDriver { #[async_trait] impl CMC for FakeDriver { - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { Ok(100) } } -#[async_trait] -impl Environment for FakeDriver { - fn now(&self) -> u64 { - self.state.try_lock().unwrap().now - } - +impl RandomnessGenerator for FakeDriver { fn random_u64(&mut self) -> Result { Ok(self.state.try_lock().unwrap().rng.next_u64()) } @@ -420,6 +423,13 @@ impl Environment for FakeDriver { fn get_rng_seed(&self) -> Option<[u8; 32]> { todo!() } +} + +#[async_trait] +impl Environment for FakeDriver { + fn now(&self) -> u64 { + self.state.try_lock().unwrap().now + } fn execute_nns_function( &self, diff --git a/rs/nns/governance/tests/fixtures/environment_fixture.rs b/rs/nns/governance/tests/fixtures/environment_fixture.rs index 1937722a93f..17491558a94 100644 --- a/rs/nns/governance/tests/fixtures/environment_fixture.rs +++ b/rs/nns/governance/tests/fixtures/environment_fixture.rs @@ -1,6 +1,7 @@ use async_trait::async_trait; use candid::{CandidType, Decode, Encode, Error}; use ic_base_types::CanisterId; +use ic_nns_governance::governance::RandomnessGenerator; use ic_nns_governance::{ governance::{Environment, HeapGrowthPotential, RngError}, pb::v1::{ExecuteNnsFunction, GovernanceError}, @@ -65,7 +66,7 @@ impl EnvironmentFixture { } } - pub fn advance_time_by(&mut self, delta_seconds: u64) { + pub fn advance_time_by(&self, delta_seconds: u64) { self.environment_fixture_state.try_lock().unwrap().now += delta_seconds } @@ -146,50 +147,6 @@ impl Environment for EnvironmentFixture { self.environment_fixture_state.try_lock().unwrap().now } - fn random_u64(&mut self) -> Result { - match self - .environment_fixture_state - .try_lock() - .unwrap() - .rng - .as_mut() - { - Some(rand) => Ok(rand.next_u64()), - None => Err(RngError::RngNotInitialized), - } - } - - fn random_byte_array(&mut self) -> Result<[u8; 32], RngError> { - match self - .environment_fixture_state - .try_lock() - .unwrap() - .rng - .as_mut() - { - Some(rand) => { - let mut bytes = [0u8; 32]; - rand.fill_bytes(&mut bytes); - Ok(bytes) - } - // Kick the thing - None => Err(RngError::RngNotInitialized), - } - } - - fn seed_rng(&mut self, _seed: [u8; 32]) { - unimplemented!() - } - - fn get_rng_seed(&self) -> Option<[u8; 32]> { - self.environment_fixture_state - .try_lock() - .unwrap() - .rng - .as_ref() - .map(|r| r.get_seed()) - } - fn execute_nns_function( &self, _proposal_id: u64, @@ -240,3 +197,49 @@ impl Environment for EnvironmentFixture { } } } + +impl RandomnessGenerator for EnvironmentFixture { + fn random_u64(&mut self) -> Result { + match self + .environment_fixture_state + .try_lock() + .unwrap() + .rng + .as_mut() + { + Some(rand) => Ok(rand.next_u64()), + None => Err(RngError::RngNotInitialized), + } + } + + fn random_byte_array(&mut self) -> Result<[u8; 32], RngError> { + match self + .environment_fixture_state + .try_lock() + .unwrap() + .rng + .as_mut() + { + Some(rand) => { + let mut bytes = [0u8; 32]; + rand.fill_bytes(&mut bytes); + Ok(bytes) + } + + None => Err(RngError::RngNotInitialized), + } + } + + fn seed_rng(&mut self, _seed: [u8; 32]) { + unimplemented!() + } + + fn get_rng_seed(&self) -> Option<[u8; 32]> { + self.environment_fixture_state + .try_lock() + .unwrap() + .rng + .as_ref() + .map(|r| r.get_seed()) + } +} diff --git a/rs/nns/governance/tests/fixtures/mod.rs b/rs/nns/governance/tests/fixtures/mod.rs index 197cce81f32..bf57420200c 100755 --- a/rs/nns/governance/tests/fixtures/mod.rs +++ b/rs/nns/governance/tests/fixtures/mod.rs @@ -52,6 +52,7 @@ use std::{ use ic_nns_governance::governance::tla::{ self, account_to_tla, Destination, ToTla, TLA_INSTRUMENTATION_STATE, }; +use ic_nns_governance::governance::RandomnessGenerator; use ic_nns_governance::{tla_log_request, tla_log_response}; pub mod environment_fixture; @@ -511,12 +512,7 @@ impl IcpLedger for NNSFixture { } } -#[async_trait] -impl Environment for NNSFixture { - fn now(&self) -> u64 { - self.nns_state.try_lock().unwrap().environment.now() - } - +impl RandomnessGenerator for NNSFixture { fn random_u64(&mut self) -> Result { self.nns_state.try_lock().unwrap().environment.random_u64() } @@ -544,6 +540,13 @@ impl Environment for NNSFixture { .environment .get_rng_seed() } +} + +#[async_trait] +impl Environment for NNSFixture { + fn now(&self) -> u64 { + self.nns_state.try_lock().unwrap().environment.now() + } fn execute_nns_function( &self, @@ -583,7 +586,7 @@ impl Environment for NNSFixture { #[async_trait] impl CMC for NNSFixture { - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { Ok(100) } } @@ -991,12 +994,7 @@ impl IcpLedger for NNS { } } -#[async_trait] -impl Environment for NNS { - fn now(&self) -> u64 { - self.fixture.now() - } - +impl RandomnessGenerator for NNS { fn random_u64(&mut self) -> Result { self.fixture.random_u64() } @@ -1012,6 +1010,12 @@ impl Environment for NNS { fn get_rng_seed(&self) -> Option<[u8; 32]> { unimplemented!() } +} +#[async_trait] +impl Environment for NNS { + fn now(&self) -> u64 { + self.fixture.now() + } fn execute_nns_function( &self, @@ -1035,8 +1039,8 @@ impl Environment for NNS { } } -pub type LedgerTransform = Box) -> Box>; -pub type EnvironmentTransform = Box) -> Box>; +pub type LedgerTransform = Box) -> Arc>; +pub type EnvironmentTransform = Box) -> Arc>; /// The NNSBuilder permits the declarative construction of an NNS fixture. All /// of the methods concern setting or querying what this initial state will @@ -1077,18 +1081,19 @@ impl NNSBuilder { ledger: self.ledger_builder.create(), environment: self.environment_builder.create(), }); - let cmc: Box = Box::new(fixture.clone()); - let mut ledger: Box = Box::new(fixture.clone()); + let cmc: Arc = Arc::new(fixture.clone()); + let mut ledger: Arc = Arc::new(fixture.clone()); for t in self.ledger_transforms { ledger = t(ledger); } - let mut environment: Box = Box::new(fixture.clone()); + let mut environment: Arc = Arc::new(fixture.clone()); for t in self.environment_transforms { environment = t(environment); } + let randomness = Box::new(fixture.clone()); let mut nns = NNS { fixture: fixture.clone(), - governance: Governance::new(self.governance, environment, ledger, cmc), + governance: Governance::new(self.governance, environment, ledger, cmc, randomness), initial_state: None, }; nns.capture_state(); diff --git a/rs/nns/governance/tests/governance.rs b/rs/nns/governance/tests/governance.rs index ef7421eb608..d7802bb9488 100644 --- a/rs/nns/governance/tests/governance.rs +++ b/rs/nns/governance/tests/governance.rs @@ -43,13 +43,14 @@ use ic_nns_common::{ use ic_nns_constants::{ GOVERNANCE_CANISTER_ID, LEDGER_CANISTER_ID as ICP_LEDGER_CANISTER_ID, SNS_WASM_CANISTER_ID, }; +use ic_nns_governance::governance::RandomnessGenerator; use ic_nns_governance::{ governance::{ get_node_provider_reward, test_data::{ CREATE_SERVICE_NERVOUS_SYSTEM, CREATE_SERVICE_NERVOUS_SYSTEM_WITH_MATCHED_FUNDING, }, - Environment, Governance, HeapGrowthPotential, RngError, + Environment, Governance, HeapGrowthPotential, EXECUTE_NNS_FUNCTION_PAYLOAD_LISTING_BYTES_MAX, INITIAL_NEURON_DISSOLVE_DELAY, MAX_DISSOLVE_DELAY_SECONDS, MAX_NEURON_AGE_FOR_AGE_BONUS, MAX_NEURON_CREATION_SPIKE, MAX_NUMBER_OF_PROPOSALS_WITH_BALLOTS, MIN_DISSOLVE_DELAY_FOR_VOTE_ELIGIBILITY_SECONDS, @@ -632,6 +633,7 @@ fn check_proposal_status_after_voting_and_after_expiration( fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let pid = behavior @@ -1482,6 +1484,7 @@ async fn test_cascade_following() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); gov.make_proposal( &NeuronId { id: 1 }, @@ -1597,6 +1600,7 @@ async fn test_minimum_icp_xdr_conversion_rate() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Set minimum conversion rate. gov.heap_data @@ -1663,6 +1667,7 @@ async fn test_minimum_icp_xdr_conversion_rate_limits_monthly_node_provider_rewar driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Set minimum conversion rate. let minimum_icp_xdr_rate = 100; @@ -1722,6 +1727,7 @@ async fn test_manage_network_economics_change_one_deep_subfield() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Call the code under test. Make a ManageNetworkEconomics proposal. @@ -1835,6 +1841,7 @@ async fn test_manage_network_economics_reject_invalid() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Call the code under test. Make a ManageNetworkEconomics proposal. @@ -2072,6 +2079,7 @@ async fn test_manage_network_economics_revalidate_at_execution_time( driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 1.2: Make a couple of proposals. @@ -2239,6 +2247,7 @@ async fn test_mint_monthly_node_provider_rewards() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run twice heartbeat so that minting rewards is reached. @@ -2285,6 +2294,7 @@ async fn test_node_provider_must_be_registered() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let node_provider = NodeProvider { id: Some(PrincipalId::try_from(b"SID2".to_vec()).unwrap()), @@ -2349,6 +2359,7 @@ async fn test_sufficient_stake() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Set stake to 0.5 ICP. gov.neuron_store @@ -2411,6 +2422,7 @@ async fn test_all_follow_proposer() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Later, we'll inspect the same field again to verify that voting power @@ -2513,6 +2525,7 @@ async fn test_follow_negative() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); gov.make_proposal( &NeuronId { id: 1 }, @@ -2601,6 +2614,7 @@ async fn test_no_default_follow_for_governance() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); gov.make_proposal( &NeuronId { id: 1 }, @@ -2670,6 +2684,7 @@ async fn test_no_voting_after_deadline() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let proposal_id = gov .make_proposal( @@ -2790,6 +2805,7 @@ fn test_enforce_private_neuron() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Call code under test. @@ -2842,6 +2858,7 @@ fn test_query_for_manage_neuron() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Test that anybody can call `get_neuron_info` as long as the // neuron exists. @@ -2981,6 +2998,7 @@ async fn test_manage_neuron() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Make a proposal to replace the list of followees (2-4) with just 2. gov.make_proposal( @@ -3164,6 +3182,7 @@ async fn test_sufficient_stake_for_manage_neuron() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Set stake to less than 0.01 ICP (same as // neuron_management_fee_per_proposal_e8s). @@ -3285,6 +3304,7 @@ async fn test_invalid_proposals_fail() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let long_string = (0..(PROPOSAL_MOTION_TEXT_BYTES_MAX + 1)) @@ -3327,6 +3347,7 @@ async fn test_compute_tally_while_open() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Make the proposal from the smaller neuron. @@ -3371,6 +3392,7 @@ async fn test_compute_tally_after_decided() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Make the proposal from the larger neuron. @@ -3436,6 +3458,7 @@ async fn test_no_compute_tally_after_deadline() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Make the proposal from the larger neuron and let the smaller neuron vote no. @@ -3511,6 +3534,7 @@ async fn test_reward_event_proposals_last_longer_than_reward_period() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let expected_initial_event = RewardEvent { day_after_genesis: 0, @@ -3715,6 +3739,7 @@ async fn test_restricted_proposals_are_not_eligible_for_voting_rewards() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); gov.run_periodic_tasks().now_or_never(); // Initial reward event @@ -3848,6 +3873,7 @@ fn test_reward_distribution_skips_deleted_neurons() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Make sure that the fixture function indeed did not create a neuron 999. @@ -3925,6 +3951,7 @@ async fn test_genesis_in_the_future_in_supported() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); gov.run_periodic_tasks().now_or_never(); // At genesis, we should create an empty reward event @@ -4198,6 +4225,7 @@ fn compute_maturities( fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let expected_initial_event = RewardEvent { @@ -4725,6 +4753,7 @@ fn test_approve_kyc() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let neuron_a = gov .neuron_store @@ -4911,6 +4940,7 @@ fn test_get_neuron_ids_by_principal() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); assert_eq!( @@ -5004,6 +5034,7 @@ fn governance_with_staked_neuron( driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Add a stake transfer for this neuron, emulating a ledger call. @@ -5351,6 +5382,7 @@ fn governance_with_staked_unclaimed_neuron( driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); (driver, gov, to_subaccount) @@ -7124,6 +7156,7 @@ async fn test_neuron_with_non_self_authenticating_controller_is_now_allowed() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Call code under test. @@ -7347,6 +7380,7 @@ fn governance_with_neurons(neurons: &[Neuron]) -> (fake::FakeDriver, Governance) driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); assert_eq!(gov.neuron_store.len(), neurons.len()); (driver, gov) @@ -8474,6 +8508,7 @@ fn test_get_proposal_info() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let caller = &principal(1); @@ -8502,6 +8537,7 @@ fn test_list_proposals_removes_execute_nns_function_payload() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let caller = &principal(1); @@ -8540,6 +8576,7 @@ fn test_list_proposals_retains_execute_nns_function_payload() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let caller = &principal(1); @@ -8580,6 +8617,7 @@ fn test_get_pending_proposals_removes_execute_nns_function_payload() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let caller = &principal(1); @@ -8635,6 +8673,7 @@ fn test_list_proposals() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let caller = &principal(1); @@ -8766,6 +8805,7 @@ fn test_filter_proposals_neuron_visibility() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Principal 1 is a manager of the neuron 3 which is managed by proposal 1. @@ -8857,6 +8897,7 @@ fn test_filter_proposals_include_all_manage_neuron_ignores_visibility() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Without the include_all_manage_neuron_proposals option, principal2 will @@ -8970,6 +9011,7 @@ fn test_filter_proposals_by_status() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); assert_eq!( @@ -9061,6 +9103,7 @@ fn test_filter_proposals_by_reward_status() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); assert_eq!( @@ -9161,6 +9204,7 @@ fn test_filter_proposals_excluding_topics() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); assert_eq!( @@ -9247,6 +9291,7 @@ fn test_filter_proposal_ballots() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Principal1 should only see its own ballot. @@ -9324,6 +9369,7 @@ async fn test_make_proposal_message() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // submit proposal @@ -9426,6 +9472,7 @@ fn test_omit_large_fields() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); fn get_logo(list_proposals_response: ListProposalInfoResponse) -> Option { @@ -9513,6 +9560,7 @@ async fn test_max_number_of_proposals_with_ballots() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Vote with neuron 1. It is smaller, so proposals are not auto-accepted. for i in 0..MAX_NUMBER_OF_PROPOSALS_WITH_BALLOTS { @@ -9671,6 +9719,7 @@ fn test_proposal_gc() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); assert_eq!(999, gov.heap_data.proposals.len()); // First check GC does not take place if @@ -9748,6 +9797,7 @@ fn test_gc_ignores_exempt_proposals() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); assert_eq!(999, gov.heap_data.proposals.len()); gov.latest_gc_timestamp_seconds = driver.now() - 60; @@ -9765,6 +9815,7 @@ async fn test_id_v1_works() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Make a proposal to replace the list of followees (2-4) with just 2. gov.make_proposal( @@ -9804,6 +9855,7 @@ fn test_can_follow_by_subaccount_and_neuron_id() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let nid = NeuronId { id: 2 }; @@ -9895,6 +9947,7 @@ fn test_manage_neuron_merge_maturity_returns_expected_error() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let response = gov @@ -9965,6 +10018,7 @@ fn test_start_dissolving() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Advance time so that the neuron has age. @@ -10024,6 +10078,7 @@ fn test_start_dissolving_panics() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let neuron_info = gov @@ -10069,6 +10124,7 @@ fn test_stop_dissolving() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let neuron_info = gov @@ -10131,6 +10187,7 @@ fn test_stop_dissolving_panics() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let neuron_info = gov @@ -10304,6 +10361,7 @@ fn test_increase_dissolve_delay() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Tests for neuron 1. Non-dissolving. increase_dissolve_delay(&mut gov, principal_id, 1, 1); @@ -10452,6 +10510,7 @@ fn test_join_neurons_fund() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); { let actual_metrics = gov.compute_cached_metrics(now, total_icp_suppply); @@ -10800,6 +10859,7 @@ fn test_neuron_set_visibility() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Call the code under test. @@ -10909,6 +10969,7 @@ fn test_deciding_and_potential_voting_power() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let original_potential_voting_power = governance @@ -11117,6 +11178,7 @@ fn test_include_public_neurons_in_full_neurons() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Call the code under test. @@ -11207,6 +11269,7 @@ async fn test_refresh_voting_power() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let delta_seconds = 99; @@ -11350,6 +11413,7 @@ fn wait_for_quiet_test_helper( fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let pid = gov .make_proposal( @@ -12264,6 +12328,7 @@ async fn test_known_neurons() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); gov.make_proposal( @@ -12521,22 +12586,6 @@ impl Environment for MockEnvironment<'_> { DEFAULT_TEST_START_TIMESTAMP_SECONDS } - fn random_u64(&mut self) -> Result { - Ok(RANDOM_U64) - } - - fn random_byte_array(&mut self) -> Result<[u8; 32], RngError> { - panic!("Unexpected call to Environment::random_byte_array"); - } - - fn seed_rng(&mut self, _seed: [u8; 32]) { - unimplemented!() - } - - fn get_rng_seed(&self) -> Option<[u8; 32]> { - unimplemented!() - } - fn execute_nns_function( &self, _proposal_id: u64, @@ -13303,12 +13352,13 @@ async fn test_settle_neurons_fund_participation_restores_lifecycle_on_sns_w_fail let driver = fake::FakeDriver::default(); let mut gov = Governance::new( governance_proto, - Box::new(MockEnvironment { + Arc::new(MockEnvironment { expected_call_canister_method_calls: Arc::clone(&expected_call_canister_method_calls), call_canister_method_min_duration: None, }), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run code under test. @@ -13438,12 +13488,13 @@ async fn test_settle_neurons_fund_participation_restores_lifecycle_on_ledger_fai ))]); let mut gov = Governance::new( governance_proto, - Box::new(MockEnvironment { + Arc::new(MockEnvironment { expected_call_canister_method_calls: Arc::clone(&expected_call_canister_method_calls), call_canister_method_min_duration: None, }), - Box::new(icp_ledger), + Arc::new(icp_ledger), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run code under test. @@ -13577,12 +13628,13 @@ async fn test_create_service_nervous_system_failure_due_to_swap_deployment_error let driver = fake::FakeDriver::default().with_ledger_accounts(vec![]); // Initialize the minting account let mut gov = Governance::new( governance_proto, - Box::new(MockEnvironment { + Arc::new(MockEnvironment { expected_call_canister_method_calls: Arc::clone(&expected_call_canister_method_calls), call_canister_method_min_duration: None, }), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run code under test. This is done indirectly via proposal. The @@ -13679,12 +13731,13 @@ async fn test_create_service_nervous_system_settles_neurons_fund_commit() { let driver = fake::FakeDriver::default().with_ledger_accounts(vec![]); // Initialize the minting account let mut gov = Governance::new( governance_proto, - Box::new(MockEnvironment { + Arc::new(MockEnvironment { expected_call_canister_method_calls: Arc::clone(&expected_call_canister_method_calls), call_canister_method_min_duration: None, }), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run code under test. This is done indirectly via proposal. The @@ -13826,12 +13879,13 @@ async fn test_create_service_nervous_system_settles_neurons_fund_abort() { let driver = fake::FakeDriver::default().with_ledger_accounts(vec![]); // Initialize the minting account let mut gov = Governance::new( governance_proto, - Box::new(MockEnvironment { + Arc::new(MockEnvironment { expected_call_canister_method_calls: Arc::clone(&expected_call_canister_method_calls), call_canister_method_min_duration: None, }), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run code under test. This is done indirectly via proposal. The @@ -13984,12 +14038,13 @@ async fn test_create_service_nervous_system_proposal_execution_fails() { // This is where the main expectation is set. To wit, we expect that // execution of the proposal will cause governance to call out to the // swap canister. - Box::new(MockEnvironment { + Arc::new(MockEnvironment { expected_call_canister_method_calls: Arc::clone(&expected_call_canister_method_calls), call_canister_method_min_duration: None, }), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run code under test. This is done indirectly via proposal. The @@ -14086,12 +14141,13 @@ async fn test_settle_neurons_fund_is_idempotent_for_create_service_nervous_syste let driver = fake::FakeDriver::default().with_ledger_accounts(vec![]); // Initialize the minting account let mut gov = Governance::new( governance_proto, - Box::new(MockEnvironment { + Arc::new(MockEnvironment { expected_call_canister_method_calls: Arc::clone(&expected_call_canister_method_calls), call_canister_method_min_duration: None, }), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); // Step 2: Run code under test. @@ -14334,6 +14390,7 @@ async fn distribute_rewards_load_test() { helper.get_fake_env(), helper.get_fake_ledger(), helper.get_fake_cmc(), + helper.get_fake_randomness_generator(), ); // Prevent gc. governance.latest_gc_timestamp_seconds = now; @@ -14430,6 +14487,7 @@ async fn test_proposal_url_not_on_list_fails() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Submit a proposal with a url not on the whitelist @@ -14504,6 +14562,7 @@ fn test_ready_to_be_settled_proposals_ids() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); let rewardable_proposal_ids = |now_timestamp_seconds| -> Vec { governance @@ -14679,6 +14738,7 @@ async fn test_metrics() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let actual_metrics = gov.compute_cached_metrics(now, Tokens::new(0, 0).unwrap()); @@ -14893,6 +14953,7 @@ fn randomly_pick_swap_start() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Generate "zillions" of outputs, and count their occurrences. @@ -14972,6 +15033,7 @@ fn compute_closest_proposal_deadline_timestamp_seconds_no_wfq_fallback() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Check that the closest deadline is the one for the proposal we injected. @@ -15022,6 +15084,7 @@ fn compute_closest_proposal_deadline_timestamp_seconds_incorporates_wfq() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Check that the closest deadline is the one for the proposal we injected, @@ -15051,6 +15114,7 @@ fn voting_period_seconds_topic_dependency() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let voting_period_fun = gov.voting_period_seconds(); @@ -15178,6 +15242,7 @@ fn test_neuron_info_private_enforcement() { driver.get_fake_env(), driver.get_fake_ledger(), driver.get_fake_cmc(), + driver.get_fake_randomness_generator(), ); let main = |neuron_id_to_expect_redact: Vec<(NeuronId, bool)>| { @@ -15346,7 +15411,7 @@ impl IcpLedger for StubIcpLedger { struct StubCMC {} #[async_trait] impl CMC for StubCMC { - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { unimplemented!() } } diff --git a/rs/nns/governance/tests/interleaving/mod.rs b/rs/nns/governance/tests/interleaving/mod.rs index 104a4ece055..198240f38a7 100644 --- a/rs/nns/governance/tests/interleaving/mod.rs +++ b/rs/nns/governance/tests/interleaving/mod.rs @@ -8,7 +8,7 @@ use futures::channel::{ use ic_base_types::CanisterId; use ic_nervous_system_common::{ledger::IcpLedger, NervousSystemError}; use icp_ledger::{AccountIdentifier, Subaccount, Tokens}; -use std::sync::{atomic, atomic::Ordering as AOrdering}; +use std::sync::{atomic, atomic::Ordering as AOrdering, Arc}; pub mod test_data; @@ -27,7 +27,7 @@ pub type LedgerObserver = USender; /// A mock ledger to test interleavings of governance method calls. pub struct InterleavingTestLedger { - underlying: Box, + underlying: Arc, observer: LedgerObserver, } @@ -39,7 +39,7 @@ impl InterleavingTestLedger { /// underlying ledger, or, alternatively, return an error. This is done /// through a one-shot channel, the sender side of which is sent to the /// observer. - pub fn new(underlying: Box, observer: LedgerObserver) -> Self { + pub fn new(underlying: Arc, observer: LedgerObserver) -> Self { InterleavingTestLedger { underlying, observer, diff --git a/rs/nns/governance/tests/interleaving_tests.rs b/rs/nns/governance/tests/interleaving_tests.rs index a3ccbb5600c..d0af451ed51 100644 --- a/rs/nns/governance/tests/interleaving_tests.rs +++ b/rs/nns/governance/tests/interleaving_tests.rs @@ -26,6 +26,7 @@ use ic_sns_wasm::pb::v1::DeployedSns; use icp_ledger::AccountIdentifier; use interleaving::{InterleavingTestLedger, LedgerControlMessage}; use rust_decimal_macros::dec; +use std::sync::Arc; use std::{ pin::Pin, sync::{atomic, atomic::Ordering as AOrdering}, @@ -60,7 +61,7 @@ fn test_cant_increase_dissolve_delay_while_disbursing() { .set_kyc_verified(true), ) .add_ledger_transform(Box::new(move |l| { - Box::new(InterleavingTestLedger::new(l, tx)) + Arc::new(InterleavingTestLedger::new(l, tx)) })) .set_economics(NetworkEconomics::default()) .create(); @@ -256,7 +257,7 @@ fn test_cant_interleave_calls_to_settle_neurons_fund() { ) .add_account_for(sns_governance_canister_id, 0) // Setup the treasury account .add_ledger_transform(Box::new(move |l| { - Box::new(InterleavingTestLedger::new(l, tx)) + Arc::new(InterleavingTestLedger::new(l, tx)) })) .set_economics(NetworkEconomics::default()) .create(); diff --git a/rs/nns/governance/tests/monitoring.rs b/rs/nns/governance/tests/monitoring.rs index 781789972d0..db9c5ff18d5 100644 --- a/rs/nns/governance/tests/monitoring.rs +++ b/rs/nns/governance/tests/monitoring.rs @@ -28,6 +28,7 @@ fn test_reward_event_amounts_metrics() { helpers.get_fake_env(), helpers.get_fake_ledger(), helpers.get_fake_cmc(), + helpers.get_fake_randomness_generator(), ); let now = 123_456_789; diff --git a/rs/nns/governance/tests/proposals.rs b/rs/nns/governance/tests/proposals.rs index 7b4d2173b7d..6cf9dc72264 100644 --- a/rs/nns/governance/tests/proposals.rs +++ b/rs/nns/governance/tests/proposals.rs @@ -218,6 +218,7 @@ async fn test_distribute_rewards_with_total_potential_voting_power() { fake_driver.get_fake_env(), fake_driver.get_fake_ledger(), fake_driver.get_fake_cmc(), + fake_driver.get_fake_randomness_generator(), ); // Step 2: Call code under test. diff --git a/rs/sns/governance/tests/fixtures/mod.rs b/rs/sns/governance/tests/fixtures/mod.rs index ef790ceab1a..c21356c8807 100755 --- a/rs/sns/governance/tests/fixtures/mod.rs +++ b/rs/sns/governance/tests/fixtures/mod.rs @@ -215,7 +215,7 @@ impl CmcFixture { #[async_trait] impl CMC for CmcFixture { - async fn neuron_maturity_modulation(&mut self) -> Result { + async fn neuron_maturity_modulation(&self) -> Result { Ok(*self.maturity_modulation.try_lock().unwrap()) } }