Eliminate uses of HashMap and thread_rng (fix #810)

soroban-env-host/src/auth.rs

@@ -1,15 +1,16 @@
 use std::cell::RefCell;
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::rc::Rc;
 
-use rand::Rng;
 use soroban_env_common::xdr::{
     ContractDataEntry, CreateContractArgs, HashIdPreimage, HashIdPreimageSorobanAuthorization,
     InvokeContractArgs, LedgerEntry, LedgerEntryData, LedgerEntryExt, ScAddress, ScErrorCode,
     ScErrorType, ScNonceKey, ScVal, SorobanAuthorizationEntry, SorobanAuthorizedFunction,
     SorobanCredentials,
 };
-use soroban_env_common::{AddressObject, Compare, Symbol, TryFromVal, TryIntoVal, Val, VecObject};
+use soroban_env_common::{
+    AddressObject, Compare, Env, Symbol, TryFromVal, TryIntoVal, Val, VecObject,
+};
 
 use crate::budget::{AsBudget, Budget};
 use crate::host::error::TryBorrowOrErr;
@@ -119,7 +120,7 @@ pub struct AuthorizationManagerSnapshot {
     account_trackers_snapshot: AccountTrackersSnapshot,
     invoker_contract_tracker_root_snapshots: Vec<AuthorizedInvocationSnapshot>,
     // This is set only for the recording mode.
-    tracker_by_address_handle: Option<HashMap<u32, usize>>,
+    tracker_by_address_handle: Option<BTreeMap<u32, usize>>,
 }
 
 // Snapshot of the `account_trackers` in `AuthorizationManager`.
@@ -149,7 +150,7 @@ struct RecordingAuthInfo {
     // This allows to disambiguate between the addresses that have the same
     // value, but are specified as two different objects (e.g. as two different
     // contract function inputs).
-    tracker_by_address_handle: RefCell<HashMap<u32, usize>>,
+    tracker_by_address_handle: RefCell<BTreeMap<u32, usize>>,
     // Whether to allow root authorized invocation to not match the root
     // contract invocation.
     disable_non_root_auth: bool,
@@ -159,7 +160,7 @@ impl RecordingAuthInfo {
     fn try_borrow_tracker_by_address_handle(
         &self,
         host: &Host,
-    ) -> Result<std::cell::Ref<'_, HashMap<u32, usize>>, HostError> {
+    ) -> Result<std::cell::Ref<'_, BTreeMap<u32, usize>>, HostError> {
         self.tracker_by_address_handle.try_borrow_or_err_with(
             host,
             "recording_auth_info.tracker_by_address_handle.try_borrow failed",
@@ -168,7 +169,7 @@ impl RecordingAuthInfo {
     fn try_borrow_tracker_by_address_handle_mut(
         &self,
         host: &Host,
-    ) -> Result<std::cell::RefMut<'_, HashMap<u32, usize>>, HostError> {
+    ) -> Result<std::cell::RefMut<'_, BTreeMap<u32, usize>>, HostError> {
         self.tracker_by_address_handle.try_borrow_mut_or_err_with(
             host,
             "recording_auth_info.tracker_by_address_handle.try_borrow_mut failed",
@@ -1448,7 +1449,7 @@ impl AccountAuthorizationTracker {
             false
         };
         let nonce = if !is_invoker {
-            let random_nonce: i64 = rand::thread_rng().gen_range(0..=i64::MAX);
+            let random_nonce: i64 = host.prng_u64_in_inclusive_range(0, u64::MAX)? as i64;
             host.consume_nonce(address, random_nonce, 0)?;
             Some((random_nonce, 0))
         } else {

soroban-env-host/src/host.rs

@@ -53,6 +53,10 @@ use self::{
 #[cfg(any(test, feature = "testutils"))]
 pub use frame::ContractFunctionSet;
 pub(crate) use frame::Frame;
+#[cfg(test)]
+use rand_chacha::ChaCha20Rng;
+#[cfg(test)]
+pub(crate) const TEST_PRNG_SEED: &[u8; 32] = b"12345678901234567890123456789012";
 
 /// Defines the maximum depth for recursive calls in the host, i.e. `Val` conversion, comparison,
 /// and deep clone, to prevent stack overflow.
@@ -113,12 +117,17 @@ struct HostImpl {
     authorization_manager: RefCell<AuthorizationManager>,
     diagnostic_level: RefCell<DiagnosticLevel>,
     base_prng: RefCell<Option<Prng>>,
+    // Some tests _of the host_ rely on pseudorandom _input_ data. For these cases we attach
+    // yet another unmetered PRNG to the host. This should not be exposed through "testutils"
+    // to clients testing contracts _against_ the host.
+    #[cfg(test)]
+    test_prng: RefCell<Option<ChaCha20Rng>>,
     // Note: we're not going to charge metering for testutils because it's out of the scope
     // of what users will be charged for in production -- it's scaffolding for testing a contract,
     // but shouldn't be charged to the contract itself (and will never be compiled-in to
     // production hosts)
     #[cfg(any(test, feature = "testutils"))]
-    contracts: RefCell<std::collections::HashMap<Hash, Rc<dyn ContractFunctionSet>>>,
+    contracts: RefCell<std::collections::BTreeMap<Hash, Rc<dyn ContractFunctionSet>>>,
     // Store a copy of the `AuthorizationManager` for the last host function
     // invocation. In order to emulate the production behavior in tests, we reset
     // authorization manager after every invocation (as it's not meant to be
@@ -220,8 +229,16 @@ impl_checked_borrow_helpers!(
     try_borrow_base_prng_mut
 );
 
+#[cfg(test)]
+impl_checked_borrow_helpers!(
+    test_prng,
+    Option<ChaCha20Rng>,
+    try_borrow_test_prng,
+    try_borrow_test_prng_mut
+);
+
 #[cfg(any(test, feature = "testutils"))]
-impl_checked_borrow_helpers!(contracts, std::collections::HashMap<Hash, Rc<dyn ContractFunctionSet>>, try_borrow_contracts, try_borrow_contracts_mut);
+impl_checked_borrow_helpers!(contracts, std::collections::BTreeMap<Hash, Rc<dyn ContractFunctionSet>>, try_borrow_contracts, try_borrow_contracts_mut);
 
 #[cfg(any(test, feature = "testutils"))]
 impl_checked_borrow_helpers!(
@@ -271,6 +288,8 @@ impl Host {
             ),
             diagnostic_level: Default::default(),
             base_prng: RefCell::new(None),
+            #[cfg(test)]
+            test_prng: RefCell::new(None),
             #[cfg(any(test, feature = "testutils"))]
             contracts: Default::default(),
             #[cfg(any(test, feature = "testutils"))]
@@ -296,6 +315,24 @@ impl Host {
         Ok(self.try_borrow_source_account()?.metered_clone(self)?)
     }
 
+    #[cfg(test)]
+    pub(crate) fn with_test_prng<T>(
+        &self,
+        f: impl FnOnce(&mut ChaCha20Rng) -> Result<T, HostError>,
+    ) -> Result<T, HostError> {
+        use rand::SeedableRng;
+
+        let mut tpo = self.try_borrow_test_prng_mut()?;
+        if let Some(p) = tpo.as_mut() {
+            f(p)
+        } else {
+            let mut p = ChaCha20Rng::from_seed(*TEST_PRNG_SEED);
+            let res = f(&mut p);
+            *tpo = Some(p);
+            res
+        }
+    }
+
     pub fn source_account_address(&self) -> Result<Option<AddressObject>, HostError> {
         if let Some(acc) = self.try_borrow_source_account()?.as_ref() {
             Ok(Some(self.add_host_object(ScAddress::Account(

soroban-env-host/src/native_contract/testutils.rs

@@ -2,7 +2,7 @@ use std::rc::Rc;
 
 use crate::{Host, LedgerInfo};
 use ed25519_dalek::{Signer, SigningKey};
-use rand::{thread_rng, Rng};
+use rand::Rng;
 use soroban_env_common::xdr::{
     AccountEntry, AccountEntryExt, AccountEntryExtensionV1, AccountEntryExtensionV1Ext,
     AccountEntryExtensionV2, AccountEntryExtensionV2Ext, AccountId, Hash, HashIdPreimage,
@@ -30,8 +30,9 @@ macro_rules! host_vec {
     };
 }
 
-pub(crate) fn generate_signing_key() -> SigningKey {
-    SigningKey::generate(&mut thread_rng())
+pub(crate) fn generate_signing_key(host: &Host) -> SigningKey {
+    host.with_test_prng(|chacha| Ok(SigningKey::generate(chacha)))
+        .unwrap()
 }
 
 pub(crate) fn signing_key_to_account_id(key: &SigningKey) -> AccountId {
@@ -203,9 +204,11 @@ pub(crate) fn authorize_single_invocation(
 ) {
     let nonce = match signer {
         TestSigner::AccountInvoker(_) => None,
-        TestSigner::Account(_) | TestSigner::AccountContract(_) => {
-            Some((thread_rng().gen_range(0..=i64::MAX), 10000))
-        }
+        TestSigner::Account(_) | TestSigner::AccountContract(_) => Some((
+            host.with_test_prng(|chacha| Ok(chacha.gen_range(0..=i64::MAX)))
+                .unwrap(),
+            10000,
+        )),
         TestSigner::ContractInvoker(_) => {
             return;
         }

soroban-env-host/src/test/auth.rs

@@ -1,5 +1,5 @@
 use ed25519_dalek::SigningKey;
-use rand::{thread_rng, Rng};
+use rand::Rng;
 use soroban_env_common::xdr::{
     AccountId, ContractDataDurability, HashIdPreimage, HashIdPreimageSorobanAuthorization,
     InvokeContractArgs, PublicKey, ScAddress, ScBytes, ScErrorCode, ScErrorType, ScNonceKey,
@@ -110,7 +110,7 @@ impl AuthTest {
         .unwrap();
         let mut accounts = vec![];
         for _ in 0..signer_cnt {
-            accounts.push(generate_signing_key());
+            accounts.push(generate_signing_key(&host));
         }
         for signing_key in &accounts {
             create_account(
@@ -218,7 +218,10 @@ impl AuthTest {
             let sc_address = self.key_to_sc_address(&self.keys[address_id]);
             let mut curr_nonces = vec![];
             for sign_root in &sign_payloads[address_id] {
-                let nonce = thread_rng().gen_range(0..=i64::MAX);
+                let nonce = self
+                    .host
+                    .with_test_prng(|chacha| Ok(chacha.gen_range(0..=i64::MAX)))
+                    .unwrap();
                 curr_nonces.push(nonce);
                 let root_invocation = self.convert_sign_node(sign_root);
                 let payload_preimage =

soroban-env-host/src/test/complex.rs

@@ -21,12 +21,13 @@ fn run_complex() -> Result<(), HostError> {
         min_temp_entry_ttl: 16,
         max_entry_ttl: 6312000,
     };
-    let account_id = generate_account_id();
-    let salt = generate_bytes_array();
+
+    let host = Host::test_host_with_recording_footprint();
+    let account_id = generate_account_id(&host);
+    let salt = generate_bytes_array(&host);
 
     // Run 1: record footprint, emulating "preflight".
     let foot = {
-        let host = Host::test_host_with_recording_footprint();
         host.set_ledger_info(info.clone())?;
         let contract_id_obj =
             host.register_test_contract_wasm_from_source_account(COMPLEX, account_id.clone(), salt);

soroban-env-host/src/test/lifecycle.rs

@@ -69,7 +69,7 @@ fn test_host() -> Host {
     let host = Host::with_storage_and_budget(storage, budget);
     host.set_ledger_info(LedgerInfo {
         protocol_version: crate::meta::get_ledger_protocol_version(crate::meta::INTERFACE_VERSION),
-        network_id: generate_bytes_array(),
+        network_id: generate_bytes_array(&host),
         ..Default::default()
     })
     .unwrap();
@@ -78,8 +78,8 @@ fn test_host() -> Host {
 }
 
 fn test_create_contract_from_source_account(host: &Host, wasm: &[u8]) -> Hash {
-    let source_account = generate_account_id();
-    let salt = generate_bytes_array();
+    let source_account = generate_account_id(host);
+    let salt = generate_bytes_array(host);
     host.set_source_account(source_account.clone()).unwrap();
     let contract_id_preimage = ContractIdPreimage::Address(ContractIdPreimageFromAddress {
         address: ScAddress::Account(source_account.clone()),
@@ -156,7 +156,7 @@ fn create_contract_using_parent_id_test() {
     let parent_contract_address = host
         .add_host_object(ScAddress::Contract(parent_contract_id.clone()))
         .unwrap();
-    let salt = generate_bytes_array();
+    let salt = generate_bytes_array(&host);
     let child_pre_image = HashIdPreimage::ContractId(HashIdPreimageContractId {
         network_id: host
             .hash_from_bytesobj_input("network_id", host.get_ledger_network_id().unwrap())
@@ -372,8 +372,8 @@ fn test_contract_wasm_update() {
 
 fn test_create_contract_from_source_account_recording_auth() {
     let host = Host::test_host_with_recording_footprint();
-    let source_account = generate_account_id();
-    let salt = generate_bytes_array();
+    let source_account = generate_account_id(&host);
+    let salt = generate_bytes_array(&host);
     host.set_source_account(source_account.clone()).unwrap();
     host.switch_to_recording_auth(true).unwrap();
     let contract_id_preimage = ContractIdPreimage::Address(ContractIdPreimageFromAddress {

soroban-env-host/src/test/metering_benchmark.rs

@@ -39,15 +39,15 @@ fn run_add_i32() -> Result<(), HostError> {
 
     let _test_span = tracy_span!("run_add_i32 test");
 
-    let account_id = generate_account_id();
-    let salt = generate_bytes_array();
+    let host = Host::test_host_with_recording_footprint();
+    let account_id = generate_account_id(&host);
+    let salt = generate_bytes_array(&host);
     let a = 4i32;
     let b = 7i32;
 
     // Run 1: record footprint, emulating "preflight".
     let foot = {
         let _run_span = tracy_span!("add_i32 run 1: recording footprint");
-        let host = Host::test_host_with_recording_footprint();
         host.set_ledger_info(LEDGER_INFO.clone())?;
         let contract_id_obj =
             host.register_test_contract_wasm_from_source_account(ADD_I32, account_id.clone(), salt);
@@ -90,13 +90,13 @@ fn run_complex() -> Result<(), HostError> {
     tracy_client::Client::start();
 
     let _test_span = tracy_span!("run_complex test");
-    let account_id = generate_account_id();
-    let salt = generate_bytes_array();
+    let host = Host::test_host_with_recording_footprint();
+    let account_id = generate_account_id(&host);
+    let salt = generate_bytes_array(&host);
 
     // Run 1: record footprint, emulating "preflight".
     let foot = {
         let _run_span = tracy_span!("complex run 1: recording footprint");
-        let host = Host::test_host_with_recording_footprint();
         host.set_ledger_info(LEDGER_INFO.clone())?;
         let contract_id_obj =
             host.register_test_contract_wasm_from_source_account(COMPLEX, account_id.clone(), salt);