Merge #1711

1711: Add data directory configuration option r=tcharding a=tcharding

Currently we have `--seed-file` for the seed file and a config option for the database.  Both of these files default to the systems data directory with `comit/` appended.  It makes more sense to configure these two items in the same fashion.

Add configuration option
```
[Data]
dir = path/to/data/dir
```

Use this path with `cnd.sqlite` and `seed.pem` appended for the database and seed file respectively.

Remove `--seed-file` command line option and the database configuration option.


Resolves: #1583

Co-authored-by: Tobin C. Harding <[email protected]>

Author: bors[bot]

api_tests/lib/cnd_runner.ts

@@ -1,4 +1,5 @@
 import * as fs from "fs";
+import * as path from "path";
 import { promisify } from "util";
 import { CndInstance } from "../lib_sdk/cnd_instance";
 import { CND_CONFIGS } from "./config";
@@ -43,8 +44,10 @@ export class CndRunner {
                 ledgerConfig
             );
 
-            if (await existsAsync(cndconfig.dbPath)) {
-                await unlinkAsync(cndconfig.dbPath); // delete the old database for the new test
+            const db = path.join(cndconfig.data, "cnd.sqlite");
+
+            if (await existsAsync(db)) {
+                await unlinkAsync(db); // delete the old database for the new test
             }
 
             await process.start();

api_tests/lib/config.ts

@@ -1,11 +1,11 @@
-import tempfile from "tempfile";
+import * as tmp from "tmp";
 import { BitcoinNodeConfig } from "./bitcoin";
 import { EthereumNodeConfig } from "./ethereum";
 import { LedgerConfig } from "./ledger_runner";
 
 export interface CndConfigFile {
     http_api: HttpApi;
-    database?: { sqlite: string };
+    data?: { dir: string };
     network: { listen: string[] };
 }
 
@@ -15,7 +15,7 @@ export interface HttpApi {
 
 export class E2ETestActorConfig {
     public readonly seed: Uint8Array;
-    public readonly dbPath: string;
+    public readonly data: string;
 
     constructor(
         public readonly httpApiPort: number,
@@ -26,7 +26,11 @@ export class E2ETestActorConfig {
         this.httpApiPort = httpApiPort;
         this.comitPort = comitPort;
         this.seed = new Uint8Array(Buffer.from(seed, "hex"));
-        this.dbPath = tempfile(`.${this.name}.sqlite`);
+
+        const tmpobj = tmp.dirSync();
+        tmpobj.removeCallback(); // Manual cleanup
+
+        this.data = tmpobj.name;
     }
 
     public generateCndConfigFile(ledgerConfig: LedgerConfig): CndConfigFile {
@@ -37,8 +41,8 @@ export class E2ETestActorConfig {
                     port: this.httpApiPort,
                 },
             },
-            database: {
-                sqlite: this.dbPath,
+            data: {
+                dir: this.data,
             },
             network: {
                 listen: [`/ip4/0.0.0.0/tcp/${this.comitPort}`],

api_tests/lib_sdk/cnd_instance.ts

@@ -1,7 +1,6 @@
 import { JsonMap, stringify } from "@iarna/toml";
 import { ChildProcess, spawn } from "child_process";
 import * as fs from "fs";
-import { PEMObject } from "pem-ts";
 import tempWrite from "temp-write";
 import { promisify } from "util";
 import { CndConfigFile, E2ETestActorConfig } from "../lib/config";
@@ -39,27 +38,20 @@ export class CndInstance {
             "config.toml"
         );
 
-        const pemObject = new PEMObject("SEED", this.actorConfig.seed);
-        const seedFile = await tempWrite(pemObject.encoded, "seed.pem");
-
-        this.process = spawn(
-            bin,
-            ["--config", configFile, "--seed-file", seedFile],
-            {
-                cwd: this.projectRoot,
-                stdio: [
-                    "ignore", // stdin
-                    await openAsync(
-                        this.logDir + "/cnd-" + this.actorConfig.name + ".log",
-                        "w"
-                    ), // stdout
-                    await openAsync(
-                        this.logDir + "/cnd-" + this.actorConfig.name + ".log",
-                        "w"
-                    ), // stderr
-                ],
-            }
-        );
+        this.process = spawn(bin, ["--config", configFile], {
+            cwd: this.projectRoot,
+            stdio: [
+                "ignore", // stdin
+                await openAsync(
+                    this.logDir + "/cnd-" + this.actorConfig.name + ".log",
+                    "w"
+                ), // stdout
+                await openAsync(
+                    this.logDir + "/cnd-" + this.actorConfig.name + ".log",
+                    "w"
+                ), // stderr
+            ],
+        });
 
         this.process.on("exit", (code: number, signal: number) => {
             console.log(

api_tests/package.json

@@ -55,8 +55,8 @@
         "rimraf": "^3.0.0",
         "satoshi-bitcoin": "^1.0.4",
         "temp-write": "^4.0.0",
-        "tempfile": "^3.0.0",
         "testcontainers": "^2.1.0",
+        "tmp": "^0.1.0",
         "ts-node": "^8.5.4",
         "tslint": "^5.20.1",
         "tslint-config-prettier": "^1.18.0",
@@ -67,5 +67,8 @@
     "prettier": {
         "trailingComma": "es5",
         "tabWidth": 4
+    },
+    "devDependencies": {
+        "@types/tmp": "^0.1.0"
     }
 }

api_tests/yarn.lock

@@ -154,6 +154,11 @@
   dependencies:
     tempfile "*"
 
+"@types/tmp@^0.1.0":
+  version "0.1.0"
+  resolved "https://registry.yarnpkg.com/@types/tmp/-/tmp-0.1.0.tgz#19cf73a7bcf641965485119726397a096f0049bd"
+  integrity sha512-6IwZ9HzWbCq6XoQWhxLpDjuADodH/MKXRUIDFudvgjcVdjFknvmR+DNsoUeer4XPrEnrZs04Jj+kfV9pFsrhmA==
+
 "@types/tv4@*":
   version "1.2.29"
   resolved "https://registry.yarnpkg.com/@types/tv4/-/tv4-1.2.29.tgz#4c6d2222b03245dd2104f4fd67f54d1658985911"
@@ -2557,6 +2562,13 @@ rfdc@^1.1.4:
   resolved "https://registry.yarnpkg.com/rfdc/-/rfdc-1.1.4.tgz#ba72cc1367a0ccd9cf81a870b3b58bd3ad07f8c2"
   integrity sha512-5C9HXdzK8EAqN7JDif30jqsBzavB7wLpaubisuQIGHWf2gUXSpzy6ArX/+Da8RjFpagWsCn+pIgxTMAmKw9Zug==
 
+rimraf@^2.6.3:
+  version "2.7.1"
+  resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-2.7.1.tgz#35797f13a7fdadc566142c29d4f07ccad483e3ec"
+  integrity sha512-uWjbaKIK3T1OSVptzX7Nl6PvQ3qAGtKEtVRjRuazjfL3Bx5eI409VZSqgND+4UNnmzLVdPj9FqFJNPqBZFve4w==
+  dependencies:
+    glob "^7.1.3"
+
 rimraf@^3.0.0:
   version "3.0.0"
   resolved "https://registry.yarnpkg.com/rimraf/-/rimraf-3.0.0.tgz#614176d4b3010b75e5c390eb0ee96f6dc0cebb9b"
@@ -2914,7 +2926,7 @@ temp-write@^4.0.0:
     temp-dir "^1.0.0"
     uuid "^3.3.2"
 
-tempfile@*, tempfile@^3.0.0:
+tempfile@*:
   version "3.0.0"
   resolved "https://registry.yarnpkg.com/tempfile/-/tempfile-3.0.0.tgz#5376a3492de7c54150d0cc0612c3f00e2cdaf76c"
   integrity sha512-uNFCg478XovRi85iD42egu+eSFUmmka750Jy7L5tfHI5hQKKtbPnxaSaXAbBqCDYrw3wx4tXjKwci4/QmsZJxw==
@@ -2974,6 +2986,13 @@ tiny-secp256k1@^1.1.0, tiny-secp256k1@^1.1.1:
     elliptic "^6.4.0"
     nan "^2.13.2"
 
+tmp@^0.1.0:
+  version "0.1.0"
+  resolved "https://registry.yarnpkg.com/tmp/-/tmp-0.1.0.tgz#ee434a4e22543082e294ba6201dcc6eafefa2877"
+  integrity sha512-J7Z2K08jbGcdA1kkQpJSqLF6T0tdQqpR2pnSUXsIchbPdTI9v3e85cLW0d6WDhwuAleOV71j2xWs8qMPfK7nKw==
+  dependencies:
+    rimraf "^2.6.3"
+
 to-buffer@^1.1.1:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/to-buffer/-/to-buffer-1.1.1.tgz#493bd48f62d7c43fcded313a03dcadb2e1213a80"

cnd/src/cli.rs

@@ -3,15 +3,11 @@ use std::path::PathBuf;
 #[derive(structopt::StructOpt, Debug)]
 #[structopt(name = "COMIT network daemon")]
 pub struct Options {
-    /// Path to configuration file.
+    /// Path to configuration file
     #[structopt(short = "c", long = "config", parse(from_os_str))]
     pub config_file: Option<PathBuf>,
 
-    /// Path to secret seed file.
-    #[structopt(short = "s", long = "seed-file", parse(from_os_str))]
-    pub seed_file: Option<PathBuf>,
-
-    /// Dump the current configuration and exit.
+    /// Dump the current configuration and exit
     #[structopt(long = "dump-config")]
     pub dump_config: bool,
 }

cnd/src/config/file.rs

@@ -1,4 +1,4 @@
-use crate::config::{Bitcoin, Database, Ethereum, Network, Socket};
+use crate::config::{Bitcoin, Data, Ethereum, Network, Socket};
 use config as config_rs;
 use log::LevelFilter;
 use std::{ffi::OsStr, path::Path};
@@ -12,7 +12,7 @@ use std::{ffi::OsStr, path::Path};
 pub struct File {
     pub network: Option<Network>,
     pub http_api: Option<HttpApi>,
-    pub database: Option<Database>,
+    pub data: Option<Data>,
     pub logging: Option<Logging>,
     pub bitcoin: Option<Bitcoin>,
     pub ethereum: Option<Ethereum>,
@@ -23,7 +23,7 @@ impl File {
         File {
             network: Option::None,
             http_api: Option::None,
-            database: Option::None,
+            data: Option::None,
             logging: Option::None,
             bitcoin: Option::None,
             ethereum: Option::None,
@@ -160,8 +160,8 @@ port = 8000
 [http_api.cors]
 allowed_origins = "all"
 
-[database]
-sqlite = "/tmp/foobar.sqlite"
+[data]
+dir = "/tmp/comit/"
 
 [logging]
 level = "DEBUG"
@@ -188,8 +188,8 @@ node_url = "http://example.com/"
                     allowed_origins: AllowedOrigins::All(All::All),
                 }),
             }),
-            database: Some(Database {
-                sqlite: PathBuf::from("/tmp/foobar.sqlite"),
+            data: Some(Data {
+                dir: PathBuf::from("/tmp/comit/"),
             }),
             logging: Some(Logging {
                 level: Some(LevelFilter::Debug),

cnd/src/config/mod.rs

@@ -9,8 +9,8 @@ use std::{net::IpAddr, path::PathBuf};
 pub use self::{file::File, settings::Settings};
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
-pub struct Database {
-    pub sqlite: PathBuf,
+pub struct Data {
+    pub dir: PathBuf,
 }
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]

cnd/src/config/settings.rs

@@ -1,11 +1,8 @@
-use crate::config::{file, Bitcoin, Database, Ethereum, File, Network, Socket};
+use crate::config::{file, Bitcoin, Data, Ethereum, File, Network, Socket};
 use anyhow::Context;
 use log::LevelFilter;
 use reqwest::Url;
-use std::{
-    net::{IpAddr, Ipv4Addr},
-    path::Path,
-};
+use std::net::{IpAddr, Ipv4Addr};
 
 /// This structs represents the settings as they are used through out the code.
 ///
@@ -17,7 +14,7 @@ use std::{
 pub struct Settings {
     pub network: Network,
     pub http_api: HttpApi,
-    pub database: Database,
+    pub data: Data,
     pub logging: Logging,
     pub bitcoin: Bitcoin,
     pub ethereum: Ethereum,
@@ -28,7 +25,7 @@ impl From<Settings> for File {
         let Settings {
             network,
             http_api: HttpApi { socket, cors },
-            database,
+            data,
             logging: Logging { level, structured },
             bitcoin,
             ethereum,
@@ -46,7 +43,7 @@ impl From<Settings> for File {
                     },
                 }),
             }),
-            database: Some(database),
+            data: Some(data),
             logging: Some(file::Logging {
                 level: Some(level),
                 structured: Some(structured),
@@ -108,7 +105,7 @@ impl Settings {
         let File {
             network,
             http_api,
-            database,
+            data,
             logging,
             bitcoin,
             ethereum,
@@ -143,14 +140,14 @@ impl Settings {
                     HttpApi { socket, cors }
                 })
                 .unwrap_or_default(),
-            database: {
-                let default_database_path = crate::data_dir()
-                    .map(|dir| Path::join(&dir, "cnd.sqlite"))
-                    .context("unable to determine default database path")?;
-                database.unwrap_or_else(|| Database {
-                    sqlite: default_database_path,
+            data: {
+                let default_data_dir =
+                    crate::data_dir().context("unable to determine default data path")?;
+                data.unwrap_or_else(|| Data {
+                    dir: default_data_dir,
                 })
             },
+
             logging: {
                 let Logging {
                     level: default_level,

cnd/src/db/mod.rs

@@ -25,7 +25,11 @@ use crate::{
     swap_protocols::{Role, SwapId},
 };
 use diesel::{self, prelude::*, sqlite::SqliteConnection};
-use std::{path::Path, sync::Arc};
+use std::{
+    ffi::OsStr,
+    path::{Path, PathBuf},
+    sync::Arc,
+};
 
 /// This module provides persistent storage by way of Sqlite.
 
@@ -39,15 +43,28 @@ pub struct Sqlite {
 impl Sqlite {
     /// Return a handle that can be used to access the database.
     ///
-    /// When this returns an Sqlite database exists at 'path', a
-    /// successful connection to the database has been made, and
-    /// the database migrations have been run.
-    pub fn new(path: &Path) -> anyhow::Result<Self> {
-        ensure_folder_tree_exists(path)?;
+    /// When this returns, an Sqlite database file 'cnd.sql' exists in 'dir', a
+    /// successful connection to the database has been made, and the database
+    /// migrations have been run.
+    pub fn new_in_dir<D: AsRef<OsStr>>(dir: D) -> anyhow::Result<Self> {
+        let dir = Path::new(&dir);
+        let path = db_path_from_dir(dir);
+        Sqlite::new(&path)
+    }
+
+    /// Return a handle that can be used to access the database.
+    ///
+    /// Reads or creates an SQLite database file at 'file'.  When this returns
+    /// an Sqlite database exists, a successful connection to the database has
+    /// been made, and the database migrations have been run.
+    pub fn new(file: &Path) -> anyhow::Result<Self> {
+        ensure_folder_tree_exists(file)?;
 
-        let connection = SqliteConnection::establish(&format!("file:{}", path.display()))?;
+        let connection = SqliteConnection::establish(&format!("file:{}", file.display()))?;
         embedded_migrations::run(&connection)?;
 
+        log::info!("SQLite database file: {}", file.display());
+
         Ok(Sqlite {
             connection: Arc::new(async_std::sync::Mutex::new(connection)),
         })
@@ -86,6 +103,12 @@ impl Sqlite {
     }
 }
 
+// Construct an absolute path to the database file using 'dir' as the base.
+fn db_path_from_dir(dir: &Path) -> PathBuf {
+    let path = dir.to_path_buf();
+    path.join("cnd.sqlite")
+}
+
 fn ensure_folder_tree_exists(path: &Path) -> anyhow::Result<()> {
     if let Some(parent) = path.parent() {
         std::fs::create_dir_all(parent)?;