Simplify global init and access

lurk-lab · Oct 19, 2023 · 2a0e685 · 2a0e685
1 parent dd58ae1
commit 2a0e685
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 79 deletions.
diff --git a/benches/common/mod.rs b/benches/common/mod.rs
@@ -1,22 +1,22 @@
 use anyhow::{bail, Result};
-use lurk::config::{Settings, LURK_CONFIG};
+use lurk::config::lurk_config;
+use std::collections::HashMap;
 
 pub const PUBLIC_PARAMS_BENCH_PATH: &str = "/var/tmp/lurk_benches/public_params";
 
 /// Sets the config settings with the temporary public parameter cache location
 pub fn set_bench_config() -> Result<()> {
-    let mut settings = Settings::new();
-    settings.public_params_dir = PUBLIC_PARAMS_BENCH_PATH.into();
+    let mut overrides = HashMap::new();
+    overrides.insert("public_params_dir", PUBLIC_PARAMS_BENCH_PATH.into());
+    let config = lurk_config(Some(&overrides));
     // If global config already set, check it has the same
     // temporary public param cache or return an error
-    LURK_CONFIG.set(settings).or_else(|preset| {
-        if preset.public_params_dir == PUBLIC_PARAMS_BENCH_PATH {
-            Ok(())
-        } else {
-            bail!(
-                "Incorrect public parameter cache directory: {}",
-                preset.public_params_dir
-            )
-        }
-    })
+    if config.public_params_dir == PUBLIC_PARAMS_BENCH_PATH {
+        Ok(())
+    } else {
+        bail!(
+            "Incorrect public parameter cache directory: {}",
+            config.public_params_dir
+        )
+    }
 }
diff --git a/src/circuit/circuit_frame.rs b/src/circuit/circuit_frame.rs
@@ -15,7 +15,7 @@ use crate::{
         data::GlobalAllocations,
         pointer::{AllocatedContPtr, AllocatedPtr, AsAllocatedHashComponents},
     },
-    config::LURK_CONFIG,
+    config::{lurk_config, LURK_CONFIG},
     eval::{empty_sym_env, lang::Lang},
     field::LurkField,
     hash::HashConst,
@@ -482,14 +482,7 @@ impl<'a, F: LurkField, C: Coprocessor<F>> MultiFrame<'a, F, C> {
         frames: &[CircuitFrame<'_, F, C>],
         g: &GlobalAllocations<F>,
     ) -> (AllocatedPtr<F>, AllocatedPtr<F>, AllocatedContPtr<F>) {
-        if cs.is_witness_generator()
-            && LURK_CONFIG
-                .get_or_init(crate::config::Settings::default)
-                .perf
-                .parallelism
-                .synthesis
-                .is_parallel()
-        {
+        if cs.is_witness_generator() && lurk_config(None).perf.parallelism.synthesis.is_parallel() {
             self.synthesize_frames_parallel(cs, store, input_expr, input_env, input_cont, frames, g)
         } else {
             self.synthesize_frames_sequential(

diff --git a/src/cli/config.rs b/src/cli/config.rs
@@ -15,16 +15,14 @@ use super::paths::{circom_default_dir, commits_default_dir, proofs_default_dir};
 /// Global config varable for CLI `Settings`
 pub static CLI_CONFIG: OnceCell<CliSettings> = OnceCell::new();
 
-/// Sets the global `LURK_CONFIG` and `CLI_CONFIG` variables unless already filled
-/// In case it fails to set, getter functions are required to use `get_or_init`
-/// to get a default value
-pub fn set_cli_config(overrides: &HashMap<&str, String>) {
+/// Sets the `LURK_CONFIG` if empty and gets the `CLI_CONFIG` from config file and env vars
+/// Optionally overrides the preconfigured settings when initializing
+pub fn cli_config(overrides: Option<&HashMap<&str, String>>) -> &'static CliSettings {
     LURK_CONFIG
         .set(Settings::from_config(&LURK_CONFIG_FILE, overrides).unwrap_or_default())
         .unwrap_or(());
     CLI_CONFIG
-        .set(CliSettings::from_config(&LURK_CONFIG_FILE, overrides).unwrap_or_default())
-        .unwrap_or(());
+        .get_or_init(|| CliSettings::from_config(&LURK_CONFIG_FILE, overrides).unwrap_or_default())
 }
 
 /// Contains the CLI config settings
@@ -60,15 +58,10 @@ pub struct CliSettings {
 }
 
 impl CliSettings {
-    /// Creates a default config, which can be modified before setting `CLI_CONFIG`
-    pub fn new() -> Self {
-        Self::default()
-    }
-
     /// Loads config settings from a file or env var, or CLI arg if applicable
     pub fn from_config(
         config_file: &Utf8PathBuf,
-        cli_settings: &HashMap<&str, String>,
+        cli_settings: Option<&HashMap<&str, String>>,
     ) -> Result<Self, ConfigError> {
         let (proofs, commits, circom, backend, field, rc, limit) = (
             "proofs_dir",
@@ -91,13 +84,13 @@ impl CliSettings {
             // Then overwrite with any `LURK` environment variables
             .add_source(Environment::with_prefix("LURK"))
             // TODO: Derive config::Source for `cli_settings` and use `add_source` instead
-            .set_override_option(proofs, cli_settings.get(proofs).map(|v| v.to_owned()))?
-            .set_override_option(commits, cli_settings.get(commits).map(|v| v.to_owned()))?
-            .set_override_option(circom, cli_settings.get(circom).map(|v| v.to_owned()))?
-            .set_override_option(backend, cli_settings.get(backend).map(|v| v.to_owned()))?
-            .set_override_option(field, cli_settings.get(field).map(|v| v.to_owned()))?
-            .set_override_option(rc, cli_settings.get(rc).map(|v| v.to_owned()))?
-            .set_override_option(limit, cli_settings.get(limit).map(|v| v.to_owned()))?
+            .set_override_option(proofs, cli_settings.and_then(|s| s.get(proofs).map(|v| v.to_owned())))?
+            .set_override_option(commits, cli_settings.and_then(|s| s.get(commits).map(|v| v.to_owned())))?
+            .set_override_option(circom, cli_settings.and_then(|s| s.get(circom).map(|v| v.to_owned())))?
+            .set_override_option(backend, cli_settings.and_then(|s| s.get(backend).map(|v| v.to_owned())))?
+            .set_override_option(field, cli_settings.and_then(|s| s.get(field).map(|v| v.to_owned())))?
+            .set_override_option(rc, cli_settings.and_then(|s| s.get(rc).map(|v| v.to_owned())))?
+            .set_override_option(limit, cli_settings.and_then(|s| s.get(limit).map(|v| v.to_owned())))?
             .build()
             .and_then(|c| c.try_deserialize())
     }
@@ -119,7 +112,6 @@ impl Default for CliSettings {
 
 #[cfg(test)]
 mod tests {
-    use super::*;
     use camino::Utf8Path;
     use std::io::prelude::*;
     use tempfile::Builder;
@@ -170,8 +162,8 @@ mod tests {
             .write_all(format!("limit = {limit}\n").as_bytes())
             .unwrap();
 
-        let cli_config = CliSettings::from_config(&config_dir, &HashMap::new()).unwrap();
-        let lurk_config = Settings::from_config(&config_dir, &HashMap::new()).unwrap();
+        let cli_config = CliSettings::from_config(&config_dir, None).unwrap();
+        let lurk_config = Settings::from_config(&config_dir, None).unwrap();
         assert_eq!(lurk_config.public_params_dir, public_params_dir);
         assert_eq!(cli_config.proofs_dir, proofs_dir);
         assert_eq!(cli_config.commits_dir, commits_dir);

diff --git a/src/cli/mod.rs b/src/cli/mod.rs
@@ -20,11 +20,7 @@ use std::{
 };
 
 use crate::{
-    cli::config::CLI_CONFIG,
-    cli::{
-        config::{set_cli_config, CliSettings},
-        paths::create_lurk_dirs,
-    },
+    cli::{backend::Backend, config::cli_config, paths::create_lurk_dirs},
     field::{LanguageField, LurkField},
     public_parameters::disk_cache::public_params_dir,
     public_parameters::instance::Metadata,
@@ -35,8 +31,6 @@ use crate::{
 
 use crate::cli::repl::{validate_non_zero, Repl};
 
-use self::backend::Backend;
-
 #[derive(Parser, Debug)]
 #[clap(version)]
 struct Cli {
@@ -316,11 +310,11 @@ impl ReplCli {
             limit
         );
 
-        set_cli_config(&cli_settings);
+        // Initializes CLI config with CLI arguments as overrides
+        let config = cli_config(Some(&cli_settings));
 
         create_lurk_dirs().unwrap();
 
-        let config = CLI_CONFIG.get_or_init(CliSettings::default);
         let rc = config.rc;
         let limit = config.limit;
         let backend = &config.backend;
@@ -373,11 +367,11 @@ impl LoadCli {
             limit
         );
 
-        set_cli_config(&cli_settings);
+        // Initializes CLI config with CLI arguments as overrides
+        let config = cli_config(Some(&cli_settings));
 
         create_lurk_dirs().unwrap();
 
-        let config = CLI_CONFIG.get_or_init(CliSettings::default);
         let rc = config.rc;
         let limit = config.limit;
         let backend = &config.backend;
@@ -545,7 +539,7 @@ impl Cli {
                 if let Some(dir) = verify_args.proofs_dir {
                     cli_settings.insert("proofs_dir", dir.to_string());
                 }
-                set_cli_config(&cli_settings);
+                cli_config(Some(&cli_settings));
                 Ok(())
             }
             Command::Circom(circom_args) => {
@@ -557,7 +551,7 @@ impl Cli {
                 if let Some(dir) = circom_args.circom_dir {
                     cli_settings.insert("circom_dir", dir.to_string());
                 }
-                set_cli_config(&cli_settings);
+                cli_config(Some(&cli_settings));
 
                 create_circom_gadget(circom_args.circom_folder, circom_args.name)?;
                 Ok(())
@@ -568,7 +562,7 @@ impl Cli {
                     cli_settings.insert("public_params_dir", dir.to_string());
                 }
 
-                set_cli_config(&cli_settings);
+                cli_config(Some(&cli_settings));
 
                 create_lurk_dirs().unwrap();
                 public_params_args.run()?;

diff --git a/src/cli/paths.rs b/src/cli/paths.rs
@@ -3,7 +3,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 
 use std::fs;
 
-use crate::cli::config::{CliSettings, CLI_CONFIG};
+use crate::cli::config::cli_config;
 use crate::public_parameters::disk_cache::public_params_dir;
 
 #[cfg(not(target_arch = "wasm32"))]
@@ -35,15 +35,15 @@ pub(crate) fn circom_default_dir() -> Utf8PathBuf {
 }
 
 pub(crate) fn proofs_dir() -> &'static Utf8PathBuf {
-    &CLI_CONFIG.get_or_init(CliSettings::default).proofs_dir
+    &cli_config(None).proofs_dir
 }
 
 pub(crate) fn commits_dir() -> &'static Utf8PathBuf {
-    &CLI_CONFIG.get_or_init(CliSettings::default).commits_dir
+    &cli_config(None).commits_dir
 }
 
 pub(crate) fn circom_dir() -> &'static Utf8PathBuf {
-    &CLI_CONFIG.get_or_init(CliSettings::default).circom_dir
+    &cli_config(None).circom_dir
 }
 
 fn lurk_leaf_dirs() -> [&'static Utf8PathBuf; 4] {

diff --git a/src/config.rs b/src/config.rs
@@ -16,6 +16,11 @@ pub static LURK_CONFIG: OnceCell<Settings> = OnceCell::new();
 /// Not meant to be changed by the end user, but useful for development (e.g unit tests)
 pub static LURK_CONFIG_FILE: Lazy<Utf8PathBuf> = Lazy::new(|| lurk_dir().join("lurk.toml"));
 
+pub fn lurk_config(overrides: Option<&HashMap<&str, String>>) -> &'static Settings {
+    LURK_CONFIG
+        .get_or_init(|| Settings::from_config(&LURK_CONFIG_FILE, overrides).unwrap_or_default())
+}
+
 /// Contains the Lurk config settings
 /// The config file is located in `$HOME/.config/lurk.toml` or `<current_dir>/.config/lurk.toml` on WASM.
 /// The config syntax is as follows:
@@ -38,16 +43,11 @@ pub struct Settings {
 }
 
 impl Settings {
-    /// Creates a default config, which can be modified before setting `LURK_CONFIG`
-    pub fn new() -> Self {
-        Self::default()
-    }
-
     /// Loads config settings from a file or env var
     /// The public parameter disk cache can also be overriden by a CLI arg if applicable
     pub fn from_config(
         config_file: &Utf8PathBuf,
-        cli_settings: &HashMap<&str, String>,
+        cli_settings: Option<&HashMap<&str, String>>,
     ) -> Result<Self, ConfigError> {
         let public_params = "public_params_dir";
         // Settings are read first to last, in order of increasing precedence.
@@ -59,8 +59,9 @@ impl Settings {
             .add_source(File::with_name(config_file.as_str()).required(false))
             // Then overwrite with any `LURK` environment variables
             .add_source(Environment::with_prefix("LURK"))
+
             // Optionally override if settings were specified via CLI arg
-            .set_override_option(public_params, cli_settings.get(public_params).map(|v| v.to_owned()))?
+            .set_override_option(public_params, cli_settings.and_then(|s| s.get(public_params).map(|v| v.to_owned())))?
             .build()
             .and_then(|c| c.try_deserialize())
     }
@@ -253,15 +254,14 @@ mod tests {
             .write_all("perf = \"max-parallel-simple\"\n".as_bytes())
             .unwrap();
 
-        let config = Settings::from_config(&config_dir, &HashMap::new()).unwrap();
+        let config = Settings::from_config(&config_dir, None).unwrap();
 
         assert_eq!(config.public_params_dir, public_params_dir);
         assert_eq!(config.perf, perf_config);
     }
 
     // Tests overwriting the config file and CLI argument
-    // Doesn't test env var as it can overwrite other tests
-    // when run in parallel
+    // Doesn't test env var as it can overwrite other tests when run in parallel
     #[test]
     fn test_config_override() {
         let tmp_dir = Builder::new().prefix("tmp").tempdir().unwrap();
@@ -277,12 +277,12 @@ mod tests {
             .write_all("perf = \"parallel-steps-only\"\n".as_bytes())
             .unwrap();
 
-        // Overwrite public params dir with CLI setting
+        // Overwrite public params dir to simulate CLI setting
         let public_params_dir_cli = tmp_dir.join("public_params_cli");
-        let mut cli_settings = HashMap::new();
-        cli_settings.insert("public_params_dir", public_params_dir_cli.to_string());
+        let mut overrides = HashMap::new();
+        overrides.insert("public_params_dir", public_params_dir_cli.to_string());
 
-        let config = Settings::from_config(&config_dir, &cli_settings).unwrap();
+        let config = Settings::from_config(&config_dir, Some(&overrides)).unwrap();
 
         assert_eq!(config.public_params_dir, public_params_dir_cli);
         assert_eq!(config.perf, PerfConfig::parallel_steps_only());
@@ -305,6 +305,6 @@ mod tests {
             .write_all(format!("public_params_dir = \"{public_params_dir_dup}\"\n").as_bytes())
             .unwrap();
 
-        assert!(Settings::from_config(&config_dir, &HashMap::new()).is_err())
+        assert!(Settings::from_config(&config_dir, None).is_err())
     }
 }
diff --git a/src/public_parameters/disk_cache.rs b/src/public_parameters/disk_cache.rs
@@ -6,7 +6,7 @@ use abomonation::{encode, Abomonation};
 use camino::{Utf8Path, Utf8PathBuf};
 use nova::traits::Group;
 
-use crate::config::{Settings, LURK_CONFIG};
+use crate::config::lurk_config;
 use crate::coprocessor::Coprocessor;
 use crate::proof::nova::{CurveCycleEquipped, PublicParams, G1, G2};
 use crate::proof::MultiFrameTrait;
@@ -17,7 +17,7 @@ use super::instance::Instance;
 /// Returns the public parameter disk cache directory, which has
 /// either been configured or defaults to `$HOME/.lurk/public_params`
 pub(crate) fn public_params_dir() -> &'static Utf8PathBuf {
-    &LURK_CONFIG.get_or_init(Settings::default).public_params_dir
+    &lurk_config(None).public_params_dir
 }
 
 pub(crate) struct DiskCache<'a, F, C, M>