diff --git a/rs/nns/governance/canister/canister.rs b/rs/nns/governance/canister/canister.rs index d2e351bf410..0d343ebe194 100644 --- a/rs/nns/governance/canister/canister.rs +++ b/rs/nns/governance/canister/canister.rs @@ -81,12 +81,13 @@ const WASM_PAGES_RESERVED_FOR_UPGRADES_MEMORY: u64 = 65_536; pub(crate) const LOG_PREFIX: &str = "[Governance] "; -// https://dfinity.atlassian.net/browse/NNS1-1050: We are not following -// standard/best practices for canister globals here. -// -// Do not access these global variables directly. Instead, use accessor -// functions, which are defined immediately after. -static mut GOVERNANCE: Option = None; +thread_local! { + static GOVERNANCE: RefCell = RefCell::new(Governance::new_uninitialized( + Box::new(CanisterEnv::new()), + Box::new(IcpLedgerCanister::::new(LEDGER_CANISTER_ID)), + Box::new(CMCCanister::::new()), + )); +} /* Recommendations for Using `unsafe` in the Governance canister: @@ -135,7 +136,7 @@ are best practices for making use of the unsafe block: /// This should only be called once the global state has been initialized, which /// happens in `canister_init` or `canister_post_upgrade`. fn governance() -> &'static Governance { - unsafe { GOVERNANCE.as_ref().expect("Canister not initialized!") } + unsafe { &*GOVERNANCE.with(|g| g.as_ptr()) } } /// Returns a mutable reference to the global state. @@ -143,19 +144,12 @@ fn governance() -> &'static Governance { /// This should only be called once the global state has been initialized, which /// happens in `canister_init` or `canister_post_upgrade`. fn governance_mut() -> &'static mut Governance { - unsafe { GOVERNANCE.as_mut().expect("Canister not initialized!") } + unsafe { &mut *GOVERNANCE.with(|g| g.as_ptr()) } } // Sets governance global state to the given object. fn set_governance(gov: Governance) { - unsafe { - assert!( - GOVERNANCE.is_none(), - "{}Trying to initialize an already-initialized governance canister!", - LOG_PREFIX - ); - GOVERNANCE = Some(gov); - } + GOVERNANCE.set(gov); governance() .validate() diff --git a/rs/nns/governance/src/governance.rs b/rs/nns/governance/src/governance.rs index 28b809f7cd5..d5829c24be5 100644 --- a/rs/nns/governance/src/governance.rs +++ b/rs/nns/governance/src/governance.rs @@ -117,7 +117,7 @@ use rust_decimal_macros::dec; use std::{ borrow::Cow, cmp::{max, Ordering}, - collections::{HashMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, convert::{TryFrom, TryInto}, fmt, future::Future, @@ -1935,6 +1935,30 @@ fn spawn_in_canister_env(future: impl Future + Sized + 'static) { } impl Governance { + /// Creates a new Governance instance with uninitialized fields. The canister should only have + /// such state before the state is recovered from the stable memory in post_upgrade or + /// 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, + ) -> Self { + Self { + heap_data: HeapGovernanceData::default(), + neuron_store: NeuronStore::new(BTreeMap::new()), + env, + ledger, + cmc, + closest_proposal_deadline_timestamp_seconds: 0, + latest_gc_timestamp_seconds: 0, + latest_gc_num_proposals: 0, + neuron_data_validator: NeuronDataValidator::new(), + minting_node_provider_rewards: false, + neuron_rate_limits: NeuronRateLimits::default(), + } + } + /// Initializes Governance for the first time from init payload. When restoring after an upgrade /// with its persisted state, `Governance::new_restored` should be called instead. pub fn new(